diff --git a/pkg/oidc/access_token_test.go b/pkg/oidc/access_token_test.go new file mode 100644 index 0000000000..978df5d7ee --- /dev/null +++ b/pkg/oidc/access_token_test.go @@ -0,0 +1,200 @@ +package oidc_test + +import ( + "context" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/pkg/oidc" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" + "github.com/stretchr/testify/require" +) + +func TestAccessTokenAudiences(t *testing.T) { + key := newRSAKey(t) + tests := []struct { + name string + audiences []string + aud any + missing bool + wantErr error + }{ + {name: "disabled missing", missing: true}, + {name: "disabled foreign", aud: "immich"}, + {name: "disabled null", aud: nil}, + {name: "disabled empty array", aud: []string{}}, + {name: "explicitly empty configuration", audiences: []string{}, aud: "immich"}, + {name: "single audience", audiences: []string{"opencloud"}, aud: "opencloud"}, + {name: "array first match", audiences: []string{"opencloud"}, aud: []string{"opencloud", "immich"}}, + {name: "array last match", audiences: []string{"opencloud"}, aud: []string{"immich", "opencloud"}}, + {name: "any allowed audience", audiences: []string{"opencloud", "opencloud-api"}, aud: "opencloud-api"}, + {name: "any allowed audience in array", audiences: []string{"opencloud", "opencloud-api"}, aud: []string{"immich", "opencloud-api"}}, + {name: "duplicate audiences", audiences: []string{"opencloud", "opencloud"}, aud: []string{"opencloud", "opencloud"}}, + {name: "URI audience", audiences: []string{"https://cloud.example/api"}, aud: "https://cloud.example/api"}, + {name: "foreign", audiences: []string{"opencloud"}, aud: "immich", wantErr: jwt.ErrTokenInvalidAudience}, + {name: "foreign array", audiences: []string{"opencloud"}, aud: []string{"immich", "account"}, wantErr: jwt.ErrTokenInvalidAudience}, + {name: "case sensitive", audiences: []string{"opencloud"}, aud: "OpenCloud", wantErr: jwt.ErrTokenInvalidAudience}, + {name: "exact match", audiences: []string{"opencloud"}, aud: "opencloud-api", wantErr: jwt.ErrTokenInvalidAudience}, + {name: "no token normalization", audiences: []string{"opencloud"}, aud: " opencloud ", wantErr: jwt.ErrTokenInvalidAudience}, + {name: "no wildcard matching", audiences: []string{"*"}, aud: "opencloud", wantErr: jwt.ErrTokenInvalidAudience}, + {name: "missing", audiences: []string{"opencloud"}, missing: true, wantErr: jwt.ErrTokenRequiredClaimMissing}, + {name: "null", audiences: []string{"opencloud"}, aud: nil, wantErr: jwt.ErrTokenRequiredClaimMissing}, + {name: "empty string", audiences: []string{"opencloud"}, aud: "", wantErr: jwt.ErrTokenRequiredClaimMissing}, + {name: "empty array", audiences: []string{"opencloud"}, aud: []string{}, wantErr: jwt.ErrTokenRequiredClaimMissing}, + {name: "array empty string", audiences: []string{"opencloud"}, aud: []string{""}, wantErr: jwt.ErrTokenRequiredClaimMissing}, + {name: "number", audiences: []string{"opencloud"}, aud: 123, wantErr: jwt.ErrTokenMalformed}, + {name: "object", audiences: []string{"opencloud"}, aud: map[string]string{"aud": "opencloud"}, wantErr: jwt.ErrTokenMalformed}, + {name: "mixed array", audiences: []string{"opencloud"}, aud: []any{"opencloud", 123}, wantErr: jwt.ErrTokenMalformed}, + {name: "null array entry", audiences: []string{"opencloud"}, aud: []any{"opencloud", nil}, wantErr: jwt.ErrTokenMalformed}, + {name: "disabled still rejects invalid type", aud: 123, wantErr: jwt.ErrTokenMalformed}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + claims := jwt.MapClaims{ + "iss": "https://issuer.example", + "sub": "alice", + "sid": "session", + "exp": time.Now().Add(time.Hour).Unix(), + } + if !tt.missing { + claims["aud"] = tt.aud + } + client := newAccessTokenTestClient(t, key, tt.audiences, &oidc.ProviderMetadata{}) + registered, all, err := client.VerifyAccessToken(context.Background(), signAccessToken(t, key, claims)) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + require.Empty(t, all, "unverified claims must not be returned") + return + } + require.NoError(t, err) + require.Equal(t, "alice", registered.Subject) + require.Equal(t, "session", registered.SessionID) + require.Equal(t, "alice", all["sub"]) + }) + } +} + +func TestAccessTokenValidationWithAudiences(t *testing.T) { + key, otherKey := newRSAKey(t), newRSAKey(t) + tests := []struct { + name string + issuer string + provider *oidc.ProviderMetadata + signingKey *signingKey + exp time.Time + nbf time.Time + wantErr error + }{ + {name: "invalid signature", signingKey: otherKey, wantErr: jwt.ErrTokenSignatureInvalid}, + {name: "invalid issuer", issuer: "https://other.example", wantErr: jwt.ErrTokenInvalidIssuer}, + {name: "expired", exp: time.Now().Add(-time.Hour), wantErr: jwt.ErrTokenExpired}, + {name: "not yet valid", nbf: time.Now().Add(time.Hour), wantErr: jwt.ErrTokenNotValidYet}, + {name: "AD FS access token issuer", issuer: "https://adfs.example", provider: &oidc.ProviderMetadata{AccessTokenIssuer: "https://adfs.example"}}, + {name: "AD FS rejects discovery issuer", provider: &oidc.ProviderMetadata{AccessTokenIssuer: "https://adfs.example"}, wantErr: jwt.ErrTokenInvalidIssuer}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.issuer == "" { + tt.issuer = "https://issuer.example" + } + if tt.provider == nil { + tt.provider = &oidc.ProviderMetadata{} + } + if tt.signingKey == nil { + tt.signingKey = key + } + if tt.exp.IsZero() { + tt.exp = time.Now().Add(time.Hour) + } + claims := jwt.MapClaims{"iss": tt.issuer, "sub": "alice", "aud": "opencloud", "exp": tt.exp.Unix()} + if !tt.nbf.IsZero() { + claims["nbf"] = tt.nbf.Unix() + } + client := newAccessTokenTestClient(t, key, []string{"opencloud"}, tt.provider) + _, _, err := client.VerifyAccessToken(context.Background(), signAccessToken(t, tt.signingKey, claims)) + require.ErrorIs(t, err, tt.wantErr) + }) + } +} + +func TestAccessTokenAudienceConfiguration(t *testing.T) { + for _, method := range []string{config.AccessTokenVerificationNone, ""} { + t.Run("incompatible method "+method, func(t *testing.T) { + // No HTTP client is supplied: invalid configuration must fail before discovery. + client, err := oidc.NewOIDCClient( + oidc.WithAccessTokenVerifyMethod(method), + oidc.WithAccessTokenAudiences([]string{"opencloud"}), + ) + require.ErrorContains(t, err, "requires the jwt verification method") + require.Nil(t, client) + }) + } + for _, audiences := range [][]string{{""}, {" \t"}, {"opencloud", ""}} { + client, err := oidc.NewOIDCClient( + oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationJWT), + oidc.WithAccessTokenAudiences(audiences), + ) + require.ErrorContains(t, err, "empty or whitespace-only") + require.Nil(t, client) + } + t.Run("none remains compatible when disabled", func(t *testing.T) { + client, err := oidc.NewOIDCClient( + oidc.WithLogger(log.NopLogger()), + oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationNone), + oidc.WithProviderMetadata(&oidc.ProviderMetadata{}), + ) + require.NoError(t, err) + _, _, err = client.VerifyAccessToken(context.Background(), "opaque-token") + require.NoError(t, err) + }) + t.Run("caller cannot mutate the policy", func(t *testing.T) { + key := newRSAKey(t) + audiences := []string{"opencloud"} + client := newAccessTokenTestClient(t, key, audiences, &oidc.ProviderMetadata{}) + audiences[0] = "immich" + _, _, err := client.VerifyAccessToken(context.Background(), signAccessToken(t, key, + jwt.MapClaims{"iss": "https://issuer.example", "aud": "immich"})) + require.ErrorIs(t, err, jwt.ErrTokenInvalidAudience) + }) +} + +func TestAccessTokenAudiencesDoNotApplyToLogoutTokens(t *testing.T) { + key := newRSAKey(t) + client := newAccessTokenTestClient(t, key, []string{"opencloud"}, &oidc.ProviderMetadata{}) + token := signAccessToken(t, key, jwt.MapClaims{ + "iss": "https://issuer.example", + "sub": "alice", + "aud": "web-client", + "events": map[string]any{ + "http://schemas.openid.net/event/backchannel-logout": map[string]any{}, + }, + }) + _, err := client.VerifyLogoutToken(context.Background(), token) + require.NoError(t, err) +} + +func newAccessTokenTestClient(t *testing.T, key *signingKey, audiences []string, provider *oidc.ProviderMetadata) oidc.OIDCClient { + t.Helper() + client, err := oidc.NewOIDCClient( + oidc.WithLogger(log.NopLogger()), + oidc.WithOidcIssuer("https://issuer.example"), + oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationJWT), + oidc.WithAccessTokenAudiences(audiences), + oidc.WithJWKS(key.jwks), + oidc.WithProviderMetadata(provider), + ) + require.NoError(t, err) + return client +} + +func signAccessToken(t *testing.T, key *signingKey, claims jwt.MapClaims) string { + t.Helper() + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + token.Header["kid"] = "1" + signed, err := token.SignedString(key.priv) + require.NoError(t, err) + return signed +} diff --git a/pkg/oidc/client.go b/pkg/oidc/client.go index b6065ec66f..220a4d3c57 100644 --- a/pkg/oidc/client.go +++ b/pkg/oidc/client.go @@ -57,6 +57,7 @@ type oidcClient struct { providerLock *sync.Mutex skipIssuerValidation bool accessTokenVerifyMethod string + accessTokenAudiences []string remoteKeySet KeySet algorithms []string @@ -82,22 +83,32 @@ var _supportedAlgorithms = map[string]bool{ PS512: true, } -// NewOIDCClient returns an OIDClient instance for the given issuer -func NewOIDCClient(opts ...Option) OIDCClient { +// NewOIDCClient returns an OIDCClient instance for the given issuer. +// Invalid access token audience configuration is rejected before creating the client. +func NewOIDCClient(opts ...Option) (OIDCClient, error) { options := newOptions(opts...) + if len(options.AccessTokenAudiences) > 0 && options.AccessTokenVerifyMethod != config.AccessTokenVerificationJWT { + return nil, errors.New("access token audience validation requires the jwt verification method") + } + for _, audience := range options.AccessTokenAudiences { + if strings.TrimSpace(audience) == "" { + return nil, errors.New("access token audiences must not contain empty or whitespace-only entries") + } + } return &oidcClient{ Logger: options.Logger, issuer: options.OIDCIssuer, httpClient: options.HTTPClient, accessTokenVerifyMethod: options.AccessTokenVerifyMethod, + accessTokenAudiences: options.AccessTokenAudiences, JWKSOptions: options.JWKSOptions, // TODO I don't like that we pass down config options ... JWKS: options.JWKS, providerLock: &sync.Mutex{}, jwksLock: &sync.Mutex{}, remoteKeySet: options.KeySet, provider: options.ProviderMetadata, - } + }, nil } func (c *oidcClient) lookupWellKnownOpenidConfiguration(ctx context.Context) error { @@ -301,7 +312,7 @@ func (c *oidcClient) verifyAccessTokenJWT(token string) (RegClaimsWithSID, jwt.M issuer = c.provider.AccessTokenIssuer } - _, err := jwt.ParseWithClaims(token, &claims, jwks.Keyfunc, jwt.WithIssuer(issuer)) + _, err := jwt.ParseWithClaims(token, &claims, jwks.Keyfunc, jwt.WithIssuer(issuer), jwt.WithAudience(c.accessTokenAudiences...)) if err != nil { return claims, mapClaims, err } diff --git a/pkg/oidc/client_test.go b/pkg/oidc/client_test.go index 6c169624ec..19d0a04dd1 100644 --- a/pkg/oidc/client_test.go +++ b/pkg/oidc/client_test.go @@ -167,11 +167,14 @@ func (v logoutVerificationTest) runGetToken(t *testing.T) (*oidc.LogoutToken, er } pm := oidc.ProviderMetadata{} - verifier := oidc.NewOIDCClient( + verifier, err := oidc.NewOIDCClient( oidc.WithOidcIssuer(issuer), oidc.WithJWKS(jwks), oidc.WithProviderMetadata(&pm), ) + if err != nil { + t.Fatal(err) + } return verifier.VerifyLogoutToken(ctx, token) } diff --git a/pkg/oidc/options.go b/pkg/oidc/options.go index bf025e7670..9d27d95657 100644 --- a/pkg/oidc/options.go +++ b/pkg/oidc/options.go @@ -35,6 +35,9 @@ type Options struct { // AccessTokenVerifyMethod to use when verifying access tokens // TODO pass a function or interface to verify? an AccessTokenVerifier? AccessTokenVerifyMethod string + // AccessTokenAudiences requires at least one matching audience in access tokens. + // An empty list disables audience validation. + AccessTokenAudiences []string // Config to use Config *goidc.Config @@ -74,6 +77,14 @@ func WithAccessTokenVerifyMethod(val string) Option { } } +// WithAccessTokenAudiences sets the allowed audiences for access tokens only. +// An empty list disables audience validation. +func WithAccessTokenAudiences(val []string) Option { + return func(o *Options) { + o.AccessTokenAudiences = append([]string(nil), val...) + } +} + // WithHTTPClient provides a function to set the httpClient option. func WithHTTPClient(val *http.Client) Option { return func(o *Options) { diff --git a/services/proxy/README.md b/services/proxy/README.md index 16030f7530..0e7d5b4b72 100644 --- a/services/proxy/README.md +++ b/services/proxy/README.md @@ -13,6 +13,72 @@ The following request authentication schemes are implemented: - Signed URL - Public Share Token +### OIDC Access Token Audiences + +For production deployments, **enable audience validation** so that OpenCloud only +accepts access tokens intended for it. This is especially relevant when the same +identity provider serves several applications: without this check, an otherwise +valid token issued for another application can also be accepted by OpenCloud. + +Set the allowed audiences as a comma-separated environment variable: + +```console +PROXY_OIDC_AUDIENCES=opencloud,opencloud-api +PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt +``` + +Alternatively, configure the list in `proxy.yaml`: + +```yaml +oidc: + audiences: + - opencloud + - opencloud-api + access_token_verify_method: jwt +``` + +These audience values are examples. Configure your identity provider to include +the intended OpenCloud resource audience in the **access tokens** issued to all +relevant clients, including web, desktop and mobile clients. Adding an audience +only to an ID token or a Userinfo response does not satisfy this check. + +The built-in IDP sets the access token's `aud` to the client ID of the +authenticated client. It does not support configuring a separate resource +audience. When using this IDP, list the client IDs of all OpenCloud clients you +use in `PROXY_OIDC_AUDIENCES`, including web, desktop and mobile clients. Setting +this proxy option does not change the tokens issued by the IDP. + +For other IDPs, please refer to their documentation for proper support for the `aud` claim. For Keycloak see e.g.: [Keycloak's audience support documentation](https://www.keycloak.org/docs/latest/server_admin/#audience-support) + +An access token must contain at least one exactly matching, case-sensitive value +in its `aud` claim. Both strings, such as `"aud": "opencloud"`, and arrays, such as +`"aud": ["another-api", "opencloud"]`, are supported. Tokens with missing, empty, +malformed or exclusively nonmatching audiences receive HTTP 401 on protected +routes. Configured audiences require JWT verification; combining a nonempty list +with `access_token_verify_method: none` prevents startup. List entries must not be +empty or consist only of whitespace. + +The default list is empty, which disables audience validation to preserve +compatibility with existing identity provider configurations. An explicitly empty +`PROXY_OIDC_AUDIENCES` overrides any YAML list and disables the check; `audiences: []` +does the same in YAML. When OIDC is active, JWT verification is enabled and the +audience check is disabled, the proxy logs one startup warning, subject to the +configured log level. No audience warning is logged when +`access_token_verify_method` is `none`. + +Restart the proxy after changing the configuration and apply the same policy to +all proxy instances. The signed access token, including its audience when +configured, is verified on a Userinfo cache miss. Cache hits reuse the cached +claims without verifying the token again or requesting Userinfo. Existing entries +in a shared or persistent cache can remain valid under the previous audience +configuration until they expire. Clear the Userinfo cache after updating all +proxy instances if the new policy must take effect immediately. + +The disabled default is a compatibility decision. It does not relax the +[audience validation requirement in RFC 9068, Section 4](https://www.rfc-editor.org/rfc/rfc9068.html#name-validating-jwt-access-token): +a resource server following that JWT access token profile must reject tokens +whose audience does not identify the resource server. + ## Configuring Routes The proxy handles routing to all endpoints that OpenCloud offers. The currently availabe default routes can be found [in the code](https://github.com/opencloud-eu/opencloud/blob/main/services/proxy/pkg/config/defaults/defaultconfig.go). Changing or adding routes can be necessary when writing own OpenCloud extensions. @@ -231,6 +297,9 @@ The default `role_claim` (or `PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM`) is `roles`. The In a production deployment, you want to have basic authentication (`PROXY_ENABLE_BASIC_AUTH`) disabled which is the default state. You also want to setup a firewall to only allow requests to the proxy service or the reverse proxy if you have one. Requests to the other services should be blocked by the firewall. +Configure `PROXY_OIDC_AUDIENCES` as described in [OIDC Access Token Audiences](#oidc-access-token-audiences). +Enabling this check is strongly recommended for production deployments. + ### Content Security Policy For OpenCloud, external resources like an IDP (e.g. Keycloak) or when using web office documents or web apps, require defining a CSP. If not defined, the referenced services will not work. diff --git a/services/proxy/pkg/command/oidc_test.go b/services/proxy/pkg/command/oidc_test.go new file mode 100644 index 0000000000..5af75b99d8 --- /dev/null +++ b/services/proxy/pkg/command/oidc_test.go @@ -0,0 +1,422 @@ +package command + +import ( + "bytes" + "crypto/rand" + "crypto/rsa" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/pkg/oidc" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config/defaults" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/middleware" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/router" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes" + bcl "github.com/opencloud-eu/opencloud/services/proxy/pkg/staticroutes/backchannellogout" + "github.com/rs/zerolog" + "github.com/stretchr/testify/require" + "github.com/vmihailenco/msgpack/v5" + "go-micro.dev/v4/store" + "golang.org/x/crypto/sha3" +) + +func TestOIDCAudienceAuthentication(t *testing.T) { + for _, skipUserInfo := range []bool{false, true} { + t.Run(fmt.Sprintf("skip_user_info=%t", skipUserInfo), func(t *testing.T) { + idp := newAudienceTestIDP(t, "opencloud") + for _, tt := range []struct { + name string + audiences []string + aud any + want int + }{ + {name: "matching string", audiences: []string{"opencloud"}, aud: "opencloud", want: http.StatusOK}, + {name: "matching array", audiences: []string{"opencloud", "opencloud-api"}, aud: []string{"immich", "opencloud-api"}, want: http.StatusOK}, + {name: "foreign despite matching userinfo", audiences: []string{"opencloud"}, aud: "immich", want: http.StatusUnauthorized}, + {name: "missing despite matching userinfo", audiences: []string{"opencloud"}, want: http.StatusUnauthorized}, + {name: "disabled accepts foreign", aud: "immich", want: http.StatusOK}, + {name: "disabled accepts missing", want: http.StatusOK}, + } { + t.Run(tt.name, func(t *testing.T) { + cache := newAudienceTestCache() + cfg := audienceTestConfig(idp, tt.audiences, skipUserInfo) + auth, err := newOIDCAuthenticator(log.NopLogger(), cfg, cache, idp.server.Client()) + require.NoError(t, err) + token := idp.accessToken(t, jwt.MapClaims{"aud": tt.aud}) + before := idp.userinfoRequests.Load() + response := audienceRequest(auth, token) + require.Equal(t, tt.want, response.status) + if tt.want == http.StatusUnauthorized { + require.Nil(t, response.claims, "the protected handler must not run") + require.Equal(t, before, idp.userinfoRequests.Load(), "reject before requesting userinfo") + require.Empty(t, cache.writes, "rejected tokens must not be cached") + return + } + require.Equal(t, "alice", response.claims["sub"]) + require.True(t, response.newSession) + cache.waitForSession(t) + expectedRequests := before + if !skipUserInfo { + expectedRequests++ + } + require.Equal(t, expectedRequests, idp.userinfoRequests.Load()) + response = audienceRequest(auth, token) + require.Equal(t, http.StatusOK, response.status) + require.False(t, response.newSession) + require.Equal(t, expectedRequests, idp.userinfoRequests.Load(), "reuse cached userinfo") + }) + } + }) + } +} + +func TestOIDCAudienceUsesAccessTokenInsteadOfUserinfo(t *testing.T) { + idp := newAudienceTestIDP(t, "different-userinfo-audience") + cache := newAudienceTestCache() + auth, err := newOIDCAuthenticator(log.NopLogger(), audienceTestConfig(idp, []string{"opencloud"}, false), cache, idp.server.Client()) + require.NoError(t, err) + token := idp.accessToken(t, jwt.MapClaims{"aud": "opencloud"}) + require.Equal(t, http.StatusOK, audienceRequest(auth, token).status) + cache.waitForSession(t) + response := audienceRequest(auth, token) + require.Equal(t, http.StatusOK, response.status) + require.Equal(t, "different-userinfo-audience", response.claims["aud"]) + require.EqualValues(t, 1, idp.userinfoRequests.Load()) + require.EqualValues(t, 1, idp.discoveryRequests.Load()) + require.EqualValues(t, 1, idp.jwksRequests.Load()) +} + +func TestOIDCAudienceValidatesTokensOnCacheMiss(t *testing.T) { + idp := newAudienceTestIDP(t, "opencloud") + for _, tt := range []struct { + name string + claims jwt.MapClaims + mangle bool + }{ + {name: "expired", claims: jwt.MapClaims{"exp": time.Now().Add(-time.Hour).Unix()}}, + {name: "not yet valid", claims: jwt.MapClaims{"nbf": time.Now().Add(time.Hour).Unix()}}, + {name: "wrong issuer", claims: jwt.MapClaims{"iss": "https://other.example"}}, + {name: "missing audience", claims: jwt.MapClaims{"aud": nil}}, + {name: "invalid signature", mangle: true}, + } { + t.Run(tt.name, func(t *testing.T) { + cache := newAudienceTestCache() + token := idp.accessToken(t, tt.claims) + if tt.mangle { + parts := strings.Split(token, ".") + sig, err := base64.RawURLEncoding.DecodeString(parts[2]) + require.NoError(t, err) + sig[0] ^= 1 + parts[2] = base64.RawURLEncoding.EncodeToString(sig) + token = strings.Join(parts, ".") + } + auth, err := newOIDCAuthenticator(log.NopLogger(), audienceTestConfig(idp, []string{"opencloud"}, false), cache, idp.server.Client()) + require.NoError(t, err) + require.Equal(t, http.StatusUnauthorized, audienceRequest(auth, token).status) + require.Zero(t, idp.userinfoRequests.Load()) + require.Empty(t, cache.writes, "rejected tokens must not be cached") + }) + } +} + +func TestOIDCAudienceRefreshesExpiredOrCorruptCachedClaims(t *testing.T) { + for _, skipUserInfo := range []bool{false, true} { + for _, corrupt := range []bool{false, true} { + t.Run(fmt.Sprintf("skip_user_info=%t/corrupt=%t", skipUserInfo, corrupt), func(t *testing.T) { + idp := newAudienceTestIDP(t, "opencloud") + cache := newAudienceTestCache() + token := idp.accessToken(t, nil) + cached, err := msgpack.Marshal(map[string]any{"sub": "stale", "exp": time.Now().Add(-time.Hour).Unix()}) + require.NoError(t, err) + if corrupt { + cached = []byte{0xc1} // Reserved/invalid MessagePack marker. + } + require.NoError(t, cache.Store.Write(&store.Record{Key: audienceTokenCacheKey(token), Value: cached, Expiry: time.Hour})) + auth, err := newOIDCAuthenticator(log.NopLogger(), audienceTestConfig(idp, []string{"opencloud"}, skipUserInfo), cache, idp.server.Client()) + require.NoError(t, err) + response := audienceRequest(auth, token) + require.Equal(t, http.StatusOK, response.status) + require.Equal(t, "alice", response.claims["sub"]) + require.True(t, response.newSession) + cache.waitForSession(t) + require.False(t, audienceRequest(auth, token).newSession) + }) + } + } +} + +func TestOIDCAudiencePreservesBackchannelLogout(t *testing.T) { + for _, skipUserInfo := range []bool{false, true} { + t.Run(fmt.Sprintf("skip_user_info=%t", skipUserInfo), func(t *testing.T) { + idp := newAudienceTestIDP(t, "opencloud") + cache := newAudienceTestCache() + cfg := audienceTestConfig(idp, []string{"opencloud"}, skipUserInfo) + auth, err := newOIDCAuthenticator(log.NopLogger(), cfg, cache, idp.server.Client()) + require.NoError(t, err) + token := idp.accessToken(t, nil) + require.Equal(t, http.StatusOK, audienceRequest(auth, token).status) + cache.waitForSession(t) + + sessionKey, err := bcl.NewKey("alice", "session") + require.NoError(t, err) + records, err := cache.Read(sessionKey) + require.NoError(t, err) + require.Len(t, records, 1) + require.Equal(t, audienceTokenCacheKey(token), string(records[0].Value)) + + logoutClient, err := oidc.NewOIDCClient( + oidc.WithLogger(log.NopLogger()), + oidc.WithOidcIssuer(idp.server.URL), + oidc.WithHTTPClient(idp.server.Client()), + oidc.WithAccessTokenVerifyMethod(config.AccessTokenVerificationJWT), + oidc.WithAccessTokenAudiences([]string{"opencloud"}), + ) + require.NoError(t, err) + routes := &staticroutes.StaticRouteHandler{ + Prefix: "/", Config: *cfg, Logger: log.NopLogger(), OidcClient: logoutClient, + UserInfoCache: cache, Proxy: http.NotFoundHandler(), + } + // Subject logout invalidates all sessions, without requiring a user/event backend. + logoutToken := idp.sign(t, jwt.MapClaims{ + "iss": idp.server.URL, "sub": "alice", "aud": "web-client", + "events": map[string]any{"http://schemas.openid.net/event/backchannel-logout": map[string]any{}}, + }) + form := url.Values{"logout_token": {logoutToken}} + req := httptest.NewRequest(http.MethodPost, "/backchannel_logout", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + response := httptest.NewRecorder() + routes.Handler().ServeHTTP(response, req) + require.Equal(t, http.StatusOK, response.Code, response.Body.String()) + _, err = cache.Read(sessionKey) + require.ErrorIs(t, err, store.ErrNotFound) + _, err = cache.Read(audienceTokenCacheKey(token)) + require.ErrorIs(t, err, store.ErrNotFound) + }) + } +} + +func TestOIDCAuthenticatorRejectsInvalidAudienceConfiguration(t *testing.T) { + for _, tt := range []struct { + name string + method string + audiences []string + wantErr string + }{ + {name: "verification disabled", method: config.AccessTokenVerificationNone, audiences: []string{"opencloud"}, wantErr: "requires the jwt verification method"}, + {name: "blank audience", method: config.AccessTokenVerificationJWT, audiences: []string{"opencloud", " \t"}, wantErr: "empty or whitespace-only"}, + } { + t.Run(tt.name, func(t *testing.T) { + cfg := defaults.FullDefaultConfig() + cfg.OIDC.AccessTokenVerifyMethod = tt.method + cfg.OIDC.Audiences = tt.audiences + // Invalid configuration must fail during setup, before any HTTP request. + auth, err := newOIDCAuthenticator(log.NopLogger(), cfg, newAudienceTestCache(), nil) + require.ErrorContains(t, err, tt.wantErr) + require.Nil(t, auth) + }) + } +} + +func TestOIDCAudienceStartupWarning(t *testing.T) { + // Match log.NewLogger's global level while testing the per-service filter. + previousLevel := zerolog.GlobalLevel() + zerolog.SetGlobalLevel(zerolog.TraceLevel) + t.Cleanup(func() { zerolog.SetGlobalLevel(previousLevel) }) + idp := newAudienceTestIDP(t, "opencloud") + for _, tt := range []struct { + name string + audiences []string + level zerolog.Level + inactive bool + verifyNone bool + want int + }{ + {name: "disabled", level: zerolog.WarnLevel, want: 1}, + {name: "enabled", audiences: []string{"opencloud"}, level: zerolog.WarnLevel}, + {name: "filtered", level: zerolog.ErrorLevel}, + {name: "OIDC inactive", inactive: true, level: zerolog.WarnLevel}, + {name: "verification disabled", verifyNone: true, level: zerolog.WarnLevel}, + } { + t.Run(tt.name, func(t *testing.T) { + var output bytes.Buffer + logger := log.Logger{Logger: zerolog.New(&output).Level(tt.level)} + cfg := audienceTestConfig(idp, tt.audiences, true) + if tt.verifyNone { + cfg.OIDC.AccessTokenVerifyMethod = config.AccessTokenVerificationNone + cfg.OIDC.SkipUserInfo = false + } + if tt.inactive { + cfg.OIDC.Issuer = "" + } + cache := newAudienceTestCache() + auth, err := newOIDCAuthenticator(logger, cfg, cache, idp.server.Client()) + require.NoError(t, err) + if !tt.inactive { + token := idp.accessToken(t, nil) + require.Equal(t, http.StatusOK, audienceRequest(auth, token).status) + if !tt.verifyNone { + cache.waitForSession(t) + } + for range 3 { + require.Equal(t, http.StatusOK, audienceRequest(auth, token).status) + } + } + require.Equal(t, tt.want, strings.Count(output.String(), "PROXY_OIDC_AUDIENCES")) + if tt.want == 1 { + require.Contains(t, output.String(), "\"level\":\"warn\"") + } else { + require.Empty(t, output.String()) + } + }) + } +} + +type audienceTestIDP struct { + server *httptest.Server + key *rsa.PrivateKey + discoveryRequests atomic.Int32 + jwksRequests atomic.Int32 + userinfoRequests atomic.Int32 +} + +func newAudienceTestIDP(t *testing.T, userinfoAudience string) *audienceTestIDP { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + idp := &audienceTestIDP{key: key} + mux := http.NewServeMux() + idp.server = httptest.NewServer(mux) + t.Cleanup(idp.server.Close) + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + idp.discoveryRequests.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "issuer": idp.server.URL, "jwks_uri": idp.server.URL + "/jwks", + "userinfo_endpoint": idp.server.URL + "/userinfo", + "id_token_signing_alg_values_supported": []string{"RS256"}, + }) + }) + mux.HandleFunc("/jwks", func(w http.ResponseWriter, r *http.Request) { + idp.jwksRequests.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"keys": []any{map[string]any{ + "kty": "RSA", "kid": "test", "alg": "RS256", "use": "sig", + "n": base64.RawURLEncoding.EncodeToString(key.N.Bytes()), "e": "AQAB", + }}}) + }) + mux.HandleFunc("/userinfo", func(w http.ResponseWriter, r *http.Request) { + idp.userinfoRequests.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"sub": "alice", "preferred_username": "alice", "aud": userinfoAudience}) + }) + return idp +} + +func (idp *audienceTestIDP) sign(t *testing.T, claims jwt.MapClaims) string { + t.Helper() + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + token.Header["kid"] = "test" + signed, err := token.SignedString(idp.key) + require.NoError(t, err) + return signed +} + +func (idp *audienceTestIDP) accessToken(t *testing.T, overrides jwt.MapClaims) string { + t.Helper() + claims := jwt.MapClaims{ + "iss": idp.server.URL, "sub": "alice", "sid": "session", "aud": "opencloud", + "exp": time.Now().Add(time.Hour).Unix(), + } + for key, value := range overrides { + if value == nil { + delete(claims, key) + } else { + claims[key] = value + } + } + return idp.sign(t, claims) +} + +func audienceTestConfig(idp *audienceTestIDP, audiences []string, skipUserInfo bool) *config.Config { + cfg := defaults.FullDefaultConfig() + cfg.OIDC.Issuer = idp.server.URL + cfg.OIDC.Audiences = audiences + cfg.OIDC.SkipUserInfo = skipUserInfo + cfg.OIDC.JWKS = config.JWKS{} // No background refresh goroutines in tests. + return cfg +} + +type audienceTestResponse struct { + status int + newSession bool + claims map[string]any +} + +func audienceRequest(auth middleware.Authenticator, token string) audienceTestResponse { + result := audienceTestResponse{} + handler := middleware.Authentication([]middleware.Authenticator{auth})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + result.newSession = oidc.NewSessionFlagFromContext(r.Context()) + result.claims = oidc.FromContext(r.Context()) + w.WriteHeader(http.StatusOK) + })) + req := httptest.NewRequest(http.MethodGet, "/protected", http.NoBody) + req = req.WithContext(router.SetRoutingInfo(req.Context(), router.RoutingInfo{})) + req.Header.Set("Authorization", "Bearer "+token) + recorder := httptest.NewRecorder() + handler.ServeHTTP(recorder, req) + result.status = recorder.Code + return result +} + +// Wait for the asynchronous session write instead of sleeping or racing the cache. +type audienceTestCache struct { + store.Store + writes chan string +} + +func newAudienceTestCache() *audienceTestCache { + return &audienceTestCache{Store: store.NewMemoryStore(), writes: make(chan string, 16)} +} + +func (cache *audienceTestCache) Write(record *store.Record, opts ...store.WriteOption) error { + err := cache.Store.Write(record, opts...) + if err == nil { + cache.writes <- record.Key + } + return err +} + +func (cache *audienceTestCache) waitForSession(t *testing.T) { + t.Helper() + key, err := bcl.NewKey("alice", "session") + require.NoError(t, err) + timer := time.NewTimer(5 * time.Second) + defer timer.Stop() + for { + select { + case written := <-cache.writes: + if written == key { + return + } + case <-timer.C: + t.Fatal("timed out waiting for session cache write") + } + } +} + +func audienceTokenCacheKey(token string) string { + hash := make([]byte, 64) + sha3.ShakeSum256(hash, []byte(token)) + return base64.URLEncoding.EncodeToString(hash) +} diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go index 12adf8c6ce..a128ec1247 100644 --- a/services/proxy/pkg/command/server.go +++ b/services/proxy/pkg/command/server.go @@ -104,18 +104,21 @@ func Server(cfg *config.Config) *cobra.Command { InsecureSkipVerify: cfg.OIDC.Insecure, //nolint:gosec }, DisableKeepAlives: true, - Proxy: http.ProxyFromEnvironment, + Proxy: http.ProxyFromEnvironment, }, Timeout: time.Second * 10, } - oidcClient := oidc.NewOIDCClient( + oidcClient, err := oidc.NewOIDCClient( oidc.WithAccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod), oidc.WithLogger(logger), oidc.WithHTTPClient(oidcHTTPClient), oidc.WithOidcIssuer(cfg.OIDC.Issuer), oidc.WithJWKSOptions(cfg.OIDC.JWKS), ) + if err != nil { + return fmt.Errorf("failed to initialize OIDC client: %w", err) + } var cancel context.CancelFunc if cfg.Context == nil { @@ -195,7 +198,10 @@ func Server(cfg *config.Config) *cobra.Command { gr := runner.NewGroup() { - middlewares := loadMiddlewares(logger, cfg, userInfoCache, signingKeyStore, traceProvider, *m, userProvider, publisher, gatewaySelector, serviceSelector) + middlewares, err := loadMiddlewares(logger, cfg, userInfoCache, signingKeyStore, traceProvider, *m, userProvider, publisher, gatewaySelector, serviceSelector) + if err != nil { + return err + } server, err := proxyHTTP.Server( proxyHTTP.Handler(lh.Handler()), @@ -244,11 +250,40 @@ func Server(cfg *config.Config) *cobra.Command { } } +func newOIDCAuthenticator(logger log.Logger, cfg *config.Config, userInfoCache microstore.Store, httpClient *http.Client) (*middleware.OIDCAuthenticator, error) { + oidcClient, err := oidc.NewOIDCClient( + oidc.WithAccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod), + oidc.WithAccessTokenAudiences(cfg.OIDC.Audiences), + oidc.WithLogger(logger), + oidc.WithHTTPClient(httpClient), + oidc.WithOidcIssuer(cfg.OIDC.Issuer), + oidc.WithJWKSOptions(cfg.OIDC.JWKS), + ) + if err != nil { + return nil, fmt.Errorf("failed to initialize OIDC authenticator: %w", err) + } + + if cfg.OIDC.Issuer != "" && cfg.OIDC.AccessTokenVerifyMethod != config.AccessTokenVerificationNone && len(cfg.OIDC.Audiences) == 0 { + logger.Warn().Msg("OIDC access token audience validation is disabled. Configure PROXY_OIDC_AUDIENCES to enable it; this is recommended for production.") + } + + return middleware.NewOIDCAuthenticator( + middleware.Logger(logger), + middleware.UserInfoCache(userInfoCache), + middleware.DefaultAccessTokenTTL(cfg.OIDC.UserinfoCache.TTL), + middleware.HTTPClient(httpClient), + middleware.OIDCIss(cfg.OIDC.Issuer), + middleware.AccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod), + middleware.OIDCClient(oidcClient), + middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo), + ), nil +} + func loadMiddlewares(logger log.Logger, cfg *config.Config, userInfoCache, signingKeyStore microstore.Store, traceProvider trace.TracerProvider, metrics metrics.Metrics, userProvider backend.UserBackend, publisher events.Publisher, - gatewaySelector pool.Selectable[gateway.GatewayAPIClient], serviceSelector selector.Selector) alice.Chain { + gatewaySelector pool.Selectable[gateway.GatewayAPIClient], serviceSelector selector.Selector) (alice.Chain, error) { rolesClient := settingssvc.NewRoleService("eu.opencloud.api.settings", cfg.GrpcClient) policiesProviderClient := policiessvc.NewPoliciesProviderService("eu.opencloud.api.policies", cfg.GrpcClient) @@ -280,7 +315,7 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, InsecureSkipVerify: cfg.OIDC.Insecure, //nolint:gosec }, DisableKeepAlives: true, - Proxy: http.ProxyFromEnvironment, + Proxy: http.ProxyFromEnvironment, }, Timeout: time.Second * 10, } @@ -301,22 +336,11 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, UserRoleAssigner: roleAssigner, }) } - authenticators = append(authenticators, middleware.NewOIDCAuthenticator( - middleware.Logger(logger), - middleware.UserInfoCache(userInfoCache), - middleware.DefaultAccessTokenTTL(cfg.OIDC.UserinfoCache.TTL), - middleware.HTTPClient(oidcHTTPClient), - middleware.OIDCIss(cfg.OIDC.Issuer), - middleware.AccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod), - middleware.OIDCClient(oidc.NewOIDCClient( - oidc.WithAccessTokenVerifyMethod(cfg.OIDC.AccessTokenVerifyMethod), - oidc.WithLogger(logger), - oidc.WithHTTPClient(oidcHTTPClient), - oidc.WithOidcIssuer(cfg.OIDC.Issuer), - oidc.WithJWKSOptions(cfg.OIDC.JWKS), - )), - middleware.SkipUserInfo(cfg.OIDC.SkipUserInfo), - )) + oidcAuthenticator, err := newOIDCAuthenticator(logger, cfg, userInfoCache, oidcHTTPClient) + if err != nil { + return alice.Chain{}, err + } + authenticators = append(authenticators, oidcAuthenticator) authenticators = append(authenticators, middleware.PublicShareAuthenticator{ Logger: logger, RevaGatewaySelector: gatewaySelector, @@ -411,5 +435,5 @@ func loadMiddlewares(logger log.Logger, cfg *config.Config, middleware.WithRevaGatewaySelector(gatewaySelector), middleware.RoleQuotas(cfg.RoleQuotas), ), - ) + ), nil } diff --git a/services/proxy/pkg/config/config.go b/services/proxy/pkg/config/config.go index 05344049a6..2c952a0a46 100644 --- a/services/proxy/pkg/config/config.go +++ b/services/proxy/pkg/config/config.go @@ -116,13 +116,14 @@ const ( // OIDC is the config for the OpenID-Connect middleware. If set the proxy will try to authenticate every request // with the configured oidc-provider type OIDC struct { - Issuer string `yaml:"issuer" env:"OC_URL;OC_OIDC_ISSUER;PROXY_OIDC_ISSUER" desc:"URL of the OIDC issuer. It defaults to URL of the builtin IDP." introductionVersion:"1.0.0"` - Insecure bool `yaml:"insecure" env:"OC_INSECURE;PROXY_OIDC_INSECURE" desc:"Disable TLS certificate validation for connections to the IDP. Note that this is not recommended for production environments." introductionVersion:"1.0.0"` - 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 IDP'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'." introductionVersion:"1.0.0"` - SkipUserInfo bool `yaml:"skip_user_info" env:"PROXY_OIDC_SKIP_USER_INFO" desc:"Do not look up user claims at the userinfo endpoint and directly read them from the access token. Incompatible with 'PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=none'." introductionVersion:"1.0.0"` - UserinfoCache *Cache `yaml:"user_info_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." introductionVersion:"1.0.0"` + Audiences []string `yaml:"audiences" env:"PROXY_OIDC_AUDIENCES" desc:"Optional comma-separated list of allowed audiences for OIDC access tokens. Empty disables audience validation for compatibility. Configuring audiences is recommended for production and requires PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=jwt. Tokens must contain at least one exactly matching, case-sensitive audience in their aud claim." introductionVersion:"%%NEXT%%"` + Issuer string `yaml:"issuer" env:"OC_URL;OC_OIDC_ISSUER;PROXY_OIDC_ISSUER" desc:"URL of the OIDC issuer. It defaults to URL of the builtin IDP." introductionVersion:"1.0.0"` + Insecure bool `yaml:"insecure" env:"OC_INSECURE;PROXY_OIDC_INSECURE" desc:"Disable TLS certificate validation for connections to the IDP. Note that this is not recommended for production environments." introductionVersion:"1.0.0"` + 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 IDP'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'." introductionVersion:"1.0.0"` + SkipUserInfo bool `yaml:"skip_user_info" env:"PROXY_OIDC_SKIP_USER_INFO" desc:"Do not look up user claims at the userinfo endpoint and directly read them from the access token. Incompatible with 'PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD=none'." introductionVersion:"1.0.0"` + UserinfoCache *Cache `yaml:"user_info_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." introductionVersion:"1.0.0"` } type JWKS struct { diff --git a/services/proxy/pkg/config/parser/parse.go b/services/proxy/pkg/config/parser/parse.go index 7a8be8c7a8..9cae495b6d 100644 --- a/services/proxy/pkg/config/parser/parse.go +++ b/services/proxy/pkg/config/parser/parse.go @@ -3,6 +3,7 @@ package parser import ( "errors" "fmt" + "strings" occfg "github.com/opencloud-eu/opencloud/pkg/config" "github.com/opencloud-eu/opencloud/pkg/shared" @@ -56,6 +57,14 @@ func Validate(cfg *config.Config) error { cfg.OIDC.SkipUserInfo, cfg.Service.Name, ) } + if len(cfg.OIDC.Audiences) > 0 && cfg.OIDC.AccessTokenVerifyMethod != config.AccessTokenVerificationJWT { + return fmt.Errorf("OIDC audiences (PROXY_OIDC_AUDIENCES) in service %s require access_token_verify_method to be 'jwt'", cfg.Service.Name) + } + for _, audience := range cfg.OIDC.Audiences { + if strings.TrimSpace(audience) == "" { + return fmt.Errorf("OIDC audiences (PROXY_OIDC_AUDIENCES) in service %s must not contain empty or whitespace-only entries", cfg.Service.Name) + } + } if cfg.ServiceAccount.ServiceAccountID == "" { return shared.MissingServiceAccountID(cfg.Service.Name) diff --git a/services/proxy/pkg/config/parser/parse_test.go b/services/proxy/pkg/config/parser/parse_test.go new file mode 100644 index 0000000000..b955ac6c6d --- /dev/null +++ b/services/proxy/pkg/config/parser/parse_test.go @@ -0,0 +1,115 @@ +package parser_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/opencloud-eu/opencloud/pkg/shared" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config/defaults" + "github.com/opencloud-eu/opencloud/services/proxy/pkg/config/parser" + "github.com/stretchr/testify/require" +) + +func TestParseOIDCAudiences(t *testing.T) { + tests := []struct { + name string + yaml string + env string + method string + setEnv bool + want []string + wantErr string + }{ + {name: "unset defaults to disabled"}, + {name: "YAML", yaml: "oidc:\n audiences: [opencloud, opencloud-api]\n", want: []string{"opencloud", "opencloud-api"}}, + {name: "empty YAML list", yaml: "oidc:\n audiences: []\n"}, + {name: "null YAML list", yaml: "oidc:\n audiences: null\n"}, + {name: "ENV", setEnv: true, env: "opencloud,opencloud-api", want: []string{"opencloud", "opencloud-api"}}, + {name: "ENV trims list entries", setEnv: true, env: " opencloud, opencloud-api ", want: []string{"opencloud", "opencloud-api"}}, + {name: "ENV precedence", yaml: "oidc:\n audiences: [yaml-audience]\n", setEnv: true, env: "env-audience", want: []string{"env-audience"}}, + {name: "empty ENV disables YAML", yaml: "oidc:\n audiences: [opencloud]\n", setEnv: true}, + {name: "existing ENV empty segment handling", setEnv: true, env: "opencloud,,opencloud-api", want: []string{"opencloud", "opencloud-api"}}, + {name: "YAML blank entry", yaml: "oidc:\n audiences: ['']\n", wantErr: "empty or whitespace-only"}, + {name: "YAML whitespace entry", yaml: "oidc:\n audiences: [' ']\n", wantErr: "empty or whitespace-only"}, + {name: "ENV whitespace entry", setEnv: true, env: "opencloud, ", wantErr: "empty or whitespace-only"}, + {name: "ENV whitespace only", setEnv: true, env: " ", wantErr: "empty or whitespace-only"}, + {name: "YAML preserves case", yaml: "oidc:\n audiences: [OpenCloud]\n", want: []string{"OpenCloud"}}, + {name: "YAML incompatible verification", yaml: "oidc:\n audiences: [opencloud]\n access_token_verify_method: none\n", wantErr: "require access_token_verify_method to be 'jwt'"}, + {name: "ENV incompatible verification", setEnv: true, env: "opencloud", method: "none", wantErr: "require access_token_verify_method to be 'jwt'"}, + {name: "ENV enables JWT over YAML none", yaml: "oidc:\n audiences: [opencloud]\n access_token_verify_method: none\n", method: "jwt", want: []string{"opencloud"}}, + {name: "empty ENV restores none compatibility", yaml: "oidc:\n audiences: [opencloud]\n access_token_verify_method: none\n", setEnv: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + t.Setenv("OC_CONFIG_DIR", dir) + t.Setenv("PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD", "") + require.NoError(t, os.Unsetenv("PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD")) + if tt.method != "" { + t.Setenv("PROXY_OIDC_ACCESS_TOKEN_VERIFY_METHOD", tt.method) + } + t.Setenv("PROXY_OIDC_SKIP_USER_INFO", "false") + t.Setenv("PROXY_OIDC_AUDIENCES", "") + require.NoError(t, os.Unsetenv("PROXY_OIDC_AUDIENCES")) + if tt.setEnv { + t.Setenv("PROXY_OIDC_AUDIENCES", tt.env) + } + require.NoError(t, os.WriteFile(filepath.Join(dir, "proxy.yaml"), []byte(tt.yaml), 0600)) + cfg := validProxyConfig() + err := parser.ParseConfig(cfg) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + require.ErrorContains(t, err, "PROXY_OIDC_AUDIENCES") + return + } + require.NoError(t, err) + if len(tt.want) == 0 { + require.Empty(t, cfg.OIDC.Audiences) + } else { + require.Equal(t, tt.want, cfg.OIDC.Audiences) + } + }) + } +} + +func TestValidateOIDCAudiences(t *testing.T) { + for _, tt := range []struct { + name string + audiences []string + method string + wantErr string + }{ + {name: "disabled JWT", method: "jwt"}, + {name: "disabled none", method: "none"}, + {name: "enabled JWT", audiences: []string{"opencloud"}, method: "jwt"}, + {name: "enabled none", audiences: []string{"opencloud"}, method: "none", wantErr: "require access_token_verify_method to be 'jwt'"}, + {name: "blank", audiences: []string{""}, method: "jwt", wantErr: "empty or whitespace-only"}, + {name: "whitespace", audiences: []string{" \t"}, method: "jwt", wantErr: "empty or whitespace-only"}, + {name: "mixed valid and blank", audiences: []string{"opencloud", ""}, method: "jwt", wantErr: "empty or whitespace-only"}, + } { + t.Run(tt.name, func(t *testing.T) { + cfg := validProxyConfig() + cfg.OIDC.Audiences = tt.audiences + cfg.OIDC.AccessTokenVerifyMethod = tt.method + err := parser.Validate(cfg) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + require.ErrorContains(t, err, "PROXY_OIDC_AUDIENCES") + } else { + require.NoError(t, err) + } + }) + } +} + +func validProxyConfig() *config.Config { + cfg := defaults.FullDefaultConfig() + cfg.MachineAuthAPIKey = "test-machine-key" + cfg.TransferSecret = "test-transfer-secret" + cfg.ServiceAccount.ServiceAccountID = "test-service-account" + cfg.ServiceAccount.ServiceAccountSecret = "test-service-secret" + cfg.Commons = &shared.Commons{URLSigningSecret: "test-url-secret"} + return cfg +} diff --git a/services/proxy/pkg/middleware/oidc_cache_test.go b/services/proxy/pkg/middleware/oidc_cache_test.go new file mode 100644 index 0000000000..1aa2d869c1 --- /dev/null +++ b/services/proxy/pkg/middleware/oidc_cache_test.go @@ -0,0 +1,62 @@ +package middleware + +import ( + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/vmihailenco/msgpack/v5" + "go-micro.dev/v4/store" + "golang.org/x/crypto/sha3" + + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/pkg/oidc" + oidcmocks "github.com/opencloud-eu/opencloud/pkg/oidc/mocks" +) + +func TestOIDCCacheTokenVerification(t *testing.T) { + for _, cached := range []bool{false, true} { + t.Run(fmt.Sprintf("cached=%t", cached), func(t *testing.T) { + client := &oidcmocks.OIDCClient{} + expiresAt := time.Now().Add(time.Hour) + claims := jwt.MapClaims{"sub": "alice", "exp": expiresAt.Unix()} + if !cached { + client.On("VerifyAccessToken", mock.Anything, "token").Return(oidc.RegClaimsWithSID{ + SessionID: "session", + RegisteredClaims: jwt.RegisteredClaims{ + Subject: "alice", ExpiresAt: jwt.NewNumericDate(expiresAt), + }, + }, claims, nil).Once() + } + cache := store.NewMemoryStore() + if cached { + hash := make([]byte, 64) + sha3.ShakeSum256(hash, []byte("token")) + data, err := msgpack.Marshal(claims) + require.NoError(t, err) + require.NoError(t, cache.Write(&store.Record{ + Key: base64.URLEncoding.EncodeToString(hash), Value: data, + })) + } + authenticator := NewOIDCAuthenticator( + Logger(log.NopLogger()), OIDCClient(client), UserInfoCache(cache), + SkipUserInfo(true), + ) + got, newSession, err := authenticator.getClaims("token", httptest.NewRequest(http.MethodGet, "/", http.NoBody)) + require.NoError(t, err) + require.Equal(t, "alice", got["sub"]) + require.Equal(t, !cached, newSession) + client.AssertExpectations(t) + if cached { + client.AssertNotCalled(t, "VerifyAccessToken", mock.Anything, mock.Anything) + } + client.AssertNotCalled(t, "UserInfo", mock.Anything, mock.Anything) + }) + } +}