diff --git a/ocis-pkg/oidc/client.go b/ocis-pkg/oidc/client.go index ea415166c..57bc43a07 100644 --- a/ocis-pkg/oidc/client.go +++ b/ocis-pkg/oidc/client.go @@ -48,6 +48,8 @@ type oidcClient struct { // Logger to use for logging, must be set Logger log.Logger + clientID string + skipClientIDCheck bool issuer string provider *ProviderMetadata providerLock *sync.Mutex @@ -90,6 +92,8 @@ func NewOIDCClient(opts ...Option) OIDCProvider { JWKSOptions: options.JWKSOptions, // TODO I don't like that we pass down config options ... providerLock: &sync.Mutex{}, jwksLock: &sync.Mutex{}, + clientID: options.ClientID, + skipClientIDCheck: options.SkipClientIDCheck, } } @@ -316,17 +320,17 @@ func (c *oidcClient) VerifyLogoutToken(ctx context.Context, rawIDToken string) ( return nil, fmt.Errorf("oidc: nonce on logout token MUST NOT be present") } // Check issuer. - if !v.config.SkipIssuerCheck && token.Issuer != v.issuer { - return nil, fmt.Errorf("oidc: id token issued by a different provider, expected %q got %q", v.issuer, token.Issuer) + if !c.skipIssuerValidation && token.Issuer != c.issuer { + return nil, fmt.Errorf("oidc: id token issued by a different provider, expected %q got %q", c.issuer, token.Issuer) } // If a client ID has been provided, make sure it's part of the audience. SkipClientIDCheck must be true if ClientID is empty. // // This check DOES NOT ensure that the ClientID is the party to which the ID Token was issued (i.e. Authorized party). - if !v.config.SkipClientIDCheck { - if v.config.ClientID != "" { - if !contains(token.Audience, v.config.ClientID) { - return nil, fmt.Errorf("oidc: expected audience %q got %q", v.config.ClientID, token.Audience) + if !c.skipClientIDCheck { + if c.clientID != "" { + if !contains(token.Audience, c.clientID) { + return nil, fmt.Errorf("oidc: expected audience %q got %q", c.clientID, token.Audience) } } else { return nil, fmt.Errorf("oidc: invalid configuration, clientID must be provided or SkipClientIDCheck must be set") @@ -342,16 +346,16 @@ func (c *oidcClient) VerifyLogoutToken(ctx context.Context, rawIDToken string) ( } sig := jws.Signatures[0] - supportedSigAlgs := v.config.SupportedSigningAlgs + supportedSigAlgs := c.algorithms if len(supportedSigAlgs) == 0 { - supportedSigAlgs = []string{gOidc.RS256} + supportedSigAlgs = []string{RS256} } if !contains(supportedSigAlgs, sig.Header.Algorithm) { return nil, fmt.Errorf("oidc: id token signed with unsupported algorithm, expected %q got %q", supportedSigAlgs, sig.Header.Algorithm) } - gotPayload, err := v.keySet.VerifySignature(ctx, rawIDToken) + gotPayload, err := c.remoteKeySet.VerifySignature(ctx, rawIDToken) if err != nil { return nil, fmt.Errorf("failed to verify signature: %v", err) } diff --git a/ocis-pkg/oidc/metadata.go b/ocis-pkg/oidc/metadata.go index b5eec20fb..c67b03b1b 100644 --- a/ocis-pkg/oidc/metadata.go +++ b/ocis-pkg/oidc/metadata.go @@ -5,8 +5,8 @@ import ( "io" "net/http" "strings" - "time" + "github.com/golang-jwt/jwt/v4" "github.com/owncloud/ocis/v2/ocis-pkg/log" ) @@ -64,19 +64,19 @@ type LogoutToken struct { // this value may differ when using Google. // // See: https://developers.google.com/identity/protocols/OpenIDConnect#obtainuserinfo - Issuer string `json:iss` // example "https://server.example.com" + Issuer string `json:"iss"` // example "https://server.example.com" // A unique string which identifies the end user. - Subject string `json:sub` //"248289761001" + Subject string `json:"sub"` //"248289761001" // The client ID, or set of client IDs, that this token is issued for. For // common uses, this is the client that initialized the auth flow. // // This package ensures the audience contains an expected value. - Audience audience `json:aud` // "s6BhdRkqt3" + Audience jwt.ClaimStrings `json:"aud"` // "s6BhdRkqt3" // When the token was issued by the provider. - IssuedAt jsonTime `json:"iat"` + IssuedAt *jwt.NumericDate `json:"iat"` // The Session Id SessionId string `json:"sid"` @@ -91,44 +91,6 @@ type logoutEvent struct { Event *struct{} `json:"http://schemas.openid.net/event/backchannel-logout"` } -type audience []string - -func (a *audience) UnmarshalJSON(b []byte) error { - var s string - if json.Unmarshal(b, &s) == nil { - *a = audience{s} - return nil - } - var auds []string - if err := json.Unmarshal(b, &auds); err != nil { - return err - } - *a = auds - return nil -} - -type jsonTime time.Time - -func (j *jsonTime) UnmarshalJSON(b []byte) error { - var n json.Number - if err := json.Unmarshal(b, &n); err != nil { - return err - } - var unix int64 - - if t, err := n.Int64(); err == nil { - unix = t - } else { - f, err := n.Float64() - if err != nil { - return err - } - unix = int64(f) - } - *j = jsonTime(time.Unix(unix, 0)) - return nil -} - func GetIDPMetadata(logger log.Logger, client *http.Client, idpURI string) (ProviderMetadata, error) { wellknownURI := strings.TrimSuffix(idpURI, "/") + wellknownPath diff --git a/ocis-pkg/oidc/options.go b/ocis-pkg/oidc/options.go index 9085fee36..2cc5716b0 100644 --- a/ocis-pkg/oidc/options.go +++ b/ocis-pkg/oidc/options.go @@ -23,6 +23,11 @@ type Options struct { // AccessTokenVerifyMethod to use when verifying access tokens // TODO pass a function or interface to verify? an AccessTokenVerifier? AccessTokenVerifyMethod string + // ClientID the client id to expect in tokens. If not set SkipClientIDCheck must be true + // TODO also check in access token + ClientID string + // SkipClientIDCheck must be true if ClientID is empty + SkipClientIDCheck bool } // newOptions initializes the available default options. @@ -56,13 +61,31 @@ func WithAccessTokenVerifyMethod(val string) Option { o.AccessTokenVerifyMethod = val } } + +// WithHTTPClient provides a function to set the httpClient option. func WithHTTPClient(val *http.Client) Option { return func(o *Options) { o.HTTPClient = val } } + +// WithJWKSOptions provides a function to set the jwksOptions option. func WithJWKSOptions(val config.JWKS) Option { return func(o *Options) { o.JWKSOptions = val } } + +// WithClientID provides a function to set the clientID option. +func WithClientID(val string) Option { + return func(o *Options) { + o.ClientID = val + } +} + +// WithSkipClientIDCheck provides a function to set the skipClientIDCheck option. +func WithSkipClientIDCheck(val bool) Option { + return func(o *Options) { + o.SkipClientIDCheck = val + } +} diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 42c9bfdae..baadf11cd 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -43,13 +43,12 @@ import ( ) type StaticRouteHandler struct { - prefix string - proxy http.Handler - userInfoCache microstore.Store - sessionLookupCache microstore.Store - logger log.Logger - config config.Config - oidcClient oidc.OIDCProvider + prefix string + proxy http.Handler + userInfoCache microstore.Store + logger log.Logger + config config.Config + oidcClient oidc.OIDCProvider } // Server is the entrypoint for the server command. @@ -71,15 +70,6 @@ func Server(cfg *config.Config) *cli.Command { microstore.Table(cfg.OIDC.UserinfoCache.Table), ) - sessionLookupCache := store.Create( - store.Store(cfg.OIDC.SessionLookupCache.Store), - store.TTL(cfg.OIDC.SessionLookupCache.TTL), - store.Size(cfg.OIDC.SessionLookupCache.Size), - microstore.Nodes(cfg.OIDC.SessionLookupCache.Nodes...), - microstore.Database(cfg.OIDC.SessionLookupCache.Database), - microstore.Table(cfg.OIDC.SessionLookupCache.Table), - ) - logger := logging.Configure(cfg.Service.Name, cfg.Log) err := tracing.Configure(cfg) if err != nil { @@ -107,6 +97,8 @@ func Server(cfg *config.Config) *cli.Command { oidc.WithHTTPClient(oidcHTTPClient), oidc.WithOidcIssuer(cfg.OIDC.Issuer), oidc.WithJWKSOptions(cfg.OIDC.JWKS), + oidc.WithClientID(cfg.OIDC.ClientID), + oidc.WithSkipClientIDCheck(cfg.OIDC.SkipClientIDCheck), ) var ( @@ -131,20 +123,19 @@ func Server(cfg *config.Config) *cli.Command { ) lh := StaticRouteHandler{ - prefix: cfg.HTTP.Root, - userInfoCache: userInfoCache, - sessionLookupCache: sessionLookupCache, - logger: logger, - config: *cfg, - oidcClient: oidcClient, - proxy: rp, + prefix: cfg.HTTP.Root, + userInfoCache: userInfoCache, + logger: logger, + config: *cfg, + oidcClient: oidcClient, + proxy: rp, } if err != nil { return fmt.Errorf("failed to initialize reverse proxy: %w", err) } { - middlewares := loadMiddlewares(ctx, logger, cfg, userInfoCache, sessionLookupCache) + middlewares := loadMiddlewares(ctx, logger, cfg, userInfoCache) server, err := proxyHTTP.Server( proxyHTTP.Handler(lh.handler()), proxyHTTP.Logger(logger), @@ -238,17 +229,22 @@ func (h *StaticRouteHandler) backchannelLogout(w http.ResponseWriter, r *http.Re } for _, record := range records { - err = h.sessionLookupCache.Delete(string(record.Value)) - if errors.Is(err, microstore.ErrNotFound) { - render.Status(r, http.StatusOK) + err = h.userInfoCache.Delete(string(record.Value)) + if !errors.Is(err, microstore.ErrNotFound) { + // Spec requires us to return a 400 BadRequest when the session could not be destroyed + h.logger.Err(err).Msg("could not delete user info from cache") + render.Status(r, http.StatusBadRequest) return } } + // we can ignore errors when cleaning up the lookup table + _ = h.userInfoCache.Delete(logoutToken.SessionId) + render.Status(r, http.StatusOK) } -func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, userInfoCache microstore.Store, sessionLookupCache microstore.Store) alice.Chain { +func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, userInfoCache microstore.Store) alice.Chain { rolesClient := settingssvc.NewRoleService("com.owncloud.api.settings", grpc.DefaultClient()) revaClient, err := pool.GetGatewayServiceClient(cfg.Reva.Address, cfg.Reva.GetRevaOptions()...) if err != nil { @@ -326,7 +322,6 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config, authenticators = append(authenticators, middleware.NewOIDCAuthenticator( middleware.Logger(logger), middleware.UserInfoCache(userInfoCache), - middleware.SessionLookupCache(sessionLookupCache), middleware.DefaultAccessTokenTTL(cfg.OIDC.UserinfoCache.TTL), middleware.HTTPClient(oidcHTTPClient), middleware.OIDCIss(cfg.OIDC.Issuer), diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index c23cb56ed..f8640e815 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -106,9 +106,10 @@ type OIDC struct { Insecure bool `yaml:"insecure" env:"OCIS_INSECURE;PROXY_OIDC_INSECURE" desc:"Disable TLS certificate validation for connections to the IDP. Note that this is not recommended for production environments."` AccessTokenVerifyMethod string `yaml:"access_token_verify_method" env:"PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD" desc:"Sets how OIDC access tokens should be verified. Possible values are 'none' and 'jwt'. When using 'none', no special validation apart from using it for accessing the IPD's userinfo endpoint will be done. When using 'jwt', it tries to parse the access token as a jwt token and verifies the signature using the keys published on the IDP's 'jwks_uri'."` UserinfoCache *Cache `yaml:"user_info_cache"` - SessionLookupCache *Cache `yaml:"session_lookup_cache"` JWKS JWKS `yaml:"jwks"` RewriteWellKnown bool `yaml:"rewrite_well_known" env:"PROXY_OIDC_REWRITE_WELLKNOWN" desc:"Enables rewriting the /.well-known/openid-configuration to the configured OIDC issuer. Needed by the Desktop Client, Android Client and iOS Client to discover the OIDC provider."` + ClientID string `yaml:"client_id" env:"OCIS_OIDC_CLIENT_ID;PROXY_OIDC_CLIENT_ID" desc:"OIDC client ID, which ownCloud Web uses. This client needs to be set up in your IDP."` + SkipClientIDCheck bool `yaml:"skip_client_id_check" env:"PROXY_OIDC_SKIP_CLIENT_ID_CHECK" desc:"If true will skip checking the configured client id is present in audience claims."` } type JWKS struct { diff --git a/services/proxy/pkg/config/defaults/defaultconfig.go b/services/proxy/pkg/config/defaults/defaultconfig.go index b3316083e..956ca69c6 100644 --- a/services/proxy/pkg/config/defaults/defaultconfig.go +++ b/services/proxy/pkg/config/defaults/defaultconfig.go @@ -53,6 +53,7 @@ func DefaultConfig() *config.Config { RefreshTimeout: 10, // seconds RefreshUnknownKID: true, }, + ClientID: "web", }, PolicySelector: nil, RoleAssignment: config.RoleAssignment{ diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index 61cdc25d1..9826c99b6 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -31,7 +31,6 @@ func NewOIDCAuthenticator(opts ...Option) *OIDCAuthenticator { return &OIDCAuthenticator{ Logger: options.Logger, userInfoCache: options.UserInfoCache, - sessionLookupCache: options.SessionLookupCache, DefaultTokenCacheTTL: options.DefaultAccessTokenTTL, HTTPClient: options.HTTPClient, OIDCIss: options.OIDCIss, @@ -46,7 +45,6 @@ type OIDCAuthenticator struct { HTTPClient *http.Client OIDCIss string userInfoCache store.Store - sessionLookupCache store.Store DefaultTokenCacheTTL time.Duration oidcClient oidc.OIDCProvider AccessTokenVerifyMethod string @@ -108,7 +106,8 @@ func (m *OIDCAuthenticator) getClaims(token string, req *http.Request) (map[stri } if sid, ok := claims["sid"]; ok { - err = m.sessionLookupCache.Write(&store.Record{ + // reuse user cache for session id lookup + err = m.userInfoCache.Write(&store.Record{ Key: fmt.Sprintf("%s", sid), Value: []byte(encodedHash), Expiry: time.Until(expiration), diff --git a/services/proxy/pkg/middleware/options.go b/services/proxy/pkg/middleware/options.go index abbb66d75..901b84e16 100644 --- a/services/proxy/pkg/middleware/options.go +++ b/services/proxy/pkg/middleware/options.go @@ -56,8 +56,6 @@ type Options struct { DefaultAccessTokenTTL time.Duration // UserInfoCache sets the access token cache store UserInfoCache store.Store - // SessionLookupCache maps the session to a hashed jwt token - SessionLookupCache store.Store // CredentialsByUserAgent sets the auth challenges on a per user-agent basis CredentialsByUserAgent map[string]string // AccessTokenVerifyMethod configures how access_tokens should be verified but the oidc_auth middleware. @@ -200,13 +198,6 @@ func UserInfoCache(val store.Store) Option { } } -// SessionLookupCache provides a function to set the SessionLookupCache -func SessionLookupCache(val store.Store) Option { - return func(o *Options) { - o.SessionLookupCache = val - } -} - // UserProvider sets the accounts user provider func UserProvider(up backend.UserBackend) Option { return func(o *Options) { diff --git a/services/web/pkg/config/config.go b/services/web/pkg/config/config.go index 41e622fd1..279df73ec 100644 --- a/services/web/pkg/config/config.go +++ b/services/web/pkg/config/config.go @@ -67,7 +67,7 @@ type WebConfig struct { type OIDC struct { MetadataURL string `json:"metadata_url,omitempty" yaml:"metadata_url" env:"WEB_OIDC_METADATA_URL" desc:"URL for the OIDC well-known configuration endpoint. Defaults to the oCIS API URL + \"/.well-known/openid-configuration\"."` Authority string `json:"authority,omitempty" yaml:"authority" env:"OCIS_URL;OCIS_OIDC_ISSUER;WEB_OIDC_AUTHORITY" desc:"URL of the OIDC issuer. It defaults to URL of the builtin IDP."` - ClientID string `json:"client_id,omitempty" yaml:"client_id" env:"WEB_OIDC_CLIENT_ID" desc:"OIDC client ID, which ownCloud Web uses. This client needs to be set up in your IDP."` + ClientID string `json:"client_id,omitempty" yaml:"client_id" env:"OCIS_OIDC_CLIENT_ID;WEB_OIDC_CLIENT_ID" desc:"OIDC client ID, which ownCloud Web uses. This client needs to be set up in your IDP."` ResponseType string `json:"response_type,omitempty" yaml:"response_type" env:"WEB_OIDC_RESPONSE_TYPE" desc:"OIDC response type to use for authentication."` Scope string `json:"scope,omitempty" yaml:"scope" env:"WEB_OIDC_SCOPE" desc:"OIDC scopes to request during authentication to authorize access to user details. Defaults to 'openid profile email'. Values are separated by blank. More example values but not limited to are 'address' or 'phone' etc."` }