diff --git a/cmd/defaults.yaml b/cmd/defaults.yaml index 0f56ddb8b..76f2fb7fb 100644 --- a/cmd/defaults.yaml +++ b/cmd/defaults.yaml @@ -339,6 +339,9 @@ OIDC: AuthMethodPrivateKeyJWT: true # ZITADEL_OIDC_AUTHMETHODPRIVATEKEYJWT GrantTypeRefreshToken: true # ZITADEL_OIDC_GRANTTYPEREFRESHTOKEN RequestObjectSupported: true # ZITADEL_OIDC_REQUESTOBJECTSUPPORTED + + # Deprecated: The signing algorithm is determined by the generated keys. + # Use the web keys resource to generate keys with different algorithms. SigningKeyAlgorithm: RS256 # ZITADEL_OIDC_SIGNINGKEYALGORITHM # Sets the default values for lifetime and expiration for OIDC # This default can be overwritten in the default instance configuration and for each instance during runtime @@ -349,10 +352,10 @@ OIDC: DefaultRefreshTokenIdleExpiration: 720h # ZITADEL_OIDC_DEFAULTREFRESHTOKENIDLEEXPIRATION # 2160h are 90 days, three months DefaultRefreshTokenExpiration: 2160h # ZITADEL_OIDC_DEFAULTREFRESHTOKENEXPIRATION - Cache: - MaxAge: 12h # ZITADEL_OIDC_CACHE_MAXAGE - # 168h is 7 days, one week - SharedMaxAge: 168h # ZITADEL_OIDC_CACHE_SHAREDMAXAGE + + # HTTP Cache-Control max-age header value to set on the jwks endpoint. + # Only used when the web keys feature is enabled. 0 sets a no-store value. + JWKSCacheControlMaxAge: 5m # ZITADEL_OIDC_JWKSCACHECONTROLMAXAGE CustomEndpoints: Auth: Path: /oauth/v2/authorize # ZITADEL_OIDC_CUSTOMENDPOINTS_AUTH_PATH diff --git a/go.mod b/go.mod index 70c0b2859..e514c62f7 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( github.com/superseriousbusiness/exifremove v0.0.0-20210330092427-6acd27eac203 github.com/ttacon/libphonenumber v1.2.1 github.com/zitadel/logging v0.6.0 - github.com/zitadel/oidc/v3 v3.27.0 + github.com/zitadel/oidc/v3 v3.28.1 github.com/zitadel/passwap v0.6.0 github.com/zitadel/saml v0.1.3 github.com/zitadel/schema v1.3.0 @@ -78,8 +78,8 @@ require ( golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8 golang.org/x/net v0.26.0 golang.org/x/oauth2 v0.22.0 - golang.org/x/sync v0.7.0 - golang.org/x/text v0.16.0 + golang.org/x/sync v0.8.0 + golang.org/x/text v0.17.0 google.golang.org/api v0.187.0 google.golang.org/genproto/googleapis/api v0.0.0-20240701130421-f6361c86f094 google.golang.org/grpc v1.65.0 diff --git a/go.sum b/go.sum index 30eee5954..aef33fe16 100644 --- a/go.sum +++ b/go.sum @@ -723,8 +723,8 @@ github.com/zenazn/goji v1.0.1 h1:4lbD8Mx2h7IvloP7r2C0D6ltZP6Ufip8Hn0wmSK5LR8= github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= github.com/zitadel/logging v0.6.0 h1:t5Nnt//r+m2ZhhoTmoPX+c96pbMarqJvW1Vq6xFTank= github.com/zitadel/logging v0.6.0/go.mod h1:Y4CyAXHpl3Mig6JOszcV5Rqqsojj+3n7y2F591Mp/ow= -github.com/zitadel/oidc/v3 v3.27.0 h1:zeYpyRH0UcgdCjVHUYzSsqf1jbSwVMPVxYKOnRXstgU= -github.com/zitadel/oidc/v3 v3.27.0/go.mod h1:ZwBEqSviCpJVZiYashzo53bEGRGXi7amE5Q8PpQg9IM= +github.com/zitadel/oidc/v3 v3.28.1 h1:PsbFm5CzEMQq9HBXUNJ8yvnWmtVYxpwV5Cinj7TTsHo= +github.com/zitadel/oidc/v3 v3.28.1/go.mod h1:WmDFu3dZ9YNKrIoZkmxjGG8QyUR4PbbhsVVSY+rpojM= github.com/zitadel/passwap v0.6.0 h1:m9F3epFC0VkBXu25rihSLGyHvWiNlCzU5kk8RoI+SXQ= github.com/zitadel/passwap v0.6.0/go.mod h1:kqAiJ4I4eZvm3Y6oAk6hlEqlZZOkjMHraGXF90GG7LI= github.com/zitadel/saml v0.1.3 h1:LI4DOCVyyU1qKPkzs3vrGcA5J3H4pH3+CL9zr9ShkpM= @@ -871,8 +871,8 @@ golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M= -golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= +golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= @@ -932,8 +932,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= -golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI= +golang.org/x/text v0.17.0 h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc= +golang.org/x/text v0.17.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk= diff --git a/internal/api/http/middleware/cache_interceptor.go b/internal/api/http/middleware/cache_interceptor.go index 04dbf2b6c..de0c0df61 100644 --- a/internal/api/http/middleware/cache_interceptor.go +++ b/internal/api/http/middleware/cache_interceptor.go @@ -108,6 +108,11 @@ func (c *Cache) serializeHeaders(w http.ResponseWriter) { control := make([]string, 0, 6) pragma := false + // Do not overwrite cache-control header if set by business logic. + if w.Header().Get(http_utils.CacheControl) != "" { + return + } + if c.Cacheability != CacheabilityNotSet { control = append(control, string(c.Cacheability)) control = append(control, fmt.Sprintf("max-age=%v", c.MaxAge.Seconds())) diff --git a/internal/api/oidc/access_token.go b/internal/api/oidc/access_token.go index 4ac1646ed..66da6e3cc 100644 --- a/internal/api/oidc/access_token.go +++ b/internal/api/oidc/access_token.go @@ -52,7 +52,9 @@ func (s *Server) verifyAccessToken(ctx context.Context, tkn string) (_ *accessTo } tokenID, subject = split[0], split[1] } else { - verifier := op.NewAccessTokenVerifier(op.IssuerFromContext(ctx), s.accessTokenKeySet) + verifier := op.NewAccessTokenVerifier(op.IssuerFromContext(ctx), s.accessTokenKeySet, + op.WithSupportedAccessTokenSigningAlgorithms(supportedSigningAlgs(ctx)...), + ) claims, err := op.VerifyAccessToken[*oidc.AccessTokenClaims](ctx, tkn, verifier) if err != nil { return nil, zerrors.ThrowPermissionDenied(err, "OIDC-Eib8e", "token is not valid or has expired") diff --git a/internal/api/oidc/key.go b/internal/api/oidc/key.go index 2db5baf83..a7e156fe7 100644 --- a/internal/api/oidc/key.go +++ b/internal/api/oidc/key.go @@ -3,16 +3,19 @@ package oidc import ( "context" "fmt" + "slices" "sync" "sync/atomic" "time" "github.com/go-jose/go-jose/v4" "github.com/jonboulle/clockwork" + "github.com/muhlemmer/gu" "github.com/zitadel/logging" "github.com/zitadel/oidc/v3/pkg/op" "github.com/zitadel/zitadel/internal/api/authz" + http_util "github.com/zitadel/zitadel/internal/api/http" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/query" @@ -22,14 +25,33 @@ import ( "github.com/zitadel/zitadel/internal/zerrors" ) -type cachedPublicKey struct { - lastUse atomic.Int64 // unix micro time. - query.PublicKey +var supportedWebKeyAlgs = []string{ + string(jose.EdDSA), + string(jose.RS256), + string(jose.RS384), + string(jose.RS512), + string(jose.ES256), + string(jose.ES384), + string(jose.ES512), } -func newCachedPublicKey(key query.PublicKey, now time.Time) *cachedPublicKey { +func supportedSigningAlgs(ctx context.Context) []string { + if authz.GetFeatures(ctx).WebKey { + return supportedWebKeyAlgs + } + return []string{string(jose.RS256)} +} + +type cachedPublicKey struct { + lastUse atomic.Int64 // unix micro time. + expiry *time.Time // expiry may be nil if the key does not expire. + webKey *jose.JSONWebKey +} + +func newCachedPublicKey(key *jose.JSONWebKey, expiry *time.Time, now time.Time) *cachedPublicKey { cachedKey := &cachedPublicKey{ - PublicKey: key, + expiry: expiry, + webKey: key, } cachedKey.setLastUse(now) return cachedKey @@ -53,14 +75,17 @@ func (c *cachedPublicKey) expired(now time.Time, validity time.Duration) bool { type publicKeyCache struct { mtx sync.RWMutex instanceKeys map[string]map[string]*cachedPublicKey - queryKey func(ctx context.Context, keyID string) (query.PublicKey, error) - clock clockwork.Clock + + // queryKey returns a public web key. + // If the key does not have expiry, Time may be nil. + queryKey func(ctx context.Context, keyID string) (*jose.JSONWebKey, *time.Time, error) + clock clockwork.Clock } // newPublicKeyCache initializes a keySetCache starts a purging Go routine. // The purge routine deletes all public keys that are older than maxAge. // When the passed context is done, the purge routine will terminate. -func newPublicKeyCache(background context.Context, maxAge time.Duration, queryKey func(ctx context.Context, keyID string) (query.PublicKey, error)) *publicKeyCache { +func newPublicKeyCache(background context.Context, maxAge time.Duration, queryKey func(ctx context.Context, keyID string) (*jose.JSONWebKey, *time.Time, error)) *publicKeyCache { k := &publicKeyCache{ instanceKeys: make(map[string]map[string]*cachedPublicKey), queryKey: queryKey, @@ -119,11 +144,11 @@ func (k *publicKeyCache) getKey(ctx context.Context, keyID string) (_ *cachedPub if ok { key.setLastUse(k.clock.Now()) } else { - newKey, err := k.queryKey(ctx, keyID) + newKey, expiry, err := k.queryKey(ctx, keyID) if err != nil { return nil, err } - key = newCachedPublicKey(newKey, k.clock.Now()) + key = newCachedPublicKey(newKey, expiry, k.clock.Now()) k.setKey(instanceID, keyID, key) } @@ -144,10 +169,10 @@ func (k *publicKeyCache) verifySignature(ctx context.Context, jws *jose.JSONWebS if err != nil { return nil, err } - if checkKeyExpiry && key.Expiry().Before(k.clock.Now()) { + if checkKeyExpiry && key.expiry != nil && key.expiry.Before(k.clock.Now()) { return nil, zerrors.ThrowInvalidArgument(err, "QUERY-ciF4k", "Errors.Key.ExpireBeforeNow") } - return jws.Verify(jsonWebkey(key)) + return jws.Verify(key.webKey) } type oidcKeySet struct { @@ -423,3 +448,68 @@ func retry(retryable func() error) (err error) { } return err } + +func (s *Server) Keys(ctx context.Context, r *op.Request[struct{}]) (_ *op.Response, err error) { + ctx, span := tracing.NewSpan(ctx) + defer func() { span.EndWithError(err) }() + + if !authz.GetFeatures(ctx).WebKey { + return s.LegacyServer.Keys(ctx, r) + } + + keyset, err := s.query.GetWebKeySet(ctx) + if err != nil { + return nil, err + } + + // Return legacy keys, so we do not invalidate all tokens + // once the feature flag is enabled. + legacyKeys, err := s.query.ActivePublicKeys(ctx, time.Now()) + logging.OnError(err).Error("oidc server: active public keys (legacy)") + appendPublicKeysToWebKeySet(keyset, legacyKeys) + + resp := op.NewResponse(keyset) + if s.jwksCacheControlMaxAge != 0 { + resp.Header.Set(http_util.CacheControl, + fmt.Sprintf("max-age=%d, must-revalidate", int(s.jwksCacheControlMaxAge/time.Second)), + ) + } + + return resp, nil +} + +func appendPublicKeysToWebKeySet(keyset *jose.JSONWebKeySet, pubkeys *query.PublicKeys) { + if pubkeys == nil || len(pubkeys.Keys) == 0 { + return + } + keyset.Keys = slices.Grow(keyset.Keys, len(pubkeys.Keys)) + + for _, key := range pubkeys.Keys { + keyset.Keys = append(keyset.Keys, jose.JSONWebKey{ + Key: key.Key(), + KeyID: key.ID(), + Algorithm: key.Algorithm(), + Use: key.Use().String(), + }) + } +} + +func queryKeyFunc(q *query.Queries) func(ctx context.Context, keyID string) (*jose.JSONWebKey, *time.Time, error) { + return func(ctx context.Context, keyID string) (*jose.JSONWebKey, *time.Time, error) { + if authz.GetFeatures(ctx).WebKey { + webKey, err := q.GetPublicWebKeyByID(ctx, keyID) + if err == nil { + return webKey, nil, nil + } + if !zerrors.IsNotFound(err) { + return nil, nil, err + } + } + + pubKey, err := q.GetPublicKeyByID(ctx, keyID) + if err != nil { + return nil, nil, err + } + return jsonWebkey(pubKey), gu.Ptr(pubKey.Expiry()), nil + } +} diff --git a/internal/api/oidc/key_test.go b/internal/api/oidc/key_test.go index 3f84722a9..b495b4132 100644 --- a/internal/api/oidc/key_test.go +++ b/internal/api/oidc/key_test.go @@ -2,12 +2,14 @@ package oidc import ( "context" + "crypto/rand" "errors" "testing" "time" "github.com/go-jose/go-jose/v4" "github.com/jonboulle/clockwork" + "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -51,36 +53,45 @@ func (k *publicKey) Key() any { var ( clock = clockwork.NewFakeClock() - keyDB = map[string]*publicKey{ + keyDB = map[string]struct { + webKey *jose.JSONWebKey + expiry *time.Time + }{ "key1": { - id: "key1", - alg: "alg", - use: crypto.KeyUsageSigning, - seq: 1, - expiry: clock.Now().Add(time.Minute), + webKey: &jose.JSONWebKey{ + Key: "abc", + KeyID: "key1", + Algorithm: "alg", + Use: "sig", + }, + expiry: gu.Ptr(clock.Now().Add(time.Minute)), }, "key2": { - id: "key2", - alg: "alg", - use: crypto.KeyUsageSigning, - seq: 3, - expiry: clock.Now().Add(10 * time.Hour), + webKey: &jose.JSONWebKey{ + Key: "def", + KeyID: "key1", + Algorithm: "alg", + Use: "sig", + }, + expiry: gu.Ptr(clock.Now().Add(10 * time.Hour)), }, "exp1": { - id: "key2", - alg: "alg", - use: crypto.KeyUsageSigning, - seq: 4, - expiry: clock.Now().Add(-time.Hour), + webKey: &jose.JSONWebKey{ + Key: "ghi", + KeyID: "exp1", + Algorithm: "alg", + Use: "sig", + }, + expiry: gu.Ptr(clock.Now().Add(-time.Hour)), }, } ) -func queryKeyDB(_ context.Context, keyID string) (query.PublicKey, error) { +func queryKeyDB(_ context.Context, keyID string) (*jose.JSONWebKey, *time.Time, error) { if key, ok := keyDB[keyID]; ok { - return key, nil + return key.webKey, key.expiry, nil } - return nil, errors.New("not found") + return nil, nil, errors.New("not found") } func Test_publicKeyCache(t *testing.T) { @@ -102,7 +113,7 @@ func Test_publicKeyCache(t *testing.T) { got, err := cache.getKey(ctx, "key1") require.NoError(t, err) require.NotNil(t, got) - assert.Equal(t, keyDB["key1"], got.PublicKey) + assert.Equal(t, keyDB["key1"].webKey, got.webKey) // move time forward clock.Advance(15 * time.Minute) @@ -122,7 +133,7 @@ func Test_publicKeyCache(t *testing.T) { got, err = cache.getKey(ctx, "key2") require.NoError(t, err) require.NotNil(t, got) - assert.Equal(t, keyDB["key2"], got.PublicKey) + assert.Equal(t, keyDB["key2"].webKey, got.webKey) // move time forward clock.Advance(15 * time.Minute) @@ -140,7 +151,7 @@ func Test_publicKeyCache(t *testing.T) { got, err = cache.getKey(ctx, "key2") require.NoError(t, err) require.NotNil(t, got) - assert.Equal(t, keyDB["key2"], got.PublicKey) + assert.Equal(t, keyDB["key2"].webKey, got.webKey) // move time forward clock.Advance(2 * time.Hour) @@ -266,3 +277,126 @@ func Test_keySetMap_VerifySignature(t *testing.T) { }) } } + +func Test_appendPublicKeysToWebKeySet(t *testing.T) { + keys := [...][]byte{ + make([]byte, 32), + make([]byte, 32), + } + for _, key := range keys { + _, err := rand.Read(key) + require.NoError(t, err) + } + + type args struct { + keyset *jose.JSONWebKeySet + pubkeys *query.PublicKeys + } + tests := []struct { + name string + args args + want *jose.JSONWebKeySet + }{ + { + name: "nil pubkeys", + args: args{ + keyset: &jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: keys[0], + KeyID: "key0", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + }, + }, + pubkeys: nil, + }, + want: &jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: keys[0], + KeyID: "key0", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + }, + }, + }, + { + name: "empty pubkeys", + args: args{ + keyset: &jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: keys[0], + KeyID: "key0", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + }, + }, + pubkeys: &query.PublicKeys{ + Keys: []query.PublicKey{}, + }, + }, + want: &jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: keys[0], + KeyID: "key0", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + }, + }, + }, + { + name: "append pubkeys", + args: args{ + keyset: &jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: keys[0], + KeyID: "key0", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + }, + }, + pubkeys: &query.PublicKeys{ + Keys: []query.PublicKey{ + &publicKey{ + id: "key1", + key: keys[1], + alg: "XYZ", + use: crypto.KeyUsageSigning, + }, + }, + }, + }, + want: &jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{ + { + Key: keys[0], + KeyID: "key0", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + { + Key: keys[1], + KeyID: "key1", + Algorithm: "XYZ", + Use: crypto.KeyUsageSigning.String(), + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + appendPublicKeysToWebKeySet(tt.args.keyset, tt.args.pubkeys) + assert.Equal(t, tt.want, tt.args.keyset) + }) + } +} diff --git a/internal/api/oidc/keys_integration_test.go b/internal/api/oidc/keys_integration_test.go new file mode 100644 index 000000000..a78e4fc1b --- /dev/null +++ b/internal/api/oidc/keys_integration_test.go @@ -0,0 +1,115 @@ +//go:build integration + +package oidc_test + +import ( + "crypto/rsa" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/go-jose/go-jose/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zitadel/oidc/v3/pkg/client" + "github.com/zitadel/oidc/v3/pkg/oidc" + "google.golang.org/protobuf/proto" + + http_util "github.com/zitadel/zitadel/internal/api/http" + "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/pkg/grpc/feature/v2" + oidc_pb "github.com/zitadel/zitadel/pkg/grpc/oidc/v2" +) + +func TestServer_Keys(t *testing.T) { + // TODO: isolated instance + + clientID, _ := createClient(t) + authRequestID := createAuthRequest(t, clientID, redirectURI, oidc.ScopeOpenID, oidc.ScopeOfflineAccess, zitadelAudienceScope) + sessionID, sessionToken, _, _ := Tester.CreateVerifiedWebAuthNSession(t, CTXLOGIN, User.GetUserId()) + linkResp, err := Tester.Client.OIDCv2.CreateCallback(CTXLOGIN, &oidc_pb.CreateCallbackRequest{ + AuthRequestId: authRequestID, + CallbackKind: &oidc_pb.CreateCallbackRequest_Session{ + Session: &oidc_pb.Session{ + SessionId: sessionID, + SessionToken: sessionToken, + }, + }, + }) + require.NoError(t, err) + + // code exchange so we are sure there is 1 legacy key pair. + code := assertCodeResponse(t, linkResp.GetCallbackUrl()) + _, err = exchangeTokens(t, clientID, code, redirectURI) + require.NoError(t, err) + + issuer := http_util.BuildHTTP(Tester.Config.ExternalDomain, Tester.Config.Port, Tester.Config.ExternalSecure) + discovery, err := client.Discover(CTX, issuer, http.DefaultClient) + require.NoError(t, err) + + tests := []struct { + name string + webKeyFeature bool + wantLen int + }{ + { + name: "legacy only", + webKeyFeature: false, + wantLen: 1, + }, + { + name: "webkeys with legacy", + webKeyFeature: true, + wantLen: 3, // 1 legacy + 2 created by enabling feature flag + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ensureWebKeyFeature(t, tt.webKeyFeature) + + assert.EventuallyWithT(t, func(ttt *assert.CollectT) { + resp, err := http.Get(discovery.JwksURI) + require.NoError(ttt, err) + require.Equal(ttt, resp.StatusCode, http.StatusOK) + defer resp.Body.Close() + + got := new(jose.JSONWebKeySet) + err = json.NewDecoder(resp.Body).Decode(got) + require.NoError(ttt, err) + + assert.Len(t, got.Keys, tt.wantLen) + for _, key := range got.Keys { + _, ok := key.Key.(*rsa.PublicKey) + require.True(ttt, ok) + require.NotEmpty(ttt, key.KeyID) + require.Equal(ttt, key.Algorithm, string(jose.RS256)) + require.Equal(ttt, key.Use, crypto.KeyUsageSigning.String()) + } + + cacheControl := resp.Header.Get("cache-control") + if tt.webKeyFeature { + require.Equal(ttt, "max-age=300, must-revalidate", cacheControl) + return + } + require.Equal(ttt, "no-store", cacheControl) + + }, time.Minute, time.Second/10) + }) + + } +} + +func ensureWebKeyFeature(t *testing.T, set bool) { + _, err := Tester.Client.FeatureV2.SetInstanceFeatures(CTXIAM, &feature.SetInstanceFeaturesRequest{ + WebKey: proto.Bool(set), + }) + require.NoError(t, err) + + t.Cleanup(func() { + _, err := Tester.Client.FeatureV2.SetInstanceFeatures(CTXIAM, &feature.SetInstanceFeaturesRequest{ + WebKey: proto.Bool(false), + }) + require.NoError(t, err) + }) +} diff --git a/internal/api/oidc/op.go b/internal/api/oidc/op.go index 67bc1765a..c8dafb50f 100644 --- a/internal/api/oidc/op.go +++ b/internal/api/oidc/op.go @@ -36,8 +36,7 @@ type Config struct { DefaultIdTokenLifetime time.Duration DefaultRefreshTokenIdleExpiration time.Duration DefaultRefreshTokenExpiration time.Duration - UserAgentCookieConfig *middleware.UserAgentCookieConfig - Cache *middleware.CacheConfig + JWKSCacheControlMaxAge time.Duration CustomEndpoints *EndpointConfig DeviceAuth *DeviceAuthorizationConfig DefaultLoginURLV2 string @@ -79,6 +78,27 @@ type OPStorage struct { assetAPIPrefix func(ctx context.Context) string } +// Provider is used to overload certain [op.Provider] methods +type Provider struct { + *op.Provider + accessTokenKeySet oidc.KeySet + idTokenHintKeySet oidc.KeySet +} + +// IDTokenHintVerifier configures a Verifier and supported signing algorithms based on the Web Key feature in the context. +func (o *Provider) IDTokenHintVerifier(ctx context.Context) *op.IDTokenHintVerifier { + return op.NewIDTokenHintVerifier(op.IssuerFromContext(ctx), o.idTokenHintKeySet, op.WithSupportedIDTokenHintSigningAlgorithms( + supportedSigningAlgs(ctx)..., + )) +} + +// AccessTokenVerifier configures a Verifier and supported signing algorithms based on the Web Key feature in the context. +func (o *Provider) AccessTokenVerifier(ctx context.Context) *op.AccessTokenVerifier { + return op.NewAccessTokenVerifier(op.IssuerFromContext(ctx), o.accessTokenKeySet, op.WithSupportedAccessTokenSigningAlgorithms( + supportedSigningAlgs(ctx)..., + )) +} + func NewServer( ctx context.Context, config Config, @@ -101,14 +121,11 @@ func NewServer( return nil, zerrors.ThrowInternal(err, "OIDC-EGrqd", "cannot create op config: %w") } storage := newStorage(config, command, query, repo, encryptionAlg, es, projections) - keyCache := newPublicKeyCache(ctx, config.PublicKeyCacheMaxAge, query.GetPublicKeyByID) + keyCache := newPublicKeyCache(ctx, config.PublicKeyCacheMaxAge, queryKeyFunc(query)) accessTokenKeySet := newOidcKeySet(keyCache, withKeyExpiryCheck(true)) idTokenHintKeySet := newOidcKeySet(keyCache) - options := []op.Option{ - op.WithAccessTokenKeySet(accessTokenKeySet), - op.WithIDTokenHintKeySet(idTokenHintKeySet), - } + var options []op.Option if !externalSecure { options = append(options, op.WithAllowInsecure()) } @@ -126,7 +143,11 @@ func NewServer( return nil, zerrors.ThrowInternal(err, "OIDC-Aij4e", "cannot create secret hasher") } server := &Server{ - LegacyServer: op.NewLegacyServer(provider, endpoints(config.CustomEndpoints)), + LegacyServer: op.NewLegacyServer(&Provider{ + Provider: provider, + accessTokenKeySet: accessTokenKeySet, + idTokenHintKeySet: idTokenHintKeySet, + }, endpoints(config.CustomEndpoints)), repo: repo, query: query, command: command, @@ -137,6 +158,7 @@ func NewServer( defaultLogoutURLV2: config.DefaultLogoutURLV2, defaultAccessTokenLifetime: config.DefaultAccessTokenLifetime, defaultIdTokenLifetime: config.DefaultIdTokenLifetime, + jwksCacheControlMaxAge: config.JWKSCacheControlMaxAge, fallbackLogger: fallbackLogger, hasher: hasher, signingKeyAlgorithm: config.SigningKeyAlgorithm, diff --git a/internal/api/oidc/server.go b/internal/api/oidc/server.go index 1152d81b4..15628cad8 100644 --- a/internal/api/oidc/server.go +++ b/internal/api/oidc/server.go @@ -34,6 +34,7 @@ type Server struct { defaultLogoutURLV2 string defaultAccessTokenLifetime time.Duration defaultIdTokenLifetime time.Duration + jwksCacheControlMaxAge time.Duration fallbackLogger *slog.Logger hasher *crypto.Hasher @@ -129,13 +130,6 @@ func (s *Server) Discovery(ctx context.Context, r *op.Request[struct{}]) (_ *op. return op.NewResponse(s.createDiscoveryConfig(ctx, allowedLanguages)), nil } -func (s *Server) Keys(ctx context.Context, r *op.Request[struct{}]) (_ *op.Response, err error) { - ctx, span := tracing.NewSpan(ctx) - defer func() { span.EndWithError(err) }() - - return s.LegacyServer.Keys(ctx, r) -} - func (s *Server) VerifyAuthRequest(ctx context.Context, r *op.Request[oidc.AuthRequest]) (_ *op.ClientRequest[oidc.AuthRequest], err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -173,6 +167,7 @@ func (s *Server) EndSession(ctx context.Context, r *op.Request[oidc.EndSessionRe func (s *Server) createDiscoveryConfig(ctx context.Context, supportedUILocales oidc.Locales) *oidc.DiscoveryConfiguration { issuer := op.IssuerFromContext(ctx) + return &oidc.DiscoveryConfiguration{ Issuer: issuer, AuthorizationEndpoint: s.Endpoints().Authorization.Absolute(issuer), @@ -192,7 +187,7 @@ func (s *Server) createDiscoveryConfig(ctx context.Context, supportedUILocales o }, GrantTypesSupported: op.GrantTypes(s.Provider()), SubjectTypesSupported: op.SubjectTypes(s.Provider()), - IDTokenSigningAlgValuesSupported: []string{s.signingKeyAlgorithm}, + IDTokenSigningAlgValuesSupported: supportedSigningAlgs(ctx), RequestObjectSigningAlgValuesSupported: op.RequestObjectSigAlgorithms(s.Provider()), TokenEndpointAuthMethodsSupported: op.AuthMethodsTokenEndpoint(s.Provider()), TokenEndpointAuthSigningAlgValuesSupported: op.TokenSigAlgorithms(s.Provider()), diff --git a/internal/api/oidc/server_test.go b/internal/api/oidc/server_test.go index 19404933b..47f24fcde 100644 --- a/internal/api/oidc/server_test.go +++ b/internal/api/oidc/server_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/zitadel/oidc/v3/pkg/oidc" "github.com/zitadel/oidc/v3/pkg/op" + "github.com/zitadel/zitadel/internal/api/authz" + "github.com/zitadel/zitadel/internal/feature" "golang.org/x/text/language" ) @@ -30,6 +32,7 @@ func TestServer_createDiscoveryConfig(t *testing.T) { fields{ LegacyServer: op.NewLegacyServer( func() *op.Provider { + //nolint:staticcheck provider, _ := op.NewForwardedOpenIDProvider("path", &op.Config{ CodeMethodS256: true, @@ -107,6 +110,92 @@ func TestServer_createDiscoveryConfig(t *testing.T) { OPTermsOfServiceURI: "", }, }, + { + "web keys feature enabled", + fields{ + LegacyServer: op.NewLegacyServer( + func() *op.Provider { + //nolint:staticcheck + provider, _ := op.NewForwardedOpenIDProvider("path", + &op.Config{ + CodeMethodS256: true, + AuthMethodPost: true, + AuthMethodPrivateKeyJWT: true, + GrantTypeRefreshToken: true, + RequestObjectSupported: true, + }, + nil, + ) + return provider + }(), + op.Endpoints{ + Authorization: op.NewEndpoint("auth"), + Token: op.NewEndpoint("token"), + Introspection: op.NewEndpoint("introspect"), + Userinfo: op.NewEndpoint("userinfo"), + Revocation: op.NewEndpoint("revoke"), + EndSession: op.NewEndpoint("logout"), + JwksURI: op.NewEndpoint("keys"), + DeviceAuthorization: op.NewEndpoint("device"), + }, + ), + signingKeyAlgorithm: "RS256", + }, + args{ + ctx: authz.WithFeatures( + op.ContextWithIssuer(context.Background(), "https://issuer.com"), + feature.Features{WebKey: true}, + ), + supportedUILocales: []language.Tag{language.English, language.German}, + }, + &oidc.DiscoveryConfiguration{ + Issuer: "https://issuer.com", + AuthorizationEndpoint: "https://issuer.com/auth", + TokenEndpoint: "https://issuer.com/token", + IntrospectionEndpoint: "https://issuer.com/introspect", + UserinfoEndpoint: "https://issuer.com/userinfo", + RevocationEndpoint: "https://issuer.com/revoke", + EndSessionEndpoint: "https://issuer.com/logout", + DeviceAuthorizationEndpoint: "https://issuer.com/device", + CheckSessionIframe: "", + JwksURI: "https://issuer.com/keys", + RegistrationEndpoint: "", + ScopesSupported: []string{oidc.ScopeOpenID, oidc.ScopeProfile, oidc.ScopeEmail, oidc.ScopePhone, oidc.ScopeAddress, oidc.ScopeOfflineAccess}, + ResponseTypesSupported: []string{string(oidc.ResponseTypeCode), string(oidc.ResponseTypeIDTokenOnly), string(oidc.ResponseTypeIDToken)}, + ResponseModesSupported: []string{string(oidc.ResponseModeQuery), string(oidc.ResponseModeFragment), string(oidc.ResponseModeFormPost)}, + GrantTypesSupported: []oidc.GrantType{oidc.GrantTypeCode, oidc.GrantTypeImplicit, oidc.GrantTypeRefreshToken, oidc.GrantTypeBearer}, + ACRValuesSupported: nil, + SubjectTypesSupported: []string{"public"}, + IDTokenSigningAlgValuesSupported: supportedWebKeyAlgs, + IDTokenEncryptionAlgValuesSupported: nil, + IDTokenEncryptionEncValuesSupported: nil, + UserinfoSigningAlgValuesSupported: nil, + UserinfoEncryptionAlgValuesSupported: nil, + UserinfoEncryptionEncValuesSupported: nil, + RequestObjectSigningAlgValuesSupported: []string{"RS256"}, + RequestObjectEncryptionAlgValuesSupported: nil, + RequestObjectEncryptionEncValuesSupported: nil, + TokenEndpointAuthMethodsSupported: []oidc.AuthMethod{oidc.AuthMethodNone, oidc.AuthMethodBasic, oidc.AuthMethodPost, oidc.AuthMethodPrivateKeyJWT}, + TokenEndpointAuthSigningAlgValuesSupported: []string{"RS256"}, + RevocationEndpointAuthMethodsSupported: []oidc.AuthMethod{oidc.AuthMethodNone, oidc.AuthMethodBasic, oidc.AuthMethodPost, oidc.AuthMethodPrivateKeyJWT}, + RevocationEndpointAuthSigningAlgValuesSupported: []string{"RS256"}, + IntrospectionEndpointAuthMethodsSupported: []oidc.AuthMethod{oidc.AuthMethodBasic, oidc.AuthMethodPrivateKeyJWT}, + IntrospectionEndpointAuthSigningAlgValuesSupported: []string{"RS256"}, + DisplayValuesSupported: nil, + ClaimTypesSupported: nil, + ClaimsSupported: []string{"sub", "aud", "exp", "iat", "iss", "auth_time", "nonce", "acr", "amr", "c_hash", "at_hash", "act", "scopes", "client_id", "azp", "preferred_username", "name", "family_name", "given_name", "locale", "email", "email_verified", "phone_number", "phone_number_verified"}, + ClaimsParameterSupported: false, + CodeChallengeMethodsSupported: []oidc.CodeChallengeMethod{"S256"}, + ServiceDocumentation: "", + ClaimsLocalesSupported: nil, + UILocalesSupported: []language.Tag{language.English, language.German}, + RequestParameterSupported: true, + RequestURIParameterSupported: false, + RequireRequestURIRegistration: false, + OPPolicyURI: "", + OPTermsOfServiceURI: "", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/api/oidc/token.go b/internal/api/oidc/token.go index be3a30ed7..56ed22590 100644 --- a/internal/api/oidc/token.go +++ b/internal/api/oidc/token.go @@ -12,9 +12,11 @@ import ( "github.com/zitadel/oidc/v3/pkg/oidc" "github.com/zitadel/oidc/v3/pkg/op" + "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/telemetry/tracing" + "github.com/zitadel/zitadel/internal/zerrors" ) /* @@ -75,22 +77,43 @@ func (s *Server) getSignerOnce() signerFunc { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() + if authz.GetFeatures(ctx).WebKey { + var webKey *jose.JSONWebKey + webKey, err = s.query.GetActiveSigningWebKey(ctx) + if err != nil { + return + } + signer, signAlg, err = signerFromWebKey(webKey) + return + } + var signingKey op.SigningKey signingKey, err = s.Provider().Storage().SigningKey(ctx) if err != nil { return } signAlg = signingKey.SignatureAlgorithm() - signer, err = op.SignerFromKey(signingKey) - if err != nil { - return - } }) return signer, signAlg, err } } +func signerFromWebKey(signingKey *jose.JSONWebKey) (jose.Signer, jose.SignatureAlgorithm, error) { + signAlg := jose.SignatureAlgorithm(signingKey.Algorithm) + signer, err := jose.NewSigner( + jose.SigningKey{ + Algorithm: signAlg, + Key: signingKey, + }, + (&jose.SignerOptions{}).WithType("JWT"), + ) + if err != nil { + return nil, "", zerrors.ThrowInternal(err, "OIDC-oaF0s", "Errors.Internal") + } + return signer, signAlg, nil +} + // userInfoFunc is a getter function that allows add-hoc retrieval of a user. type userInfoFunc func(ctx context.Context, roleAssertion bool, triggerType domain.TriggerType) (*oidc.UserInfo, error) diff --git a/internal/api/oidc/userinfo_integration_test.go b/internal/api/oidc/userinfo_integration_test.go index ad95defd6..e93807807 100644 --- a/internal/api/oidc/userinfo_integration_test.go +++ b/internal/api/oidc/userinfo_integration_test.go @@ -36,6 +36,7 @@ func TestServer_UserInfo(t *testing.T) { name string legacy bool trigger bool + webKey bool }{ { name: "legacy enabled", @@ -51,6 +52,17 @@ func TestServer_UserInfo(t *testing.T) { legacy: false, trigger: true, }, + + // This is the only functional test we need to cover web keys. + // - By creating tokens the signer is tested + // - When obtaining the tokens, the RP verifies the ID Token using the key set from the jwks endpoint. + // - By calling userinfo with the access token as JWT, the Token Verifier with the public key cache is tested. + { + name: "web keys", + legacy: false, + trigger: false, + webKey: true, + }, } for _, tt := range tests { @@ -58,6 +70,7 @@ func TestServer_UserInfo(t *testing.T) { _, err := Tester.Client.FeatureV2.SetInstanceFeatures(iamOwnerCTX, &feature.SetInstanceFeaturesRequest{ OidcLegacyIntrospection: &tt.legacy, OidcTriggerIntrospectionProjections: &tt.trigger, + WebKey: &tt.webKey, }) require.NoError(t, err) testServer_UserInfo(t) diff --git a/internal/integration/config/zitadel.yaml b/internal/integration/config/zitadel.yaml index 291099ea1..d58529b7a 100644 --- a/internal/integration/config/zitadel.yaml +++ b/internal/integration/config/zitadel.yaml @@ -53,4 +53,13 @@ SystemAPIUsers: KeyData: "LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlJQklqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FROEFNSUlCQ2dLQ0FRRUF6aStGRlNKTDdmNXl3NEtUd3pnTQpQMzRlUEd5Y20vTStrVDBNN1Y0Q2d4NVYzRWFESXZUUUtUTGZCYUVCNDV6YjlMdGpJWHpEdzByWFJvUzJoTzZ0CmgrQ1lRQ3ozS0N2aDA5QzBJenhaaUIySVMzSC9hVCs1Qng5RUZZK3ZuQWtaamNjYnlHNVlOUnZtdE9sbnZJZUkKSDdxWjB0RXdrUGZGNUdFWk5QSlB0bXkzVUdWN2lvZmRWUVMxeFJqNzMrYU13NXJ2SDREOElkeWlBQzNWZWtJYgpwdDBWajBTVVgzRHdLdG9nMzM3QnpUaVBrM2FYUkYwc2JGaFFvcWRKUkk4TnFnWmpDd2pxOXlmSTV0eXhZc3duCitKR3pIR2RIdlczaWRPRGxtd0V0NUsycGFzaVJJV0syT0dmcSt3MEVjbHRRSGFidXFFUGdabG1oQ2tSZE5maXgKQndJREFRQUIKLS0tLS1FTkQgUFVCTElDIEtFWS0tLS0tCg==" InitProjections: - Enabled: true \ No newline at end of file + Enabled: true + +# Extend key lifetimes so we do not see more legacy keys when +# integration tests are rerun on the same DB with more than 6 hours apart. +# The test counts the amount of keys returned from the JWKS endpoint and fails +# with 2 or more legacy public keys, +SystemDefaults: + KeyConfig: + PrivateKeyLifetime: 7200h + PublicKeyLifetime: 14400h diff --git a/proto/zitadel/resources/webkey/v3alpha/webkey_service.proto b/proto/zitadel/resources/webkey/v3alpha/webkey_service.proto index 8cf6c379d..c9783d3b0 100644 --- a/proto/zitadel/resources/webkey/v3alpha/webkey_service.proto +++ b/proto/zitadel/resources/webkey/v3alpha/webkey_service.proto @@ -147,7 +147,7 @@ service ZITADELWebKeys { option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = { summary: "Activate a signing key for the instance"; - description: "Switch the active signing web key. The previously active key will be deactivated." + description: "Switch the active signing web key. The previously active key will be deactivated. Note that the JWKs OIDC endpoint returns a cacheable response. Therefore it is not advised to activate a key that has been created within the cache duration (default is 5min), as the public key may not have been propagated to caches and clients yet." responses: { key: "200" value: {