From 06ffd9cf8a66f3c747a362c90f0db1911da246a2 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Wed, 10 Aug 2022 13:39:15 +0200 Subject: [PATCH] some more cleaning up --- services/proxy/pkg/command/server.go | 21 ++++++-------- .../proxy/pkg/middleware/authentication.go | 2 ++ services/proxy/pkg/middleware/oidc_auth.go | 29 +++++++++++++++---- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 7377d95829..e54538f716 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -16,7 +16,6 @@ import ( "github.com/owncloud/ocis/v2/ocis-pkg/log" pkgmiddleware "github.com/owncloud/ocis/v2/ocis-pkg/middleware" "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" - "github.com/owncloud/ocis/v2/ocis-pkg/sync" "github.com/owncloud/ocis/v2/ocis-pkg/version" settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0" storesvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/store/v0" @@ -175,14 +174,12 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config) UserProvider: userProvider, }) } - tokenCache := sync.NewCache(cfg.OIDC.UserinfoCache.Size) - authenticators = append(authenticators, middleware.OIDCAuthenticator{ - Logger: logger, - TokenCache: &tokenCache, - TokenCacheTTL: time.Duration(cfg.OIDC.UserinfoCache.TTL), - HTTPClient: oidcHTTPClient, - OIDCIss: cfg.OIDC.Issuer, - ProviderFunc: func() (middleware.OIDCProvider, error) { + authenticators = append(authenticators, middleware.NewOIDCAuthenticator( + logger, + cfg.OIDC.UserinfoCache.TTL, + oidcHTTPClient, + cfg.OIDC.Issuer, + func() (middleware.OIDCProvider, error) { // Initialize a provider by specifying the issuer URL. // it will fetch the keys from the issuer using the .well-known // endpoint @@ -191,9 +188,9 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config) cfg.OIDC.Issuer, ) }, - JWKSOptions: cfg.OIDC.JWKS, - AccessTokenVerifyMethod: cfg.OIDC.AccessTokenVerifyMethod, - }) + cfg.OIDC.JWKS, + cfg.OIDC.AccessTokenVerifyMethod, + )) authenticators = append(authenticators, middleware.PublicShareAuthenticator{ Logger: logger, RevaGatewayClient: revaClient, diff --git a/services/proxy/pkg/middleware/authentication.go b/services/proxy/pkg/middleware/authentication.go index 887e116be2..53dfd41435 100644 --- a/services/proxy/pkg/middleware/authentication.go +++ b/services/proxy/pkg/middleware/authentication.go @@ -36,6 +36,7 @@ var ( "/data": {}, "/konnect/v1/userinfo": {}, "/status.php": {}, + "/favicon.ico": {}, } // _unprotectedPathPrefixes contains paths which don't need to be authenticated. _unprotectedPathPrefixes = [...]string{ @@ -44,6 +45,7 @@ var ( "/user-management", "/.well-known", "/js", + "/css", "/icons", "/themes", "/signin", diff --git a/services/proxy/pkg/middleware/oidc_auth.go b/services/proxy/pkg/middleware/oidc_auth.go index a6c1b5519d..a59a3b2457 100644 --- a/services/proxy/pkg/middleware/oidc_auth.go +++ b/services/proxy/pkg/middleware/oidc_auth.go @@ -30,11 +30,28 @@ type OIDCProvider interface { UserInfo(ctx context.Context, ts oauth2.TokenSource) (*gOidc.UserInfo, error) } +func NewOIDCAuthenticator(logger log.Logger, tokenCacheTTL int, oidcHTTPClient *http.Client, oidcIss string, providerFunc func() (OIDCProvider, error), + jwksOptions config.JWKS, accessTokenVerifyMethod string) OIDCAuthenticator { + tokenCache := osync.NewCache(tokenCacheTTL) + return OIDCAuthenticator{ + Logger: logger, + tokenCache: &tokenCache, + TokenCacheTTL: time.Duration(tokenCacheTTL), + HTTPClient: oidcHTTPClient, + OIDCIss: oidcIss, + ProviderFunc: providerFunc, + JWKSOptions: jwksOptions, + AccessTokenVerifyMethod: accessTokenVerifyMethod, + providerLock: &sync.Mutex{}, + jwksLock: &sync.Mutex{}, + } +} + type OIDCAuthenticator struct { Logger log.Logger HTTPClient *http.Client OIDCIss string - TokenCache *osync.Cache + tokenCache *osync.Cache TokenCacheTTL time.Duration ProviderFunc func() (OIDCProvider, error) AccessTokenVerifyMethod string @@ -49,7 +66,7 @@ type OIDCAuthenticator struct { func (m OIDCAuthenticator) getClaims(token string, req *http.Request) (map[string]interface{}, error) { var claims map[string]interface{} - hit := m.TokenCache.Load(token) + hit := m.tokenCache.Load(token) if hit == nil { aClaims, err := m.verifyAccessToken(token) if err != nil { @@ -72,7 +89,7 @@ func (m OIDCAuthenticator) getClaims(token string, req *http.Request) (map[strin } expiration := m.extractExpiration(aClaims) - m.TokenCache.Store(token, claims, expiration) + m.tokenCache.Store(token, claims, expiration) m.Logger.Debug().Interface("claims", claims).Interface("userInfo", userInfo).Time("expiration", expiration.UTC()).Msg("unmarshalled and cached userinfo") return claims, nil @@ -149,7 +166,7 @@ type jwksJSON struct { JWKSURL string `json:"jwks_uri"` } -func (m *OIDCAuthenticator) getKeyfunc() *keyfunc.JWKS { +func (m OIDCAuthenticator) getKeyfunc() *keyfunc.JWKS { m.jwksLock.Lock() defer m.jwksLock.Unlock() if m.JWKS == nil { @@ -200,7 +217,7 @@ func (m *OIDCAuthenticator) getKeyfunc() *keyfunc.JWKS { return m.JWKS } -func (m *OIDCAuthenticator) getProvider() OIDCProvider { +func (m OIDCAuthenticator) getProvider() OIDCProvider { m.providerLock.Lock() defer m.providerLock.Unlock() if m.provider == nil { @@ -229,11 +246,11 @@ func (m OIDCAuthenticator) Authenticate(r *http.Request) (*http.Request, bool) { if m.getProvider() == nil { return nil, false } + // Force init of jwks keyfunc if needed (contacts the .well-known and jwks endpoints on first call) if m.AccessTokenVerifyMethod == config.AccessTokenVerificationJWT && m.getKeyfunc() == nil { return nil, false } - token := strings.TrimPrefix(r.Header.Get(_headerAuthorization), _bearerPrefix) claims, err := m.getClaims(token, r)