diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index 25bd4f603f..44698f1e4c 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -299,6 +299,10 @@ type EmbeddedAuthServerConfig struct { // +listType=atomic // +optional BaselineClientScopes []string `json:"baselineClientScopes,omitempty"` + + // CIMD configures Client ID Metadata Document support. When omitted, CIMD is disabled. + // +optional + CIMD *EmbeddedAuthServerCIMDConfig `json:"cimd,omitempty"` } // TokenLifespanConfig holds configuration for token lifetimes. @@ -325,6 +329,31 @@ type TokenLifespanConfig struct { AuthCodeLifespan string `json:"authCodeLifespan,omitempty"` } +// EmbeddedAuthServerCIMDConfig configures Client ID Metadata Document (CIMD) support +// on the embedded authorization server. When enabled, the AS accepts HTTPS URLs as +// client_id values and resolves them via the CIMD protocol, allowing clients such as +// VS Code to authenticate without prior Dynamic Client Registration. +type EmbeddedAuthServerCIMDConfig struct { + // Enabled activates CIMD client lookup. When false (the default), the AS only + // accepts client_id values that were registered via DCR. + // +kubebuilder:default=false + Enabled bool `json:"enabled"` + + // CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + // Defaults to 256 when Enabled is true and this field is omitted. + // +kubebuilder:validation:Minimum=1 + // +optional + CacheMaxSize int `json:"cacheMaxSize,omitempty"` + + // CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + // Cache-Control header parsing is not yet implemented; all entries use this value. + // Format: Go duration string (e.g. "5m", "10m", "1h"). + // Defaults to 5 minutes when Enabled is true and this field is omitted. + // +kubebuilder:validation:Pattern=`^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$` + // +optional + CacheFallbackTTL string `json:"cacheFallbackTtl,omitempty"` +} + // UpstreamProviderType identifies the type of upstream Identity Provider. type UpstreamProviderType string diff --git a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go index 07077737b5..cbfcf8cf8d 100644 --- a/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1beta1/zz_generated.deepcopy.go @@ -225,6 +225,21 @@ func (in *DCRUpstreamConfig) DeepCopy() *DCRUpstreamConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EmbeddedAuthServerCIMDConfig) DeepCopyInto(out *EmbeddedAuthServerCIMDConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EmbeddedAuthServerCIMDConfig. +func (in *EmbeddedAuthServerCIMDConfig) DeepCopy() *EmbeddedAuthServerCIMDConfig { + if in == nil { + return nil + } + out := new(EmbeddedAuthServerCIMDConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EmbeddedAuthServerConfig) DeepCopyInto(out *EmbeddedAuthServerConfig) { *out = *in @@ -260,6 +275,11 @@ func (in *EmbeddedAuthServerConfig) DeepCopyInto(out *EmbeddedAuthServerConfig) *out = make([]string, len(*in)) copy(*out, *in) } + if in.CIMD != nil { + in, out := &in.CIMD, &out.CIMD + *out = new(EmbeddedAuthServerCIMDConfig) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EmbeddedAuthServerConfig. diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 346c4d87ad..84d4ed979d 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -582,6 +582,16 @@ func BuildAuthServerRunConfig( } config.Storage = storageCfg + // Build CIMD configuration. CacheFallbackTTL is passed as-is (string); + // resolveCIMDConfig in the runner parses it to time.Duration at startup. + if authConfig.CIMD != nil && authConfig.CIMD.Enabled { + config.CIMD = &authserver.CIMDRunConfig{ + Enabled: authConfig.CIMD.Enabled, + CacheMaxSize: authConfig.CIMD.CacheMaxSize, + CacheFallbackTTL: authConfig.CIMD.CacheFallbackTTL, + } + } + return config, nil } diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index fcb0bd6379..bec4c98179 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -2645,3 +2645,132 @@ func TestValidateAndAddAuthServerRefOptions(t *testing.T) { }) } } + +// TestBuildAuthServerRunConfig_CIMD verifies that BuildAuthServerRunConfig +// correctly converts the CRD EmbeddedAuthServerCIMDConfig into +// authserver.CIMDRunConfig. The four cases cover the nil path (CIMD off +// by default), explicit values (fields are mapped and TTL is parsed), zero +// optional fields (authserver applies its own defaults at startup), and an +// invalid TTL string (returns a parse error). +func TestBuildAuthServerRunConfig_CIMD(t *testing.T) { + t.Parallel() + + // baseAuthConfig returns a minimal EmbeddedAuthServerConfig that is valid + // enough for BuildAuthServerRunConfig to proceed past signing-key and + // upstream validation without requiring real secrets. + baseAuthConfig := func(cimd *mcpv1beta1.EmbeddedAuthServerCIMDConfig) *mcpv1beta1.EmbeddedAuthServerConfig { + return &mcpv1beta1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1beta1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1beta1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + CIMD: cimd, + } + } + + defaultAudiences := []string{"https://mcp.example.com"} + defaultScopes := []string{"openid", "offline_access"} + + tests := []struct { + name string + cimd *mcpv1beta1.EmbeddedAuthServerCIMDConfig + wantCIMD bool + wantErr bool + errContains string + checkFunc func(t *testing.T, got *authserver.CIMDRunConfig) + }{ + { + name: "nil CIMD leaves config.CIMD nil", + cimd: nil, + wantCIMD: false, + }, + { + name: "CIMD disabled leaves config.CIMD nil", + cimd: &mcpv1beta1.EmbeddedAuthServerCIMDConfig{ + Enabled: false, + CacheMaxSize: 100, + CacheFallbackTTL: "10m", + }, + wantCIMD: false, + }, + { + name: "CIMD enabled with explicit values maps all fields", + cimd: &mcpv1beta1.EmbeddedAuthServerCIMDConfig{ + Enabled: true, + CacheMaxSize: 512, + CacheFallbackTTL: "10m", + }, + wantCIMD: true, + checkFunc: func(t *testing.T, got *authserver.CIMDRunConfig) { + t.Helper() + assert.True(t, got.Enabled) + assert.Equal(t, 512, got.CacheMaxSize) + assert.Equal(t, "10m", got.CacheFallbackTTL) + }, + }, + { + name: "CIMD enabled with zero optional fields leaves defaults to authserver", + cimd: &mcpv1beta1.EmbeddedAuthServerCIMDConfig{ + Enabled: true, + }, + wantCIMD: true, + checkFunc: func(t *testing.T, got *authserver.CIMDRunConfig) { + t.Helper() + assert.True(t, got.Enabled) + assert.Zero(t, got.CacheMaxSize, "zero means authserver applies its own default at startup") + assert.Zero(t, got.CacheFallbackTTL, "zero means authserver applies its own default at startup") + }, + }, + { + name: "invalid CacheFallbackTTL passes through to runner for validation", + cimd: &mcpv1beta1.EmbeddedAuthServerCIMDConfig{ + Enabled: true, + CacheFallbackTTL: "not-a-duration", + }, + wantCIMD: true, + checkFunc: func(t *testing.T, got *authserver.CIMDRunConfig) { + t.Helper() + // The converter passes the string through; parse errors are caught + // by CIMDRunConfig.Validate() or resolveCIMDConfig in the runner. + assert.Equal(t, "not-a-duration", got.CacheFallbackTTL) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cfg, err := BuildAuthServerRunConfig( + "default", "test-server", + baseAuthConfig(tt.cimd), + defaultAudiences, defaultScopes, + "https://mcp.example.com", + ) + + if tt.wantErr { + require.Error(t, err) + if tt.errContains != "" { + assert.Contains(t, err.Error(), tt.errContains) + } + return + } + + require.NoError(t, err) + require.NotNil(t, cfg) + + if !tt.wantCIMD { + assert.Nil(t, cfg.CIMD, "expected config.CIMD to be nil") + return + } + + require.NotNil(t, cfg.CIMD, "expected config.CIMD to be set") + if tt.checkFunc != nil { + tt.checkFunc(t, cfg.CIMD) + } + }) + } +} diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 61b2c43c46..4218a7ffe0 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -237,6 +237,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing @@ -1516,6 +1543,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index 4f639ef1ee..887443f812 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -110,6 +110,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing @@ -3034,6 +3061,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index d657a82b07..43edce708f 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -240,6 +240,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing @@ -1519,6 +1546,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index cedf9c2334..baba421153 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -113,6 +113,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing @@ -3037,6 +3064,33 @@ spec: maxItems: 10 type: array x-kubernetes-list-type: atomic + cimd: + description: CIMD configures Client ID Metadata Document support. + When omitted, CIMD is disabled. + properties: + cacheFallbackTtl: + description: |- + CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + Cache-Control header parsing is not yet implemented; all entries use this value. + Format: Go duration string (e.g. "5m", "10m", "1h"). + Defaults to 5 minutes when Enabled is true and this field is omitted. + pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$ + type: string + cacheMaxSize: + description: |- + CacheMaxSize is the maximum number of CIMD documents held in the LRU cache. + Defaults to 256 when Enabled is true and this field is omitted. + minimum: 1 + type: integer + enabled: + default: false + description: |- + Enabled activates CIMD client lookup. When false (the default), the AS only + accepts client_id values that were registered via DCR. + type: boolean + required: + - enabled + type: object hmacSecretRefs: description: |- HMACSecretRefs references Kubernetes Secrets containing symmetric secrets for signing diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index 43bc2c2116..e83be618fc 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -129,10 +129,6 @@ func (c *RunConfig) validateBaselineClientScopes() error { } // CIMDRunConfig controls client_id metadata document (CIMD) support. -// -// TODO(cimd): expose these fields in the MCPExternalAuthConfig CRD so Kubernetes -// operators can configure CIMD through the normal CRD workflow instead of -// writing RunConfig YAML directly. type CIMDRunConfig struct { // Enabled activates CIMD client lookup when true. Enabled bool `json:"enabled" yaml:"enabled"` diff --git a/pkg/authserver/server_impl.go b/pkg/authserver/server_impl.go index 39a22468ca..91932395c2 100644 --- a/pkg/authserver/server_impl.go +++ b/pkg/authserver/server_impl.go @@ -177,7 +177,7 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se // so that GetClient calls for HTTPS client_id values are intercepted at the // fosite level (not just the handler level). if cfg.CIMDEnabled { - stor, err = storage.NewCIMDStorageDecorator(stor, true, cfg.CIMDCacheMaxSize, cfg.CIMDCacheFallbackTTL) + stor, err = storage.NewCIMDStorageDecorator(stor, true, cfg.CIMDCacheMaxSize, cfg.CIMDCacheFallbackTTL, cfg.ScopesSupported) if err != nil { return nil, fmt.Errorf("failed to initialize CIMD storage decorator: %w", err) } diff --git a/pkg/authserver/storage/cimd_decorator.go b/pkg/authserver/storage/cimd_decorator.go index fbead1ef50..95c812304d 100644 --- a/pkg/authserver/storage/cimd_decorator.go +++ b/pkg/authserver/storage/cimd_decorator.go @@ -28,10 +28,11 @@ import ( // Only GetClient is overridden. DCR clients (opaque IDs) continue to work // exactly as before. type CIMDStorageDecorator struct { - Storage // embed full interface — all methods delegate - sf singleflight.Group // deduplicates concurrent fetches for the same URL - cache *lru.Cache[string, *cimdCacheEntry] - ttl time.Duration + Storage // embed full interface — all methods delegate + sf singleflight.Group // deduplicates concurrent fetches for the same URL + cache *lru.Cache[string, *cimdCacheEntry] + ttl time.Duration + scopesSupported []string // AS-configured scopes; nil means accept any } type cimdCacheEntry struct { @@ -43,11 +44,15 @@ type cimdCacheEntry struct { // it returns base unchanged (no allocation). cacheMaxSize must be >= 1; // fallbackTTL is the fixed TTL applied to every cache entry (Cache-Control // header parsing is not yet implemented; all entries use this value). +// scopesSupported is the AS-configured scope allowlist; documents that declare +// scopes outside this set are rejected at fetch time. Pass nil to skip scope +// validation (e.g. when ScopesSupported is unset and DefaultScopes applies). func NewCIMDStorageDecorator( base Storage, enabled bool, cacheMaxSize int, fallbackTTL time.Duration, + scopesSupported []string, ) (Storage, error) { if !enabled { return base, nil @@ -63,9 +68,10 @@ func NewCIMDStorageDecorator( } return &CIMDStorageDecorator{ - Storage: base, - cache: c, - ttl: fallbackTTL, + Storage: base, + cache: c, + ttl: fallbackTTL, + scopesSupported: scopesSupported, }, nil } @@ -129,7 +135,49 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli id, m, defaultCIMDTokenEndpointAuthMethod) } - client := buildFositeClient(doc) + // Reject documents that declare grant_types the embedded AS does not support. + // Consistent with DCR which restricts public clients to authorization_code + refresh_token. + allowedGrantTypes := map[string]bool{"authorization_code": true, "refresh_token": true} + for _, gt := range doc.GrantTypes { + if !allowedGrantTypes[gt] { + return nil, fmt.Errorf("%w: CIMD document at %s claims grant_type %q "+ + "but this server only supports %v for public clients", + fosite.ErrNotFound.WithHint("unsupported grant_type"), + id, gt, defaultCIMDGrantTypes) + } + } + + // Reject documents that declare response_types the embedded AS does not support. + allowedResponseTypes := map[string]bool{"code": true} + for _, rt := range doc.ResponseTypes { + if !allowedResponseTypes[rt] { + return nil, fmt.Errorf("%w: CIMD document at %s claims response_type %q "+ + "but this server only supports %v", + fosite.ErrNotFound.WithHint("unsupported response_type"), + id, rt, defaultCIMDResponseTypes) + } + } + + // Compute and validate the client scope list consistent with DCR. + // When ScopesSupported is configured: use registration.ValidateScopes which + // validates each declared scope against the allowlist and falls back to + // DefaultScopes (also validated) when the document omits the field — the + // same logic the DCR handler applies. + // When ScopesSupported is not configured: no AS-level validation; use the + // declared scopes directly, or nil to let buildFositeClient apply DefaultScopes. + var resolvedScopes []string + if len(d.scopesSupported) > 0 { + computed, dcrErr := registration.ValidateScopes(strings.Fields(doc.Scope), d.scopesSupported) + if dcrErr != nil { + return nil, fmt.Errorf("%w: CIMD document at %s: %s", + fosite.ErrNotFound.WithHint(string(dcrErr.Error)), id, dcrErr.ErrorDescription) + } + resolvedScopes = computed + } else if doc.Scope != "" { + resolvedScopes = strings.Fields(doc.Scope) + } + + client := buildFositeClient(doc, resolvedScopes) d.cache.Add(id, &cimdCacheEntry{ client: client, @@ -157,7 +205,9 @@ const defaultCIMDTokenEndpointAuthMethod = "none" // buildFositeClient converts a ClientMetadataDocument into a fosite.Client. // Redirect URIs containing http://localhost are wrapped in a LoopbackClient // so that RFC 8252 §7.3 dynamic port matching applies. -func buildFositeClient(doc *cimd.ClientMetadataDocument) fosite.Client { +// resolvedScopes is the already-validated scope list computed by fetch() via +// registration.ValidateScopes; when nil, DefaultScopes is used (unconstrained AS). +func buildFositeClient(doc *cimd.ClientMetadataDocument, resolvedScopes []string) fosite.Client { grantTypes := doc.GrantTypes if len(grantTypes) == 0 { grantTypes = defaultCIMDGrantTypes @@ -173,13 +223,12 @@ func buildFositeClient(doc *cimd.ClientMetadataDocument) fosite.Client { tokenEndpointAuthMethod = defaultCIMDTokenEndpointAuthMethod } - // When the document omits the scope field, apply the same defaults as DCR - // registration so CIMD clients can request openid/profile/email/offline_access - // without needing to enumerate them explicitly in the metadata document. - // Clone to avoid aliasing the package-level DefaultScopes slice. - scopes := slices.Clone(registration.DefaultScopes) - if doc.Scope != "" { - scopes = strings.Fields(doc.Scope) + // Scopes were computed and validated by fetch() via registration.ValidateScopes, + // consistent with the DCR handler. Fall back to DefaultScopes only when the + // decorator has no ScopesSupported restriction (unconstrained AS). + scopes := resolvedScopes + if len(scopes) == 0 { + scopes = slices.Clone(registration.DefaultScopes) } defaultClient := &fosite.DefaultClient{ diff --git a/pkg/authserver/storage/cimd_decorator_test.go b/pkg/authserver/storage/cimd_decorator_test.go index 99c3d66dc8..202cde45b9 100644 --- a/pkg/authserver/storage/cimd_decorator_test.go +++ b/pkg/authserver/storage/cimd_decorator_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "sync" "sync/atomic" "testing" @@ -62,7 +63,7 @@ func newTestBase(t *testing.T) *MemoryStorage { // newEnabledDecorator creates a CIMDStorageDecorator wrapping base. func newEnabledDecorator(t *testing.T, base *MemoryStorage, maxSize int, ttl time.Duration) *CIMDStorageDecorator { t.Helper() - got, err := NewCIMDStorageDecorator(base, true, maxSize, ttl) + got, err := NewCIMDStorageDecorator(base, true, maxSize, ttl, nil) require.NoError(t, err) return got.(*CIMDStorageDecorator) } @@ -77,7 +78,7 @@ func cimdURL(srv *httptest.Server, path string) string { func TestNewCIMDStorageDecorator_DisabledReturnsBase(t *testing.T) { t.Parallel() base := newTestBase(t) - got, err := NewCIMDStorageDecorator(base, false, 10, time.Minute) + got, err := NewCIMDStorageDecorator(base, false, 10, time.Minute, nil) require.NoError(t, err) assert.Same(t, base, got, "disabled decorator must return base unchanged") } @@ -85,21 +86,21 @@ func TestNewCIMDStorageDecorator_DisabledReturnsBase(t *testing.T) { func TestNewCIMDStorageDecorator_ZeroCacheSizeReturnsError(t *testing.T) { t.Parallel() base := newTestBase(t) - _, err := NewCIMDStorageDecorator(base, true, 0, time.Minute) + _, err := NewCIMDStorageDecorator(base, true, 0, time.Minute, nil) require.Error(t, err) } func TestNewCIMDStorageDecorator_NegativeCacheSizeReturnsError(t *testing.T) { t.Parallel() base := newTestBase(t) - _, err := NewCIMDStorageDecorator(base, true, -1, time.Minute) + _, err := NewCIMDStorageDecorator(base, true, -1, time.Minute, nil) require.Error(t, err) } func TestNewCIMDStorageDecorator_EnabledReturnsCIMDDecorator(t *testing.T) { t.Parallel() base := newTestBase(t) - got, err := NewCIMDStorageDecorator(base, true, 10, time.Minute) + got, err := NewCIMDStorageDecorator(base, true, 10, time.Minute, nil) require.NoError(t, err) require.NotNil(t, got) _, isCIMD := got.(*CIMDStorageDecorator) @@ -336,7 +337,7 @@ func TestBuildFositeClient_Defaults(t *testing.T) { RedirectURIs: []string{"https://example.com/callback"}, } - got := buildFositeClient(doc) + got := buildFositeClient(doc, nil) assert.Equal(t, "https://example.com/meta.json", got.GetID()) assert.True(t, got.IsPublic()) assert.ElementsMatch(t, []string{"authorization_code", "refresh_token"}, []string(got.GetGrantTypes())) @@ -355,7 +356,7 @@ func TestBuildFositeClient_ExplicitGrantTypes(t *testing.T) { GrantTypes: []string{"authorization_code"}, } - got := buildFositeClient(doc) + got := buildFositeClient(doc, nil) assert.ElementsMatch(t, []string{"authorization_code"}, []string(got.GetGrantTypes())) } @@ -368,7 +369,9 @@ func TestBuildFositeClient_ScopeParsing(t *testing.T) { Scope: "openid profile email", } - got := buildFositeClient(doc) + // Scope parsing is now done by fetch() before calling buildFositeClient. + resolvedScopes := strings.Fields(doc.Scope) + got := buildFositeClient(doc, resolvedScopes) assert.ElementsMatch(t, []string{"openid", "profile", "email"}, []string(got.GetScopes())) } @@ -380,7 +383,7 @@ func TestBuildFositeClient_LoopbackRedirectWrapsInLoopbackClient(t *testing.T) { RedirectURIs: []string{"http://localhost/callback"}, } - got := buildFositeClient(doc) + got := buildFositeClient(doc, nil) // LoopbackClient adds MatchRedirectURI — check the distinctive method is present. type loopbackMatcher interface { MatchRedirectURI(string) bool @@ -403,7 +406,7 @@ func TestBuildFositeClient_NonLoopbackRedirectReturnsOpenIDConnectClient(t *test RedirectURIs: []string{"https://example.com/callback"}, } - got := buildFositeClient(doc) + got := buildFositeClient(doc, nil) _, ok := got.(*fosite.DefaultOpenIDConnectClient) assert.True(t, ok, "non-loopback redirect URI must produce a DefaultOpenIDConnectClient") } @@ -416,7 +419,7 @@ func TestBuildFositeClient_TokenEndpointAuthMethodDefault(t *testing.T) { RedirectURIs: []string{"https://example.com/callback"}, } - got := buildFositeClient(doc) + got := buildFositeClient(doc, nil) if oidc, ok := got.(fosite.OpenIDConnectClient); ok { assert.Equal(t, "none", oidc.GetTokenEndpointAuthMethod()) } @@ -442,3 +445,127 @@ func TestFetch_RejectsUnsupportedTokenEndpointAuthMethod(t *testing.T) { _, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json") require.Error(t, err, "fetch must fail when token_endpoint_auth_method is not \"none\"") } + +// --- C4: grant_types / response_types validation --- + +func TestFetch_RejectsUnsupportedGrantType(t *testing.T) { + t.Parallel() + for _, unsupported := range []string{"client_credentials", "implicit", "urn:ietf:params:oauth:grant-type:device_code"} { + t.Run(unsupported, func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + clientID := "http://" + r.Host + r.URL.Path + doc := cimd.ClientMetadataDocument{ + ClientID: clientID, + RedirectURIs: []string{"https://example.com/callback"}, + GrantTypes: []string{unsupported}, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(doc) + })) + t.Cleanup(srv.Close) + dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute) + _, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json") + require.Error(t, err, "unsupported grant_type %q must be rejected", unsupported) + }) + } +} + +func TestFetch_AcceptsSupportedGrantTypes(t *testing.T) { + t.Parallel() + srv := serveCIMDDoc(t, "/meta.json", nil) + dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute) + // Default grant_types (omitted in document) must succeed + _, err := dec.fetchOrCached(context.Background(), cimdURL(srv, "/meta.json")) + require.NoError(t, err) +} + +func TestFetch_RejectsUnsupportedResponseType(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + clientID := "http://" + r.Host + r.URL.Path + doc := cimd.ClientMetadataDocument{ + ClientID: clientID, + RedirectURIs: []string{"https://example.com/callback"}, + ResponseTypes: []string{"token"}, + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(doc) + })) + t.Cleanup(srv.Close) + dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute) + _, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json") + require.Error(t, err, "unsupported response_type \"token\" must be rejected") +} + +// --- C3: scope validation against ScopesSupported --- + +func TestBuildFositeClient_ScopeDefaultsToScopesSupported(t *testing.T) { + t.Parallel() + doc := &cimd.ClientMetadataDocument{ + ClientID: "https://example.com/meta.json", + RedirectURIs: []string{"https://example.com/callback"}, + // Scope deliberately omitted + } + scopesSupported := []string{"openid", "profile"} + got := buildFositeClient(doc, scopesSupported) + assert.ElementsMatch(t, scopesSupported, []string(got.GetScopes()), + "omitted scope should default to ScopesSupported, not DefaultScopes") +} + +func TestBuildFositeClient_ScopeDefaultsToDefaultScopesWhenNoScopesSupported(t *testing.T) { + t.Parallel() + doc := &cimd.ClientMetadataDocument{ + ClientID: "https://example.com/meta.json", + RedirectURIs: []string{"https://example.com/callback"}, + } + got := buildFositeClient(doc, nil) + assert.ElementsMatch(t, registration.DefaultScopes, []string(got.GetScopes()), + "omitted scope with no ScopesSupported should default to registration.DefaultScopes") +} + +func TestFetch_RejectsScopeOutsideScopesSupported(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + clientID := "http://" + r.Host + r.URL.Path + doc := cimd.ClientMetadataDocument{ + ClientID: clientID, + RedirectURIs: []string{"https://example.com/callback"}, + Scope: "openid profile email", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(doc) + })) + t.Cleanup(srv.Close) + + // Decorator configured with scopesSupported=["openid"] only + got, err := NewCIMDStorageDecorator(newTestBase(t), true, 10, time.Minute, []string{"openid"}) + require.NoError(t, err) + dec := got.(*CIMDStorageDecorator) + + _, err = dec.fetchOrCached(context.Background(), srv.URL+"/meta.json") + require.Error(t, err, "scope outside ScopesSupported must be rejected") +} + +func TestFetch_AcceptsScopeWithinScopesSupported(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + clientID := "http://" + r.Host + r.URL.Path + doc := cimd.ClientMetadataDocument{ + ClientID: clientID, + RedirectURIs: []string{"https://example.com/callback"}, + Scope: "openid", + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(doc) + })) + t.Cleanup(srv.Close) + + got, err := NewCIMDStorageDecorator(newTestBase(t), true, 10, time.Minute, []string{"openid", "profile"}) + require.NoError(t, err) + dec := got.(*CIMDStorageDecorator) + + client, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json") + require.NoError(t, err) + assert.ElementsMatch(t, []string{"openid"}, []string(client.GetScopes())) +}