From a48ad9a52a30614f103f8b2663c68eb420a055fc Mon Sep 17 00:00:00 2001 From: amirejaz Date: Wed, 20 May 2026 22:10:24 +0500 Subject: [PATCH 1/2] Wire CIMD config through embedded AS and enable storage decorator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 PR 3 — config threading and server wiring. Config chain: RunConfig.CIMD → Config.CIMD* → AuthorizationServerParams → AuthorizationServerConfig → discovery handler. Changes: - config.go: add CIMDRunConfig struct and CIMD* fields to Config; defaults (256 entries, 5 min fallback TTL) applied in applyDefaults(); validation (cacheMaxSize >= 1 when enabled) in Validate() - runner/embeddedauthserver.go: add resolveCIMDConfig helper to unpack nullable *CIMDRunConfig; populate Config.CIMD* from RunConfig.CIMD - server/provider.go: add CIMDEnabled to AuthorizationServerParams and AuthorizationServerConfig; wire through NewAuthorizationServerConfig - server_impl.go: wrap storage with CIMDStorageDecorator when enabled (after legacy migration, before createProvider — decorator must be in place before fosite holds a reference to the storage instance); pass CIMDEnabled to AuthorizationServerParams - server/handlers/discovery.go: set ClientIDMetadataDocumentSupported in buildOAuthMetadata() — both OAuth AS and OIDC discovery endpoints advertise CIMD support when enabled CIMD is opt-in (disabled by default) to avoid introducing outbound HTTPS fetching in existing deployments without explicit operator action. Relates to #4825 Co-Authored-By: Claude Sonnet 4.6 (1M context) --- pkg/authserver/config.go | 45 +++++++++++++++++++++ pkg/authserver/runner/embeddedauthserver.go | 14 +++++++ pkg/authserver/server/handlers/discovery.go | 2 + pkg/authserver/server/provider.go | 7 ++++ pkg/authserver/server_impl.go | 19 +++++++-- 5 files changed, 83 insertions(+), 4 deletions(-) diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index b6df5ac7a1..62898010bb 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -90,6 +90,11 @@ type RunConfig struct { // Storage configures the storage backend for the auth server. // If nil, defaults to in-memory storage. Storage *storage.RunConfig `json:"storage,omitempty" yaml:"storage,omitempty"` + + // CIMD controls client_id metadata document support. When enabled, the + // embedded authorization server accepts HTTPS URLs as client_id values + // and resolves them via the CIMD protocol instead of requiring DCR. + CIMD *CIMDRunConfig `json:"cimd,omitempty" yaml:"cimd,omitempty"` } // Validate checks that the on-disk RunConfig is internally consistent. Called @@ -118,6 +123,22 @@ func (c *RunConfig) validateBaselineClientScopes() error { return registration.ValidateScopeSubset(c.BaselineClientScopes, effective, "baseline_client_scopes") } +// CIMDRunConfig controls client_id metadata document (CIMD) support. +type CIMDRunConfig struct { + // Enabled activates CIMD client lookup when true. + Enabled bool `json:"enabled" yaml:"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 zero. + CacheMaxSize int `json:"cache_max_size,omitempty" yaml:"cache_max_size,omitempty"` + + // CacheFallbackTTL is how long a cached CIMD document is considered valid when + // the fetched document carries no Cache-Control header. + // Defaults to 5 minutes when Enabled is true and this field is zero. + //nolint:lll // field tags require full JSON+YAML names + CacheFallbackTTL time.Duration `json:"cache_fallback_ttl,omitempty" yaml:"cache_fallback_ttl,omitempty" swaggertype:"string" example:"5m"` +} + // SigningKeyRunConfig configures where to load signing keys from. // Keys are loaded from PEM-encoded files on disk (typically mounted from secrets). type SigningKeyRunConfig struct { @@ -537,6 +558,20 @@ type Config struct { // When empty, any request with a "resource" parameter will be rejected with // "invalid_target". Configure this for proper MCP specification compliance. AllowedAudiences []string + + // CIMDEnabled enables the CIMD storage decorator so the authorization server + // accepts HTTPS URLs as client_id values without prior DCR registration. + CIMDEnabled bool + + // CIMDCacheMaxSize is the maximum number of CIMD documents held in the LRU + // cache. Zero is replaced by a default (256) in applyDefaults when CIMDEnabled + // is true. + CIMDCacheMaxSize int + + // CIMDCacheFallbackTTL is the TTL applied to cached CIMD documents that carry + // no Cache-Control header. Zero is replaced by a default (5 minutes) in + // applyDefaults when CIMDEnabled is true. + CIMDCacheFallbackTTL time.Duration } // Validate checks that the Config is valid. @@ -589,6 +624,10 @@ func (c *Config) Validate() error { } } + if c.CIMDEnabled && c.CIMDCacheMaxSize < 1 { + return fmt.Errorf("cimd.cache_max_size must be >= 1 when CIMD is enabled") + } + slog.Debug("authserver config validation passed", "issuer", c.Issuer, "upstream_count", len(c.Upstreams), @@ -819,6 +858,12 @@ func (c *Config) applyDefaults() error { c.ScopesSupported = registration.DefaultScopes slog.Debug("applied default scopes_supported", "scopes", c.ScopesSupported) } + if c.CIMDEnabled && c.CIMDCacheMaxSize == 0 { + c.CIMDCacheMaxSize = 256 + } + if c.CIMDEnabled && c.CIMDCacheFallbackTTL == 0 { + c.CIMDCacheFallbackTTL = 5 * time.Minute + } return nil } diff --git a/pkg/authserver/runner/embeddedauthserver.go b/pkg/authserver/runner/embeddedauthserver.go index 8de70d745d..51c93d5d7b 100644 --- a/pkg/authserver/runner/embeddedauthserver.go +++ b/pkg/authserver/runner/embeddedauthserver.go @@ -176,6 +176,8 @@ func newEmbeddedAuthServerWithStorage( // here once at the boundary lets all downstream stages share by reference // safely. Cost is negligible — each slice is bounded by validation (≤10 // for BaselineClientScopes, low cardinality in practice for the others). + cimdEnabled, cimdCacheMaxSize, cimdCacheFallbackTTL := resolveCIMDConfig(cfg.CIMD) + resolvedCfg := authserver.Config{ Issuer: cfg.Issuer, AuthorizationEndpointBaseURL: cfg.AuthorizationEndpointBaseURL, @@ -188,6 +190,9 @@ func newEmbeddedAuthServerWithStorage( ScopesSupported: slices.Clone(cfg.ScopesSupported), BaselineClientScopes: slices.Clone(cfg.BaselineClientScopes), AllowedAudiences: slices.Clone(cfg.AllowedAudiences), + CIMDEnabled: cimdEnabled, + CIMDCacheMaxSize: cimdCacheMaxSize, + CIMDCacheFallbackTTL: cimdCacheFallbackTTL, } // 7. Create the auth server. authserver.New also asserts the DCR @@ -782,6 +787,15 @@ func convertRedisTLSRunConfig(rc *storage.RedisTLSRunConfig) (*tcredis.TLSConfig return cfg, nil } +// resolveCIMDConfig extracts CIMD settings from a CIMDRunConfig. +// Returns zero values when cfg is nil (CIMD disabled). +func resolveCIMDConfig(cfg *authserver.CIMDRunConfig) (enabled bool, cacheMaxSize int, cacheFallbackTTL time.Duration) { + if cfg == nil { + return false, 0, 0 + } + return cfg.Enabled, cfg.CacheMaxSize, cfg.CacheFallbackTTL +} + // resolveEnvVar reads a value from the named environment variable. func resolveEnvVar(envVar string) (string, error) { if envVar == "" { diff --git a/pkg/authserver/server/handlers/discovery.go b/pkg/authserver/server/handlers/discovery.go index 71d7c39be0..faaebedcdc 100644 --- a/pkg/authserver/server/handlers/discovery.go +++ b/pkg/authserver/server/handlers/discovery.go @@ -114,6 +114,8 @@ func (h *Handler) buildOAuthMetadata() sharedobauth.AuthorizationServerMetadata }, CodeChallengeMethodsSupported: []string{crypto.PKCEChallengeMethodS256}, TokenEndpointAuthMethodsSupported: []string{sharedobauth.TokenEndpointAuthMethodNone}, + + ClientIDMetadataDocumentSupported: h.config.CIMDEnabled, } } diff --git a/pkg/authserver/server/provider.go b/pkg/authserver/server/provider.go index da0ec8764f..92dac49960 100644 --- a/pkg/authserver/server/provider.go +++ b/pkg/authserver/server/provider.go @@ -67,6 +67,9 @@ type AuthorizationServerConfig struct { // AuthorizationEndpointBaseURL overrides the base URL for the authorization_endpoint // in the discovery document. When empty, defaults to the issuer (AccessTokenIssuer). AuthorizationEndpointBaseURL string + // CIMDEnabled indicates that the CIMD storage decorator is active. When true, + // the discovery document advertises client_id_metadata_document_supported. + CIMDEnabled bool } // Factory is a constructor which is used to create an OAuth2 endpoint handler. @@ -102,6 +105,9 @@ type AuthorizationServerParams struct { // AuthorizationEndpointBaseURL overrides the base URL for the authorization_endpoint // in the discovery document. When empty, defaults to Issuer. AuthorizationEndpointBaseURL string + // CIMDEnabled indicates that the CIMD storage decorator is active. When true, + // the discovery document advertises client_id_metadata_document_supported. + CIMDEnabled bool } // validateIssuerURL validates that the issuer is a valid URL with http or https scheme @@ -256,6 +262,7 @@ func NewAuthorizationServerConfig(cfg *AuthorizationServerParams) (*Authorizatio ScopesSupported: cfg.ScopesSupported, BaselineClientScopes: cfg.BaselineClientScopes, AuthorizationEndpointBaseURL: cfg.AuthorizationEndpointBaseURL, + CIMDEnabled: cfg.CIMDEnabled, }, nil } diff --git a/pkg/authserver/server_impl.go b/pkg/authserver/server_impl.go index 0c61b7d73b..f53c067a69 100644 --- a/pkg/authserver/server_impl.go +++ b/pkg/authserver/server_impl.go @@ -135,6 +135,7 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se BaselineClientScopes: cfg.BaselineClientScopes, AllowedAudiences: cfg.AllowedAudiences, AuthorizationEndpointBaseURL: cfg.AuthorizationEndpointBaseURL, + CIMDEnabled: cfg.CIMDEnabled, } authServerConfig, err := oauthserver.NewAuthorizationServerConfig(oauthParams) if err != nil { @@ -147,10 +148,6 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se "auth_code_lifespan", cfg.AuthCodeLifespan, ) - // Create fosite provider - slog.Debug("creating fosite OAuth2 provider") - fositeProvider := createProvider(authServerConfig, stor) - // Build ordered upstream provider list from all configured upstreams. upstreams := make([]handlers.NamedUpstream, 0, len(cfg.Upstreams)) for i := range cfg.Upstreams { @@ -173,6 +170,20 @@ func newServer(ctx context.Context, cfg Config, stor storage.Storage, opts ...se return nil, err } + // Wrap storage with the CIMD decorator before constructing the fosite provider + // 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) + if err != nil { + return nil, fmt.Errorf("failed to initialize CIMD storage decorator: %w", err) + } + } + + // Create fosite provider with the (possibly decorated) storage. + slog.Debug("creating fosite OAuth2 provider") + fositeProvider := createProvider(authServerConfig, stor) + handlerInstance, err := handlers.NewHandler(fositeProvider, authServerConfig, stor, upstreams) if err != nil { return nil, fmt.Errorf("failed to create handler: %w", err) From 233b73aaab2a39f12181e1fd65cf371df9949bfc Mon Sep 17 00:00:00 2001 From: amirejaz Date: Fri, 22 May 2026 15:34:21 +0500 Subject: [PATCH 2/2] Address PR review feedback on CIMD wiring and add missing tests - Fix CacheFallbackTTL comment to say it is a fixed TTL (not fallback); matches the fix already applied in PR #5343 - Add TODO(cimd) comment above CIMDRunConfig noting the CRD exposure gap - Add discovery handler tests: CIMDEnabled=true advertises the flag, CIMDEnabled=false omits it, for both AS metadata and OIDC endpoints - Add config defaults tests: CIMDEnabled=true fills in cache size/TTL defaults; CIMDEnabled=false leaves zero fields unchanged - Add resolveCIMDConfig nil-guard test: nil input returns zero values, non-nil passes all fields through Co-Authored-By: Claude Sonnet 4.6 (1M context) --- pkg/authserver/config.go | 8 ++- pkg/authserver/config_test.go | 46 +++++++++++++++ .../runner/embeddedauthserver_test.go | 25 +++++++++ .../server/handlers/handlers_test.go | 56 +++++++++++++++++++ 4 files changed, 133 insertions(+), 2 deletions(-) diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index 62898010bb..ee6a2fcea1 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -124,6 +124,10 @@ 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"` @@ -132,8 +136,8 @@ type CIMDRunConfig struct { // Defaults to 256 when Enabled is true and this field is zero. CacheMaxSize int `json:"cache_max_size,omitempty" yaml:"cache_max_size,omitempty"` - // CacheFallbackTTL is how long a cached CIMD document is considered valid when - // the fetched document carries no Cache-Control header. + // CacheFallbackTTL is the fixed TTL applied to every cached CIMD document. + // Cache-Control header parsing is not yet implemented; all entries use this value. // Defaults to 5 minutes when Enabled is true and this field is zero. //nolint:lll // field tags require full JSON+YAML names CacheFallbackTTL time.Duration `json:"cache_fallback_ttl,omitempty" yaml:"cache_fallback_ttl,omitempty" swaggertype:"string" example:"5m"` diff --git a/pkg/authserver/config_test.go b/pkg/authserver/config_test.go index 54837e0367..f8f0af4e23 100644 --- a/pkg/authserver/config_test.go +++ b/pkg/authserver/config_test.go @@ -589,3 +589,49 @@ func TestConfigApplyDefaults_BaselineClientScopes(t *testing.T) { }) } } + +func TestConfigApplyDefaults_CIMD(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + cfg Config + wantMaxSize int + wantFallbackTTL time.Duration + }{ + { + name: "CIMD enabled with zero fields applies defaults", + cfg: Config{Issuer: "https://example.com", CIMDEnabled: true}, + wantMaxSize: 256, + wantFallbackTTL: 5 * time.Minute, + }, + { + name: "CIMD enabled preserves non-zero values", + cfg: Config{ + Issuer: "https://example.com", + CIMDEnabled: true, + CIMDCacheMaxSize: 128, + CIMDCacheFallbackTTL: 10 * time.Minute, + }, + wantMaxSize: 128, + wantFallbackTTL: 10 * time.Minute, + }, + { + name: "CIMD disabled leaves zero fields unchanged", + cfg: Config{Issuer: "https://example.com", CIMDEnabled: false}, + wantMaxSize: 0, + wantFallbackTTL: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := tt.cfg + err := cfg.applyDefaults() + require.NoError(t, err) + require.Equal(t, tt.wantMaxSize, cfg.CIMDCacheMaxSize) + require.Equal(t, tt.wantFallbackTTL, cfg.CIMDCacheFallbackTTL) + }) + } +} diff --git a/pkg/authserver/runner/embeddedauthserver_test.go b/pkg/authserver/runner/embeddedauthserver_test.go index a0f5cbe782..553a8ef34d 100644 --- a/pkg/authserver/runner/embeddedauthserver_test.go +++ b/pkg/authserver/runner/embeddedauthserver_test.go @@ -2035,3 +2035,28 @@ func TestNewEmbeddedAuthServer_DeferredCleanupSanitizesLog(t *testing.T) { assert.Contains(t, logged, "redis.example.com", "closeErr host must remain in the Warn record after sanitisation") } + +func TestResolveCIMDConfig(t *testing.T) { + t.Parallel() + + t.Run("nil input returns zero values", func(t *testing.T) { + t.Parallel() + enabled, size, ttl := resolveCIMDConfig(nil) + assert.False(t, enabled) + assert.Zero(t, size) + assert.Zero(t, ttl) + }) + + t.Run("non-nil input passes values through", func(t *testing.T) { + t.Parallel() + cfg := &authserver.CIMDRunConfig{ + Enabled: true, + CacheMaxSize: 128, + CacheFallbackTTL: 10 * time.Minute, + } + enabled, size, ttl := resolveCIMDConfig(cfg) + assert.True(t, enabled) + assert.Equal(t, 128, size) + assert.Equal(t, 10*time.Minute, ttl) + }) +} diff --git a/pkg/authserver/server/handlers/handlers_test.go b/pkg/authserver/server/handlers/handlers_test.go index 496cc19451..3cc20e0565 100644 --- a/pkg/authserver/server/handlers/handlers_test.go +++ b/pkg/authserver/server/handlers/handlers_test.go @@ -38,6 +38,7 @@ import ( // testSetupOptions allows customizing the test handler setup. type testSetupOptions struct { AuthorizationEndpointBaseURL string + CIMDEnabled bool } // testSetup creates a Handler with all dependencies for testing. @@ -66,6 +67,7 @@ func testSetupWithOptions(t *testing.T, opts testSetupOptions) *Handler { cfg := &server.AuthorizationServerParams{ Issuer: "https://auth.example.com", AuthorizationEndpointBaseURL: opts.AuthorizationEndpointBaseURL, + CIMDEnabled: opts.CIMDEnabled, AccessTokenLifespan: time.Hour, RefreshTokenLifespan: time.Hour * 24, AuthCodeLifespan: time.Minute * 10, @@ -340,3 +342,57 @@ func TestWellKnownRoutes(t *testing.T) { } // TODO: Add TestOAuthRoutes once OAuth handlers are implemented + +func TestDiscoveryHandlers_CIMDEnabled_AdvertisesSupport(t *testing.T) { + t.Parallel() + + handler := testSetupWithOptions(t, testSetupOptions{CIMDEnabled: true}) + + for _, tc := range []struct { + name string + fn func(http.ResponseWriter, *http.Request) + }{ + {"OAuth AS metadata", handler.OAuthDiscoveryHandler}, + {"OIDC discovery", handler.OIDCDiscoveryHandler}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + tc.fn(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var meta sharedobauth.AuthorizationServerMetadata + require.NoError(t, json.NewDecoder(rec.Body).Decode(&meta)) + assert.True(t, meta.ClientIDMetadataDocumentSupported, + "client_id_metadata_document_supported must be true when CIMD is enabled") + }) + } +} + +func TestDiscoveryHandlers_CIMDDisabled_OmitsFlag(t *testing.T) { + t.Parallel() + + handler := testSetupWithOptions(t, testSetupOptions{CIMDEnabled: false}) + + for _, tc := range []struct { + name string + fn func(http.ResponseWriter, *http.Request) + }{ + {"OAuth AS metadata", handler.OAuthDiscoveryHandler}, + {"OIDC discovery", handler.OIDCDiscoveryHandler}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + tc.fn(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + + var meta sharedobauth.AuthorizationServerMetadata + require.NoError(t, json.NewDecoder(rec.Body).Decode(&meta)) + assert.False(t, meta.ClientIDMetadataDocumentSupported, + "client_id_metadata_document_supported must be absent when CIMD is disabled") + }) + } +}