diff --git a/api/v1beta3/auth_config_types.go b/api/v1beta3/auth_config_types.go index c2499cd6..46a10679 100644 --- a/api/v1beta3/auth_config_types.go +++ b/api/v1beta3/auth_config_types.go @@ -384,6 +384,13 @@ type JwtAuthenticationSpec struct { // +optional IssuerUrl string `json:"issuerUrl,omitempty"` + // The expected value of the "iss" (issuer) claim of the JWT. + // If set, Authorino rejects, at the authentication phase, any token whose "iss" claim does not equal this value. + // If omitted, the issuer claim is not verified — it can still be checked via an authorization rule (CEL, pattern-matching or OPA). + // Applies to both issuerUrl and jwksUrl. + // +optional + Issuer string `json:"issuer,omitempty"` + // Decides how long the OIDC configuration will be cached. // If omitted or set to zero, Authorino will never refresh the OIDC configuration. // This configuration does not affect the caching of JSON Web Keys (JWK), which is always updated whenever the kid of a JWT does not match any of the cached JWKs (https://openid.net/specs/openid-connect-core-1_0.html#RotateSigKeys) diff --git a/controllers/auth_config_controller.go b/controllers/auth_config_controller.go index 888f9ebf..5c3c449a 100644 --- a/controllers/auth_config_controller.go +++ b/controllers/auth_config_controller.go @@ -381,12 +381,15 @@ func (r *AuthConfigReconciler) translateAuthConfig(ctx context.Context, authConf case api.JwtAuthentication: var jwtVerifier identity_evaluators.JWTVerifier if identity.Jwt.IssuerUrl != "" { - jwtVerifier = identity_evaluators.NewOIDCProviderVerifier(ctx, identity.Jwt.IssuerUrl, identity.Jwt.TTL, identity.Jwt.Timeout) + jwtVerifier = identity_evaluators.NewOIDCProviderVerifier(ctx, identity.Jwt.IssuerUrl, identity.Jwt.Issuer, identity.Jwt.TTL, identity.Jwt.Timeout) } else if identity.Jwt.JwksUrl != "" { - jwtVerifier = identity_evaluators.NewJwksVerifier(ctx, identity.Jwt.JwksUrl, identity.Jwt.Timeout) + jwtVerifier = identity_evaluators.NewJwksVerifier(ctx, identity.Jwt.JwksUrl, identity.Jwt.Issuer, identity.Jwt.Timeout) } else { return nil, fmt.Errorf("missing issuerUrl or jwksUrl for JWT authentication method") // should never happen if properly validated at the API level } + if identity.Jwt.Issuer == "" { + log.FromContext(ctxWithLogger).Info("JWT authentication does not verify the token issuer (iss) claim; set issuer to enforce it, or verify it via an authorization rule", "authentication", identityCfgName) + } translatedIdentity.JWTAuthentication = identity_evaluators.NewJWTAuthentication(jwtVerifier, authCred) // apiKey diff --git a/controllers/auth_config_controller_test.go b/controllers/auth_config_controller_test.go index eff15584..c0c445f2 100644 --- a/controllers/auth_config_controller_test.go +++ b/controllers/auth_config_controller_test.go @@ -4,8 +4,10 @@ import ( "context" "fmt" "os" + "strings" "testing" + "github.com/go-logr/logr/funcr" api "github.com/kuadrant/authorino/api/v1beta3" "github.com/kuadrant/authorino/pkg/evaluators" "github.com/kuadrant/authorino/pkg/httptest" @@ -382,6 +384,55 @@ func TestEmptyAuthConfigIdentitiesDefaultsToAnonymousAccess(t *testing.T) { assert.Equal(t, len(config.IdentityConfigs), 1) } +func translateJwtAuthConfig(t *testing.T, issuer string) []string { + t.Helper() + + var logs []string + recorder := funcr.New(func(prefix, args string) { + logs = append(logs, args) + }, funcr.Options{}) + + r := &AuthConfigReconciler{} + _, err := r.translateAuthConfig(log.IntoContext(context.TODO(), recorder), &api.AuthConfig{ + Spec: api.AuthConfigSpec{ + Hosts: []string{"app.com"}, + Authentication: map[string]api.AuthenticationSpec{ + "keycloak": { + AuthenticationMethodSpec: api.AuthenticationMethodSpec{ + Jwt: &api.JwtAuthenticationSpec{ + IssuerUrl: "http://127.0.0.1:9001/auth/realms/demo", + Issuer: issuer, + }, + }, + }, + }, + }, + }) + assert.NilError(t, err) + return logs +} + +func logsContain(logs []string, substr string) bool { + for _, l := range logs { + if strings.Contains(l, substr) { + return true + } + } + return false +} + +const unsafeIssuerDefaultLogMsg = "does not verify the token issuer (iss) claim" + +func TestJwtIssuerUnsetLogsWarning(t *testing.T) { + logs := translateJwtAuthConfig(t, "") + assert.Check(t, logsContain(logs, unsafeIssuerDefaultLogMsg), "expected an INFO log warning about the unverified issuer default") +} + +func TestJwtIssuerSetDoesNotLogWarning(t *testing.T) { + logs := translateJwtAuthConfig(t, "http://127.0.0.1:9001/auth/realms/demo") + assert.Check(t, !logsContain(logs, unsafeIssuerDefaultLogMsg), "did not expect the issuer warning when issuer is set") +} + func TestBootstrapIndex(t *testing.T) { mockController := gomock.NewController(t) defer mockController.Finish() diff --git a/docs/features.md b/docs/features.md index d99c185e..e9d8d258 100644 --- a/docs/features.md +++ b/docs/features.md @@ -198,6 +198,8 @@ The `kid` claim stated in the JWT header must match one of the keys cached by Au After veryfing and validating a JWT, the decoded payload of the JWT is appended to the authorization JSON as the resolved identity object. +By default, Authorino does **not** verify the `iss` (issuer) claim of the JWT; the issuer can be enforced with an authorization rule (CEL, pattern-matching or OPA) using the resolved identity. Set `authentication.jwt.issuer` to the expected issuer to have Authorino reject, already at the authentication phase, any token whose `iss` claim does not equal that value. This is recommended whenever distinct issuers may share signing keys (e.g. multi-tenant identity providers, or Festival Wristbands issued by more than one `AuthConfig` backed by the same signing-key Secret). The `issuer` field applies to both `issuerUrl` and `jwksUrl`, and it usually matches `issuerUrl` — but it may differ when the OpenID Connect discovery endpoint is reached at a different URL than the issuer stamped into the tokens (e.g. cluster-internal discovery vs external issuer). + _Important!_ Authorino does **not** implement [OAuth2 grants](https://datatracker.ietf.org/doc/html/rfc6749#section-4) nor [OIDC authentication flows](https://openid.net/specs/openid-connect-core-1_0.html#Authentication). As a common recommendation of good practice, obtaining and refreshing access tokens is for clients to negotiate directly with the auth servers and token issuers. Authorino will only validate those tokens using the parameters provided by the trusted issuer authorities. For an excellent summary of the underlying concepts and standards that relate OpenID Connect and JSON Object Signing and Encryption (JOSE), see this [article](https://access.redhat.com/blogs/766093/posts/1976593) by Jan Rusnacko. For official specification and RFCs, see [OpenID Connect Core](https://openid.net/specs/openid-connect-core-1_0.html), [OpenID Connect Discovery](https://openid.net/specs/openid-connect-discovery-1_0.html), [JSON Web Token (JWT) (RFC7519)](https://datatracker.ietf.org/doc/html/rfc7519), and [JSON Object Signing and Encryption (JOSE)](http://www.iana.org/assignments/jose/jose.xhtml). diff --git a/install/crd/authorino.kuadrant.io_authconfigs.yaml b/install/crd/authorino.kuadrant.io_authconfigs.yaml index b0b449f1..589865c3 100644 --- a/install/crd/authorino.kuadrant.io_authconfigs.yaml +++ b/install/crd/authorino.kuadrant.io_authconfigs.yaml @@ -243,6 +243,14 @@ spec: jwt: description: Authentication based on JWT tokens. properties: + issuer: + description: |- + The expected value of the "iss" (issuer) claim of the JWT. + If set, Authorino rejects, at the authentication phase, any token whose "iss" claim does not equal this value. + If omitted, the issuer claim is not verified — it can still be checked via an authorization rule (CEL, pattern-matching or OPA). + Applies to both issuerUrl and jwksUrl. It usually matches issuerUrl, but may differ when the OpenID Connect discovery + endpoint is reached at a different URL than the issuer stamped into the tokens (e.g. cluster-internal discovery vs external issuer). + type: string issuerUrl: description: |- URL of the OpenID Connect (OIDC) token issuer endpoint. diff --git a/install/manifests.yaml b/install/manifests.yaml index 07722f27..e52d8af3 100644 --- a/install/manifests.yaml +++ b/install/manifests.yaml @@ -278,6 +278,14 @@ spec: jwt: description: Authentication based on JWT tokens. properties: + issuer: + description: |- + The expected value of the "iss" (issuer) claim of the JWT. + If set, Authorino rejects, at the authentication phase, any token whose "iss" claim does not equal this value. + If omitted, the issuer claim is not verified — it can still be checked via an authorization rule (CEL, pattern-matching or OPA). + Applies to both issuerUrl and jwksUrl. It usually matches issuerUrl, but may differ when the OpenID Connect discovery + endpoint is reached at a different URL than the issuer stamped into the tokens (e.g. cluster-internal discovery vs external issuer). + type: string issuerUrl: description: |- URL of the OpenID Connect (OIDC) token issuer endpoint. diff --git a/pkg/evaluators/identity/jwt.go b/pkg/evaluators/identity/jwt.go index d4384c6f..18534b13 100644 --- a/pkg/evaluators/identity/jwt.go +++ b/pkg/evaluators/identity/jwt.go @@ -20,11 +20,17 @@ const ( msg_oidcProviderVerifierConfigRefreshSuccess = "openid connect configuration updated" msg_oidcProviderVerifierConfigRefreshError = "failed to discovery openid connect configuration" msg_oidcProviderVerifierConfigRefreshDisabled = "auto-refresh of openid connect configuration disabled" - msg_jwksVerifierFailedToCreate = "failed to create JWKS verifier" msg_jwtVerifierDoesNotStoreOpenIdConfig = "rule does not store openid configuration" ) -var tokenVerifierConfig = &oidc.Config{SkipClientIDCheck: true, SkipIssuerCheck: true} +// oidcConfig returns the go-oidc verifier config shared by both JWT verifier flavors. +// SkipClientIDCheck is always on: Authorino is not an OAuth2 client and has no audience of +// its own to match. The issuer check is enabled only when an expected issuer is configured — +// an empty issuer means "do not verify the iss claim" (the default, backwards compatible), +// a non-empty issuer means "reject any token whose iss does not equal it". +func oidcConfig(issuer string) *oidc.Config { + return &oidc.Config{SkipClientIDCheck: true, SkipIssuerCheck: issuer == ""} +} type JWTAuthentication struct { auth.AuthCredentials @@ -93,6 +99,8 @@ type JWTVerifier interface { type oidcProviderVerifier struct { issuerUrl string + issuer string + config *oidc.Config timeout *int mu sync.RWMutex @@ -100,9 +108,11 @@ type oidcProviderVerifier struct { refresher workers.Worker } -func NewOIDCProviderVerifier(ctx gocontext.Context, issuerUrl string, ttl int, timeout *int) JWTVerifier { +func NewOIDCProviderVerifier(ctx gocontext.Context, issuerUrl string, issuer string, ttl int, timeout *int) JWTVerifier { v := &oidcProviderVerifier{ issuerUrl: issuerUrl, + issuer: issuer, + config: oidcConfig(issuer), timeout: timeout, } ctxWithLogger := log.IntoContext(ctx, log.FromContext(ctx).WithName("jwt")) @@ -117,10 +127,11 @@ func (v *oidcProviderVerifier) Verify(ctx gocontext.Context, rawIDToken string) return nil, errors.New(msg_oidcProviderVerifierConfigMissingError) } - v.mu.RLock() - defer v.mu.RUnlock() - - idToken, err := provider.Verifier(tokenVerifierConfig).Verify(ctx, rawIDToken) + // No lock is held across Verify on purpose: getOpenIdProvider already returned a stable + // provider snapshot under its own lock, the go-oidc Provider is immutable, and v.config is + // set once at construction. Holding a read lock across the crypto checks and lazy JWKS fetch + // would needlessly block the background refresher's write lock in getOpenIdProvider. + idToken, err := provider.Verifier(v.config).Verify(ctx, rawIDToken) if err != nil { return nil, err } @@ -176,6 +187,15 @@ func (v *oidcProviderVerifier) getOpenIdProvider(ctx gocontext.Context, force bo httpClient := httputil.NewClientWithTracing(ctx, v.timeout) discoveryCtx := oidc.ClientContext(gocontext.Background(), httpClient) + // When an expected issuer is configured that differs from the discovery URL, pin it so + // discovery and JWKS are still fetched from issuerUrl while the verifier enforces the + // token's iss against the configured issuer. This supports setups where the OpenID + // Connect discovery endpoint is reached at a different URL than the issuer stamped into + // tokens (e.g. cluster-internal discovery vs external issuer). + if v.issuer != "" && v.issuer != v.issuerUrl { + discoveryCtx = oidc.InsecureIssuerURLContext(discoveryCtx, v.issuer) + } + if provider, err := oidc.NewProvider(discoveryCtx, v.issuerUrl); err != nil { log.FromContext(ctx).Error(err, msg_oidcProviderVerifierConfigRefreshError, "issuerUrl", v.issuerUrl) } else { @@ -200,27 +220,25 @@ func (v *oidcProviderVerifier) setupOpenIdProviderRefresh(ctx gocontext.Context, } type jwksVerifier struct { - jwks oidc.KeySet + verifier *oidc.IDTokenVerifier } -func NewJwksVerifier(ctx gocontext.Context, jwksUrl string, timeout *int) JWTVerifier { +func NewJwksVerifier(ctx gocontext.Context, jwksUrl string, issuer string, timeout *int) JWTVerifier { // Create HTTP client with timeout and trace propagation. // Use Background context for request lifecycle (to avoid cancellation from reconciliation), // but propagate trace context from caller's ctx for observability. httpClient := httputil.NewClientWithTracing(ctx, timeout) jwkCtx := oidc.ClientContext(gocontext.Background(), httpClient) + // The remote key set self-refreshes on key rotation, and issuer and config are fixed for the + // lifetime of the verifier, so it can be built once here and reused for every request. return &jwksVerifier{ - jwks: oidc.NewRemoteKeySet(jwkCtx, jwksUrl), + verifier: oidc.NewVerifier(issuer, oidc.NewRemoteKeySet(jwkCtx, jwksUrl), oidcConfig(issuer)), } } func (v *jwksVerifier) Verify(ctx gocontext.Context, rawIDToken string) (*oidc.IDToken, error) { - verifier := oidc.NewVerifier("", v.jwks, tokenVerifierConfig) - if verifier == nil { - return nil, errors.New(msg_jwksVerifierFailedToCreate) - } - return verifier.Verify(ctx, rawIDToken) + return v.verifier.Verify(ctx, rawIDToken) } // impl: auth.AuthConfigCleaner diff --git a/pkg/evaluators/identity/jwt_test.go b/pkg/evaluators/identity/jwt_test.go index 48f03a48..52f29847 100644 --- a/pkg/evaluators/identity/jwt_test.go +++ b/pkg/evaluators/identity/jwt_test.go @@ -2,7 +2,11 @@ package identity import ( "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" "fmt" + gohttptest "net/http/httptest" "sync" "testing" "time" @@ -15,6 +19,8 @@ import ( mock_workers "github.com/kuadrant/authorino/pkg/workers/mocks" "github.com/coreos/go-oidc/v3/oidc" + jose "github.com/go-jose/go-jose/v4" + "github.com/golang-jwt/jwt/v5" "go.uber.org/mock/gomock" "gotest.tools/assert" ) @@ -69,7 +75,7 @@ func TestOIDCProviderVerifierUnknownHost(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewOIDCProviderVerifier(context.TODO(), "http://unreachable-server", 0, nil) + jwtVerifier := NewOIDCProviderVerifier(context.TODO(), "http://unreachable-server", "", 0, nil) authCredMock := mock_auth.NewMockAuthCredentials(ctrl) evaluator := NewJWTAuthentication(jwtVerifier, authCredMock) @@ -91,7 +97,7 @@ func TestOIDCProviderVerifierNotFound(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), 0, nil) + jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), "", 0, nil) authCredMock := mock_auth.NewMockAuthCredentials(ctrl) evaluator := NewJWTAuthentication(jwtVerifier, authCredMock) @@ -113,7 +119,7 @@ func TestOIDCProviderVerifierInternalError(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), 0, nil) + jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), "", 0, nil) authCredMock := mock_auth.NewMockAuthCredentials(ctrl) evaluator := NewJWTAuthentication(jwtVerifier, authCredMock) @@ -143,7 +149,7 @@ func TestOIDCProviderVerifierRefresh(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), 3, nil) // refresh every 3 seconds + jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), "", 3, nil) // refresh every 3 seconds authCredMock := mock_auth.NewMockAuthCredentials(ctrl) evaluator := NewJWTAuthentication(jwtVerifier, authCredMock) defer func(evaluator *JWTAuthentication, ctx context.Context) { @@ -176,7 +182,7 @@ func TestOIDCProviderVerifierRefreshDisabled(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), 0, nil) // refresh disabled + jwtVerifier := NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), "", 0, nil) // refresh disabled defer func(verifier *oidcProviderVerifier, ctx context.Context) { _ = verifier.Clean(ctx) }(jwtVerifier.(*oidcProviderVerifier), context.Background()) @@ -223,7 +229,7 @@ func TestJWKSVerifierTokenExpired(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewJwksVerifier(context.TODO(), fmt.Sprintf("http://%v/certs", oidcServerHost), nil) + jwtVerifier := NewJwksVerifier(context.TODO(), fmt.Sprintf("http://%v/certs", oidcServerHost), "", nil) authCredMock := mock_auth.NewMockAuthCredentials(ctrl) evaluator := NewJWTAuthentication(jwtVerifier, authCredMock) @@ -242,7 +248,7 @@ func TestJWKSVerifierMalformedJWT(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - jwtVerifier := NewJwksVerifier(context.TODO(), fmt.Sprintf("http://%v/certs", oidcServerHost), nil) + jwtVerifier := NewJwksVerifier(context.TODO(), fmt.Sprintf("http://%v/certs", oidcServerHost), "", nil) authCredMock := mock_auth.NewMockAuthCredentials(ctrl) evaluator := NewJWTAuthentication(jwtVerifier, authCredMock) @@ -254,3 +260,181 @@ func TestJWKSVerifierMalformedJWT(t *testing.T) { assert.Check(t, token == nil) assert.ErrorContains(t, err, "oidc: malformed jwt") } + +const ( + issuerTestServerHost = "127.0.0.1:9007" + trustedIssuer = "http://" + issuerTestServerHost + foreignIssuer = "http://foreign-issuer.example.com" + externalIssuer = "https://external-issuer.example.com" + signingKeyId = "shared-signing-key" +) + +// newSharedSigningKey returns an RSA key plus the JWKS document advertising its +// public part, modelling an identity provider whose signing key is shared across +// more than one issuer (multi-tenant IdPs, or Authorino wristbands issued by two +// AuthConfigs backed by the same signing-key Secret). +func newSharedSigningKey(t *testing.T) (*rsa.PrivateKey, string) { + t.Helper() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + assert.NilError(t, err) + + jwks := jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{ + Key: key.Public(), + KeyID: signingKeyId, + Algorithm: "RS256", + Use: "sig", + }}} + encoded, err := json.Marshal(jwks) + assert.NilError(t, err) + + return key, string(encoded) +} + +// signToken mints an unexpired RS256 token for the given issuer, signed with key. +func signToken(t *testing.T, key *rsa.PrivateKey, issuer string) string { + t.Helper() + + token := jwt.NewWithClaims(jwt.SigningMethodRS256, jwt.MapClaims{ + "iss": issuer, + "sub": "user", + "iat": time.Now().Unix(), + "exp": time.Now().Add(time.Hour).Unix(), + }) + token.Header["kid"] = signingKeyId + + raw, err := token.SignedString(key) + assert.NilError(t, err) + return raw +} + +// newIdPMock serves the OIDC discovery document and JWKS of the trusted issuer. +func newIdPMock(jwks string) *gohttptest.Server { + return newIdPMockWithIssuer(trustedIssuer, jwks) +} + +// newIdPMockWithIssuer serves discovery at issuerTestServerHost but advertises advertisedIssuer, +// modelling a cluster-internal discovery URL with a different (e.g. external) issuer in tokens. +// JWKS is always served from the discovery host so keys remain fetchable. +func newIdPMockWithIssuer(advertisedIssuer, jwks string) *gohttptest.Server { + return httptest.NewHttpServerMock(issuerTestServerHost, map[string]httptest.HttpServerMockResponseFunc{ + "/.well-known/openid-configuration": httptest.NewHttpServerMockResponseFuncJSON( + fmt.Sprintf(`{"issuer":%q,"jwks_uri":"%v/certs"}`, advertisedIssuer, trustedIssuer), + ), + "/certs": httptest.NewHttpServerMockResponseFuncJSON(jwks), + }) +} + +func callWithToken(t *testing.T, verifier JWTVerifier, rawToken string) (any, error) { + t.Helper() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + authCredMock := mock_auth.NewMockAuthCredentials(ctrl) + evaluator := NewJWTAuthentication(verifier, authCredMock) + + pipelineMock := mock_auth.NewMockAuthPipeline(ctrl) + pipelineMock.EXPECT().GetRequest().Return(jwtAuthenticationRequestMock) + authCredMock.EXPECT(). + GetCredentialsFromAuthReq(jwtAuthenticationRequestMock.GetAttributes().GetRequest().GetHttp()). + Return(rawToken, nil) + + return evaluator.Call(pipelineMock, context.TODO()) +} + +// issuerUrl path, issuer set: a token signed by a key in the configured provider's JWKS +// but whose `iss` names a different issuer must NOT authenticate. +func TestOIDCProviderVerifier_IssuerSet_RejectsForeignIssuer(t *testing.T) { + key, jwks := newSharedSigningKey(t) + authServer := newIdPMock(jwks) + defer authServer.Close() + + verifier := NewOIDCProviderVerifier(context.TODO(), trustedIssuer, trustedIssuer, 0, nil) + + obj, err := callWithToken(t, verifier, signToken(t, key, foreignIssuer)) + + assert.Check(t, obj == nil, "token from a foreign issuer was accepted as a valid identity") + assert.ErrorContains(t, err, "issued by a different provider") +} + +// issuerUrl path, issuer unset (the default): legacy behavior is preserved — the foreign-issuer +// token is still accepted, and callers may enforce `iss` via an authorization rule. Pins the +// opt-in nature of the field. +func TestOIDCProviderVerifier_IssuerUnset_AcceptsForeignIssuer(t *testing.T) { + key, jwks := newSharedSigningKey(t) + authServer := newIdPMock(jwks) + defer authServer.Close() + + verifier := NewOIDCProviderVerifier(context.TODO(), trustedIssuer, "", 0, nil) + + obj, err := callWithToken(t, verifier, signToken(t, key, foreignIssuer)) + + assert.NilError(t, err) + assert.Equal(t, obj.(map[string]any)["iss"].(string), foreignIssuer) +} + +// issuerUrl path, issuer set: the happy path (matching `iss`) still authenticates. +func TestOIDCProviderVerifier_IssuerSet_AcceptsMatchingIssuer(t *testing.T) { + key, jwks := newSharedSigningKey(t) + authServer := newIdPMock(jwks) + defer authServer.Close() + + verifier := NewOIDCProviderVerifier(context.TODO(), trustedIssuer, trustedIssuer, 0, nil) + + obj, err := callWithToken(t, verifier, signToken(t, key, trustedIssuer)) + + assert.NilError(t, err) + assert.Equal(t, obj.(map[string]any)["iss"].(string), trustedIssuer) +} + +// issuerUrl path, issuer differs from issuerUrl (cluster-internal discovery / external issuer): +// discovery and JWKS are fetched from issuerUrl while `iss` is enforced against the external +// issuer — accepting the legitimate external-issuer token and rejecting a wrong-issuer one. +func TestOIDCProviderVerifier_IssuerDiffersFromIssuerUrl_EnforcesConfiguredIssuer(t *testing.T) { + key, jwks := newSharedSigningKey(t) + authServer := newIdPMockWithIssuer(externalIssuer, jwks) // served at trustedIssuer, advertises externalIssuer + defer authServer.Close() + + verifier := NewOIDCProviderVerifier(context.TODO(), trustedIssuer, externalIssuer, 0, nil) + + obj, err := callWithToken(t, verifier, signToken(t, key, externalIssuer)) + assert.NilError(t, err) + assert.Equal(t, obj.(map[string]any)["iss"].(string), externalIssuer) + + obj, err = callWithToken(t, verifier, signToken(t, key, foreignIssuer)) + assert.Check(t, obj == nil, "wrong-issuer token accepted in the internal/external split configuration") + assert.ErrorContains(t, err, "issued by a different provider") +} + +// jwksUrl path, issuer set: `iss` is enforced +func TestJWKSVerifier_IssuerSet_EnforcesIssuer(t *testing.T) { + key, jwks := newSharedSigningKey(t) + authServer := newIdPMock(jwks) + defer authServer.Close() + + verifier := NewJwksVerifier(context.TODO(), trustedIssuer+"/certs", trustedIssuer, nil) + + obj, err := callWithToken(t, verifier, signToken(t, key, trustedIssuer)) + assert.NilError(t, err) + assert.Equal(t, obj.(map[string]any)["iss"].(string), trustedIssuer) + + obj, err = callWithToken(t, verifier, signToken(t, key, foreignIssuer)) + assert.Check(t, obj == nil, "foreign-issuer token accepted on the jwksUrl path with issuer set") + assert.ErrorContains(t, err, "issued by a different provider") +} + +// jwksUrl path, issuer unset: legacy behavior — any signature-valid token is accepted +// regardless of `iss`. +func TestJWKSVerifier_IssuerUnset_IgnoresIssuer(t *testing.T) { + key, jwks := newSharedSigningKey(t) + authServer := newIdPMock(jwks) + defer authServer.Close() + + verifier := NewJwksVerifier(context.TODO(), trustedIssuer+"/certs", "", nil) + + obj, err := callWithToken(t, verifier, signToken(t, key, foreignIssuer)) + + assert.NilError(t, err) + assert.Equal(t, obj.(map[string]any)["iss"].(string), foreignIssuer) +} diff --git a/pkg/evaluators/metadata/user_info_test.go b/pkg/evaluators/metadata/user_info_test.go index 46d5cce8..9f399810 100644 --- a/pkg/evaluators/metadata/user_info_test.go +++ b/pkg/evaluators/metadata/user_info_test.go @@ -37,7 +37,7 @@ type userInfoTestData struct { func newUserInfoTestData(ctrl *gomock.Controller) userInfoTestData { authCredMock := mock_auth.NewMockAuthCredentials(ctrl) - newOIDC := identity.NewJWTAuthentication(identity.NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%s", authServerHost), 0, nil), authCredMock) + newOIDC := identity.NewJWTAuthentication(identity.NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%s", authServerHost), "", 0, nil), authCredMock) ctx, cancel := context.WithCancel(context.TODO()) return userInfoTestData{ ctx, @@ -107,7 +107,7 @@ func TestUserInfoMissingOIDCConfig(t *testing.T) { defer ctrl.Finish() ta := newUserInfoTestData(ctrl) - otherOidcEvaluator := identity.NewJWTAuthentication(identity.NewOIDCProviderVerifier(context.TODO(), "http://wrongServer", 0, nil), ta.authCredMock) + otherOidcEvaluator := identity.NewJWTAuthentication(identity.NewOIDCProviderVerifier(context.TODO(), "http://wrongServer", "", 0, nil), ta.authCredMock) ta.pipelineMock.EXPECT().GetResolvedIdentity().Return(ta.idConfEvalMock, nil) ta.idConfEvalMock.EXPECT().GetOpenIdConfig().Return(otherOidcEvaluator) diff --git a/pkg/service/auth_pipeline_test.go b/pkg/service/auth_pipeline_test.go index c37b50cb..b4ea4984 100644 --- a/pkg/service/auth_pipeline_test.go +++ b/pkg/service/auth_pipeline_test.go @@ -566,7 +566,7 @@ func BenchmarkAuthPipeline(b *testing.B) { authCredMock := mock_auth.NewMockAuthCredentials(ctrl) authCredMock.EXPECT().GetIdentifier().Return("Bearer").AnyTimes() // this will only be invoked if the access token below is expired authCredMock.EXPECT().GetCredentialsFromAuthReq(gomock.Any()).Return("eyJhbGciOiJSUzI1NiIsInR5cCIgOiAiSldUIiwia2lkIiA6ICJ5cm0tSWpweGRfd3dzVmZPR1FUWWE2NHVmdEVlOHY3VG5sQzFMLUl4ZUlJIn0.eyJleHAiOjIxNDU4NjU3NzMsImlhdCI6MTY1OTA4ODE3MywianRpIjoiZDI0ODliMWEtYjY0Yi00MzRhLWJhNmItMmQ4OGIyY2I1ZWE3IiwiaXNzIjoiaHR0cDovL2tleWNsb2FrOjgwODAvYXV0aC9yZWFsbXMva3VhZHJhbnQiLCJhdWQiOlsicmVhbG0tbWFuYWdlbWVudCIsImFjY291bnQiXSwic3ViIjoiMWEwYjZjNmUtNDdmNy00ZjI1LWEyNjYtYzg3MzZhOTkxODQ0IiwidHlwIjoiQmVhcmVyIiwiYXpwIjoiZGVtbyIsInNlc3Npb25fc3RhdGUiOiIxMTdkMTc1Ni1mM2RlLTRjM2MtOWEwZS0zYjU5Mzc2YmI0ZTgiLCJhY3IiOiIxIiwicmVhbG1fYWNjZXNzIjp7InJvbGVzIjpbIm9mZmxpbmVfYWNjZXNzIiwibWVtYmVyIiwidW1hX2F1dGhvcml6YXRpb24iXX0sInJlc291cmNlX2FjY2VzcyI6eyJyZWFsbS1tYW5hZ2VtZW50Ijp7InJvbGVzIjpbInZpZXctaWRlbnRpdHktcHJvdmlkZXJzIiwidmlldy1yZWFsbSIsIm1hbmFnZS1pZGVudGl0eS1wcm92aWRlcnMiLCJpbXBlcnNvbmF0aW9uIiwicmVhbG0tYWRtaW4iLCJjcmVhdGUtY2xpZW50IiwibWFuYWdlLXVzZXJzIiwicXVlcnktcmVhbG1zIiwidmlldy1hdXRob3JpemF0aW9uIiwicXVlcnktY2xpZW50cyIsInF1ZXJ5LXVzZXJzIiwibWFuYWdlLWV2ZW50cyIsIm1hbmFnZS1yZWFsbSIsInZpZXctZXZlbnRzIiwidmlldy11c2VycyIsInZpZXctY2xpZW50cyIsIm1hbmFnZS1hdXRob3JpemF0aW9uIiwibWFuYWdlLWNsaWVudHMiLCJxdWVyeS1ncm91cHMiXX0sImFjY291bnQiOnsicm9sZXMiOlsibWFuYWdlLWFjY291bnQiLCJtYW5hZ2UtYWNjb3VudC1saW5rcyJdfX0sInNjb3BlIjoicHJvZmlsZSBlbWFpbCIsInNpZCI6IjExN2QxNzU2LWYzZGUtNGMzYy05YTBlLTNiNTkzNzZiYjRlOCIsImVtYWlsX3ZlcmlmaWVkIjpmYWxzZSwibmFtZSI6IlBldGVyIFdobyIsInByZWZlcnJlZF91c2VybmFtZSI6InBldGVyIiwiZ2l2ZW5fbmFtZSI6IlBldGVyIiwiZmFtaWx5X25hbWUiOiJXaG8iLCJlbWFpbCI6InBldGVyQGt1YWRyYW50LmlvIn0.Yy2aWR6_u0NBLx8x--OToYipfQ1f1KcC8zedsKDiymcbBiAaxrBQmaV2JC1PQVEgyxwmyMk0Rao2MdKGWk6pXB9mTUF5FX-pS8mkPIMUt1UVGJgzq7WR9KfRqdZSzRtFQHoDmTeA1-msayMYTAD8xtUH4JYRNbIXjY2cEtn8LjuLpQVR3DR4_ARMrEYXiDBS3rmmFKHdipqU7ozwJ_gtpZv8vfeiO3mUPyQLJKQ-nKpe_Z5z7tm_Ewh5MN2oBfn_0pcdANB3pe2RclGAm-YHlyNDTnAZL2Y1gdCmwzwigk7AJcgWtPqnRzvEQ9zRBxQRai5W5aNKYTxuKIG8k9N05w", nil).MinTimes(1) - idConfig := &evaluators.IdentityConfig{JWTAuthentication: identity.NewJWTAuthentication(identity.NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), 0, nil), authCredMock)} + idConfig := &evaluators.IdentityConfig{JWTAuthentication: identity.NewJWTAuthentication(identity.NewOIDCProviderVerifier(context.TODO(), fmt.Sprintf("http://%v", oidcServerHost), "", 0, nil), authCredMock)} authzConfig := &evaluators.AuthorizationConfig{JSON: &authorization.JSONPatternMatching{Rules: jsonexp.All(jsonexp.Pattern{Selector: "auth.identity.realm_access.roles", Operator: jsonexp.IncludesOperator, Value: "member"})}} pipeline := newTestAuthPipeline(evaluators.AuthConfig{IdentityConfigs: []auth.AuthConfigEvaluator{idConfig}, AuthorizationConfigs: []auth.AuthConfigEvaluator{authzConfig}}, &request)