From e9bd7d7bac49692a814e4f89e01f572eb3a263a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Fri, 26 Jan 2024 17:44:50 +0200 Subject: [PATCH] feat(op): split the access and ID token hint verifiers (#525) * feat(op): split the access and ID token hint verifiers In zitadel we require different behaviors wrt public key expiry between access tokens and ID token hints. This change splits the two verifiers in the OP. The default is still based on Storage and passed to both verifier fields. * add new options to tests --- pkg/op/op.go | 59 ++++++++++++++++++++++++++--------------------- pkg/op/op_test.go | 8 +++++-- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/pkg/op/op.go b/pkg/op/op.go index fdc073c2..14c5356d 100644 --- a/pkg/op/op.go +++ b/pkg/op/op.go @@ -257,13 +257,16 @@ func NewForwardedOpenIDProvider(path string, config *Config, storage Storage, op // op.AuthCallbackURL(provider) which is probably /callback. On the redirect back // to the AuthCallbackURL, the request id should be passed as the "id" parameter. func NewProvider(config *Config, storage Storage, issuer func(insecure bool) (IssuerFromRequest, error), opOpts ...Option) (_ *Provider, err error) { + keySet := &OpenIDKeySet{storage} o := &Provider{ - config: config, - storage: storage, - endpoints: DefaultEndpoints, - timer: make(<-chan time.Time), - corsOpts: &defaultCORSOptions, - logger: slog.Default(), + config: config, + storage: storage, + accessTokenKeySet: keySet, + idTokenHinKeySet: keySet, + endpoints: DefaultEndpoints, + timer: make(<-chan time.Time), + corsOpts: &defaultCORSOptions, + logger: slog.Default(), } for _, optFunc := range opOpts { @@ -276,19 +279,11 @@ func NewProvider(config *Config, storage Storage, issuer func(insecure bool) (Is if err != nil { return nil, err } - o.Handler = CreateRouter(o, o.interceptors...) - o.decoder = schema.NewDecoder() o.decoder.IgnoreUnknownKeys(true) - o.encoder = oidc.NewEncoder() - o.crypto = NewAESCrypto(config.CryptoKey) - - // Avoid potential race conditions by calling these early - _ = o.openIDKeySet() // sets keySet - return o, nil } @@ -299,7 +294,8 @@ type Provider struct { insecure bool endpoints *Endpoints storage Storage - keySet *openIDKeySet + accessTokenKeySet oidc.KeySet + idTokenHinKeySet oidc.KeySet crypto Crypto decoder *schema.Decoder encoder *schema.Encoder @@ -435,7 +431,7 @@ func (o *Provider) Encoder() httphelper.Encoder { } func (o *Provider) IDTokenHintVerifier(ctx context.Context) *IDTokenHintVerifier { - return NewIDTokenHintVerifier(IssuerFromContext(ctx), o.openIDKeySet(), o.idTokenHintVerifierOpts...) + return NewIDTokenHintVerifier(IssuerFromContext(ctx), o.idTokenHinKeySet, o.idTokenHintVerifierOpts...) } func (o *Provider) JWTProfileVerifier(ctx context.Context) *JWTProfileVerifier { @@ -443,14 +439,7 @@ func (o *Provider) JWTProfileVerifier(ctx context.Context) *JWTProfileVerifier { } func (o *Provider) AccessTokenVerifier(ctx context.Context) *AccessTokenVerifier { - return NewAccessTokenVerifier(IssuerFromContext(ctx), o.openIDKeySet(), o.accessTokenVerifierOpts...) -} - -func (o *Provider) openIDKeySet() oidc.KeySet { - if o.keySet == nil { - o.keySet = &openIDKeySet{o.Storage()} - } - return o.keySet + return NewAccessTokenVerifier(IssuerFromContext(ctx), o.accessTokenKeySet, o.accessTokenVerifierOpts...) } func (o *Provider) Crypto() Crypto { @@ -480,13 +469,13 @@ func (o *Provider) HttpHandler() http.Handler { return o } -type openIDKeySet struct { +type OpenIDKeySet struct { Storage } // VerifySignature implements the oidc.KeySet interface // providing an implementation for the keys stored in the OP Storage interface -func (o *openIDKeySet) VerifySignature(ctx context.Context, jws *jose.JSONWebSignature) ([]byte, error) { +func (o *OpenIDKeySet) VerifySignature(ctx context.Context, jws *jose.JSONWebSignature) ([]byte, error) { keySet, err := o.Storage.KeySet(ctx) if err != nil { return nil, fmt.Errorf("error fetching keys: %w", err) @@ -617,6 +606,15 @@ func WithHttpInterceptors(interceptors ...HttpInterceptor) Option { } } +// WithAccessTokenKeySet allows passing a KeySet with public keys for Access Token verification. +// The default KeySet uses the [Storage] interface +func WithAccessTokenKeySet(keySet oidc.KeySet) Option { + return func(o *Provider) error { + o.accessTokenKeySet = keySet + return nil + } +} + func WithAccessTokenVerifierOpts(opts ...AccessTokenVerifierOpt) Option { return func(o *Provider) error { o.accessTokenVerifierOpts = opts @@ -624,6 +622,15 @@ func WithAccessTokenVerifierOpts(opts ...AccessTokenVerifierOpt) Option { } } +// WithIDTokenHintKeySet allows passing a KeySet with public keys for ID Token Hint verification. +// The default KeySet uses the [Storage] interface. +func WithIDTokenHintKeySet(keySet oidc.KeySet) Option { + return func(o *Provider) error { + o.idTokenHinKeySet = keySet + return nil + } +} + func WithIDTokenHintVerifierOpts(opts ...IDTokenHintVerifierOpt) Option { return func(o *Provider) error { o.idTokenHintVerifierOpts = opts diff --git a/pkg/op/op_test.go b/pkg/op/op_test.go index f97f666c..b2a758c9 100644 --- a/pkg/op/op_test.go +++ b/pkg/op/op_test.go @@ -58,8 +58,12 @@ func init() { } func newTestProvider(config *op.Config) op.OpenIDProvider { - provider, err := op.NewOpenIDProvider(testIssuer, config, - storage.NewStorage(storage.NewUserStore(testIssuer)), op.WithAllowInsecure(), + storage := storage.NewStorage(storage.NewUserStore(testIssuer)) + keySet := &op.OpenIDKeySet{storage} + provider, err := op.NewOpenIDProvider(testIssuer, config, storage, + op.WithAllowInsecure(), + op.WithAccessTokenKeySet(keySet), + op.WithIDTokenHintKeySet(keySet), ) if err != nil { panic(err)