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
zitadel · Jan 26, 2024 · e9bd7d7 · e9bd7d7
1 parent 437a049
commit e9bd7d7
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 28 deletions.
diff --git 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,22 +431,15 @@ 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 {
 	return NewJWTProfileVerifier(o.Storage(), IssuerFromContext(ctx), 1*time.Hour, time.Second)
 }
 
 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,13 +606,31 @@ 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
 		return nil
 	}
 }
 
+// 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
@@ -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)