Skip to content

Commit 8fbc935

Browse files
amirejazclaude
andauthored
Wire CIMD config through embedded AS and enable storage decorator (#5348)
* Wire CIMD config through embedded AS and enable storage decorator 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> * Extend IsClientIDMetadataDocumentURL to accept http://localhost URLs The embedded AS must resolve CIMD client_ids for local development and testing the same way FetchClientMetadataDocument does — accepting http://localhost and http://127.x.x.x in addition to https://. This brings IsClientIDMetadataDocumentURL in line with validateCIMDClientURL in pkg/oauthproto/cimd/fetch.go which already permits these schemes. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Regenerate swagger docs to include CIMDRunConfig Run task docs to pick up the new CIMDRunConfig struct added in this PR. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Default to registration.DefaultScopes when CIMD doc omits scope field When a CIMD document does not include a scope field, buildFositeClient was producing a client with no allowed scopes. Any authorization request specifying openid, profile, or offline_access would then fail with "scope not allowed". DCR registration applies DefaultScopes in the same situation, so CIMD clients should behave identically. Found during manual testing with VS Code as a CIMD client. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Trigger CI re-run * Address PR review feedback on CIMD wiring - Fix IsClientIDMetadataDocumentURL comment: accepted loopbacks are localhost/127.0.0.1/[::1], not the full 127.x.x.x range - Fix hostFromURL doc: returns host+port, not host without port - Add loopback and subdomain-bypass test cases to TestIsClientIDMetadataDocumentURL (Claude blocking issue) - Fix Config.CIMDCacheFallbackTTL comment: Cache-Control is not parsed - Add validation: CIMDCacheFallbackTTL must be non-negative when enabled - Clone registration.DefaultScopes in buildFositeClient to prevent aliasing the package-level slice - Fix CIMDDisabled discovery test to decode to map[string]interface{} so it verifies the field is truly absent (omitempty) not just false The DefaultScopes change in cimd_decorator.go was introduced here (rather than in PR #5343) because the gap was discovered during manual testing of the wiring: VS Code's CIMD document omits scope and the nil scopes caused an auth failure. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Use net/url to extract host in IsClientIDMetadataDocumentURL net/url is already imported by four other files in pkg/oauthproto, so the previous comment about keeping the package "import-free" was incorrect. url.Parse handles IPv6 brackets and other edge cases correctly without manual string manipulation. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> * Address tgrunnagle feedback on CIMD wiring C1 - Fix CacheFallbackTTL to string in CIMDRunConfig so YAML/JSON operators can write "5m" instead of nanoseconds. resolveCIMDConfig parses it with time.ParseDuration at runtime. C6 - Add slog.Debug when applyDefaults substitutes CIMD defaults, matching the pattern used by the sibling token-lifespan defaults. C7 - Add CIMDRunConfig.Validate() and call it from RunConfig.Validate() so CIMD misconfiguration fails at the wire-format boundary before the runner resolves secrets. C10 - Add test cases to TestConfigValidate and TestRunConfigValidate covering all new CIMD validation paths. C11 - Add TestNewServer_CIMDEnabled_WrapsStorage: verifies the CIMD decorator is actually installed in the storage chain when CIMDEnabled=true. C12 - Add comment in discovery.go explaining why client_id_metadata_document_supported is in OIDC discovery. C13 - Add doc comment on server.storage noting it may be decorated. C14 - Add note to IsClientIDMetadataDocumentURL about pkg/auth/remote callers and why the loopback widening is safe there. Regenerate swagger docs: CacheFallbackTTL now serialises as string. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 394b2e7 commit 8fbc935

16 files changed

Lines changed: 439 additions & 13 deletions

docs/server/docs.go

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.json

Lines changed: 22 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/server/swagger.yaml

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/authserver/config.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,23 @@ type RunConfig struct {
9090
// Storage configures the storage backend for the auth server.
9191
// If nil, defaults to in-memory storage.
9292
Storage *storage.RunConfig `json:"storage,omitempty" yaml:"storage,omitempty"`
93+
94+
// CIMD controls client_id metadata document support. When enabled, the
95+
// embedded authorization server accepts HTTPS URLs as client_id values
96+
// and resolves them via the CIMD protocol instead of requiring DCR.
97+
CIMD *CIMDRunConfig `json:"cimd,omitempty" yaml:"cimd,omitempty"`
9398
}
9499

95100
// Validate checks that the on-disk RunConfig is internally consistent. Called
96101
// by the runner before resolving secrets and building the runtime Config; it
97102
// catches operator-supplied misconfiguration early so server startup fails
98103
// loudly instead of degrading silently at runtime.
99104
func (c *RunConfig) Validate() error {
105+
if c.CIMD != nil {
106+
if err := c.CIMD.Validate(); err != nil {
107+
return fmt.Errorf("cimd: %w", err)
108+
}
109+
}
100110
return c.validateBaselineClientScopes()
101111
}
102112

@@ -118,6 +128,46 @@ func (c *RunConfig) validateBaselineClientScopes() error {
118128
return registration.ValidateScopeSubset(c.BaselineClientScopes, effective, "baseline_client_scopes")
119129
}
120130

131+
// CIMDRunConfig controls client_id metadata document (CIMD) support.
132+
//
133+
// TODO(cimd): expose these fields in the MCPExternalAuthConfig CRD so Kubernetes
134+
// operators can configure CIMD through the normal CRD workflow instead of
135+
// writing RunConfig YAML directly.
136+
type CIMDRunConfig struct {
137+
// Enabled activates CIMD client lookup when true.
138+
Enabled bool `json:"enabled" yaml:"enabled"`
139+
140+
// CacheMaxSize is the maximum number of CIMD documents held in the LRU cache.
141+
// Defaults to 256 when Enabled is true and this field is zero.
142+
CacheMaxSize int `json:"cache_max_size,omitempty" yaml:"cache_max_size,omitempty"`
143+
144+
// CacheFallbackTTL is the fixed TTL applied to every cached CIMD document.
145+
// Cache-Control header parsing is not yet implemented; all entries use this value.
146+
// Format: Go duration string (e.g. "5m", "10m", "1h").
147+
// Defaults to 5 minutes when Enabled is true and this field is omitted.
148+
CacheFallbackTTL string `json:"cache_fallback_ttl,omitempty" yaml:"cache_fallback_ttl,omitempty" example:"5m"`
149+
}
150+
151+
// Validate checks that the CIMDRunConfig fields are internally consistent.
152+
func (c *CIMDRunConfig) Validate() error {
153+
if !c.Enabled {
154+
return nil
155+
}
156+
if c.CacheMaxSize < 0 {
157+
return fmt.Errorf("cache_max_size must be non-negative when CIMD is enabled, got %d", c.CacheMaxSize)
158+
}
159+
if c.CacheFallbackTTL != "" {
160+
d, err := time.ParseDuration(c.CacheFallbackTTL)
161+
if err != nil {
162+
return fmt.Errorf("cache_fallback_ttl: %w", err)
163+
}
164+
if d < 0 {
165+
return fmt.Errorf("cache_fallback_ttl must be non-negative when CIMD is enabled, got %s", c.CacheFallbackTTL)
166+
}
167+
}
168+
return nil
169+
}
170+
121171
// SigningKeyRunConfig configures where to load signing keys from.
122172
// Keys are loaded from PEM-encoded files on disk (typically mounted from secrets).
123173
type SigningKeyRunConfig struct {
@@ -537,6 +587,20 @@ type Config struct {
537587
// When empty, any request with a "resource" parameter will be rejected with
538588
// "invalid_target". Configure this for proper MCP specification compliance.
539589
AllowedAudiences []string
590+
591+
// CIMDEnabled enables the CIMD storage decorator so the authorization server
592+
// accepts HTTPS URLs as client_id values without prior DCR registration.
593+
CIMDEnabled bool
594+
595+
// CIMDCacheMaxSize is the maximum number of CIMD documents held in the LRU
596+
// cache. Zero is replaced by a default (256) in applyDefaults when CIMDEnabled
597+
// is true.
598+
CIMDCacheMaxSize int
599+
600+
// CIMDCacheFallbackTTL is the fixed TTL applied to all cached CIMD documents
601+
// (Cache-Control header parsing is not yet implemented). Zero is replaced by
602+
// a default (5 minutes) in applyDefaults when CIMDEnabled is true.
603+
CIMDCacheFallbackTTL time.Duration
540604
}
541605

542606
// Validate checks that the Config is valid.
@@ -589,6 +653,13 @@ func (c *Config) Validate() error {
589653
}
590654
}
591655

656+
if c.CIMDEnabled && c.CIMDCacheMaxSize < 1 {
657+
return fmt.Errorf("cimd.cache_max_size must be >= 1 when CIMD is enabled")
658+
}
659+
if c.CIMDEnabled && c.CIMDCacheFallbackTTL < 0 {
660+
return fmt.Errorf("cimd.cache_fallback_ttl must be non-negative when CIMD is enabled")
661+
}
662+
592663
slog.Debug("authserver config validation passed",
593664
"issuer", c.Issuer,
594665
"upstream_count", len(c.Upstreams),
@@ -819,6 +890,14 @@ func (c *Config) applyDefaults() error {
819890
c.ScopesSupported = registration.DefaultScopes
820891
slog.Debug("applied default scopes_supported", "scopes", c.ScopesSupported)
821892
}
893+
if c.CIMDEnabled && c.CIMDCacheMaxSize == 0 {
894+
c.CIMDCacheMaxSize = 256
895+
slog.Debug("applied default cimd cache_max_size", "size", c.CIMDCacheMaxSize)
896+
}
897+
if c.CIMDEnabled && c.CIMDCacheFallbackTTL == 0 {
898+
c.CIMDCacheFallbackTTL = 5 * time.Minute
899+
slog.Debug("applied default cimd cache_fallback_ttl", "ttl", c.CIMDCacheFallbackTTL)
900+
}
822901
return nil
823902
}
824903

pkg/authserver/config_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ func TestConfigValidate(t *testing.T) {
114114
{name: "baseline scope not in scopes_supported", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, ScopesSupported: []string{"openid"}, BaselineClientScopes: []string{"offline_access"}}, wantErr: true, errMsg: `baseline_client_scopes contains "offline_access"`},
115115
{name: "nil supported with baseline in DefaultScopes passes", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, ScopesSupported: nil, BaselineClientScopes: []string{"offline_access"}}},
116116

117+
// CIMD validation
118+
{name: "CIMD enabled zero cache_max_size rejected", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, CIMDEnabled: true, CIMDCacheMaxSize: 0}, wantErr: true, errMsg: "cache_max_size must be >= 1"},
119+
{name: "CIMD enabled negative cache_max_size rejected", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, CIMDEnabled: true, CIMDCacheMaxSize: -1}, wantErr: true, errMsg: "cache_max_size must be >= 1"},
120+
{name: "CIMD enabled negative cache_fallback_ttl rejected", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, CIMDEnabled: true, CIMDCacheMaxSize: 256, CIMDCacheFallbackTTL: -time.Second}, wantErr: true, errMsg: "cache_fallback_ttl must be non-negative"},
121+
{name: "CIMD disabled ignores invalid cache fields", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, CIMDEnabled: false, CIMDCacheMaxSize: -1, CIMDCacheFallbackTTL: -time.Second}},
122+
{name: "CIMD enabled with valid bounds passes", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}, CIMDEnabled: true, CIMDCacheMaxSize: 256, CIMDCacheFallbackTTL: 5 * time.Minute}},
123+
117124
// Valid configs
118125
{name: "valid minimal", config: Config{Issuer: "https://example.com", KeyProvider: validKeyProvider, HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}}},
119126
{name: "valid nil key provider", config: Config{Issuer: "https://example.com", HMACSecrets: validHMAC, Upstreams: validUpstreams, AllowedAudiences: []string{"https://mcp.example.com"}}},
@@ -431,6 +438,14 @@ func TestRunConfigValidate(t *testing.T) {
431438
wantErr: true,
432439
errMsg: "foo",
433440
},
441+
// CIMD RunConfig validation
442+
{name: "CIMD nil passes", config: RunConfig{CIMD: nil}},
443+
{name: "CIMD disabled passes even with invalid fields", config: RunConfig{CIMD: &CIMDRunConfig{Enabled: false, CacheMaxSize: -1, CacheFallbackTTL: "-1s"}}},
444+
{name: "CIMD enabled negative cache_max_size rejected", config: RunConfig{CIMD: &CIMDRunConfig{Enabled: true, CacheMaxSize: -1}}, wantErr: true, errMsg: "cache_max_size"},
445+
{name: "CIMD enabled invalid TTL string rejected", config: RunConfig{CIMD: &CIMDRunConfig{Enabled: true, CacheFallbackTTL: "not-a-duration"}}, wantErr: true, errMsg: "cache_fallback_ttl"},
446+
{name: "CIMD enabled negative TTL rejected", config: RunConfig{CIMD: &CIMDRunConfig{Enabled: true, CacheFallbackTTL: "-5m"}}, wantErr: true, errMsg: "cache_fallback_ttl"},
447+
{name: "CIMD enabled valid passes", config: RunConfig{CIMD: &CIMDRunConfig{Enabled: true, CacheMaxSize: 64, CacheFallbackTTL: "5m"}}},
448+
{name: "CIMD enabled omitted optional fields pass", config: RunConfig{CIMD: &CIMDRunConfig{Enabled: true}}},
434449
}
435450

436451
for _, tt := range tests {
@@ -589,3 +604,49 @@ func TestConfigApplyDefaults_BaselineClientScopes(t *testing.T) {
589604
})
590605
}
591606
}
607+
608+
func TestConfigApplyDefaults_CIMD(t *testing.T) {
609+
t.Parallel()
610+
611+
tests := []struct {
612+
name string
613+
cfg Config
614+
wantMaxSize int
615+
wantFallbackTTL time.Duration
616+
}{
617+
{
618+
name: "CIMD enabled with zero fields applies defaults",
619+
cfg: Config{Issuer: "https://example.com", CIMDEnabled: true},
620+
wantMaxSize: 256,
621+
wantFallbackTTL: 5 * time.Minute,
622+
},
623+
{
624+
name: "CIMD enabled preserves non-zero values",
625+
cfg: Config{
626+
Issuer: "https://example.com",
627+
CIMDEnabled: true,
628+
CIMDCacheMaxSize: 128,
629+
CIMDCacheFallbackTTL: 10 * time.Minute,
630+
},
631+
wantMaxSize: 128,
632+
wantFallbackTTL: 10 * time.Minute,
633+
},
634+
{
635+
name: "CIMD disabled leaves zero fields unchanged",
636+
cfg: Config{Issuer: "https://example.com", CIMDEnabled: false},
637+
wantMaxSize: 0,
638+
wantFallbackTTL: 0,
639+
},
640+
}
641+
642+
for _, tt := range tests {
643+
t.Run(tt.name, func(t *testing.T) {
644+
t.Parallel()
645+
cfg := tt.cfg
646+
err := cfg.applyDefaults()
647+
require.NoError(t, err)
648+
require.Equal(t, tt.wantMaxSize, cfg.CIMDCacheMaxSize)
649+
require.Equal(t, tt.wantFallbackTTL, cfg.CIMDCacheFallbackTTL)
650+
})
651+
}
652+
}

pkg/authserver/runner/embeddedauthserver.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ func newEmbeddedAuthServerWithStorage(
176176
// here once at the boundary lets all downstream stages share by reference
177177
// safely. Cost is negligible — each slice is bounded by validation (≤10
178178
// for BaselineClientScopes, low cardinality in practice for the others).
179+
cimdEnabled, cimdCacheMaxSize, cimdCacheFallbackTTL := resolveCIMDConfig(cfg.CIMD)
180+
179181
resolvedCfg := authserver.Config{
180182
Issuer: cfg.Issuer,
181183
AuthorizationEndpointBaseURL: cfg.AuthorizationEndpointBaseURL,
@@ -188,6 +190,9 @@ func newEmbeddedAuthServerWithStorage(
188190
ScopesSupported: slices.Clone(cfg.ScopesSupported),
189191
BaselineClientScopes: slices.Clone(cfg.BaselineClientScopes),
190192
AllowedAudiences: slices.Clone(cfg.AllowedAudiences),
193+
CIMDEnabled: cimdEnabled,
194+
CIMDCacheMaxSize: cimdCacheMaxSize,
195+
CIMDCacheFallbackTTL: cimdCacheFallbackTTL,
191196
}
192197

193198
// 7. Create the auth server. authserver.New also asserts the DCR
@@ -782,6 +787,27 @@ func convertRedisTLSRunConfig(rc *storage.RedisTLSRunConfig) (*tcredis.TLSConfig
782787
return cfg, nil
783788
}
784789

790+
// resolveCIMDConfig extracts CIMD settings from a CIMDRunConfig.
791+
// Returns zero values when cfg is nil (CIMD disabled).
792+
// The CacheFallbackTTL string is parsed to time.Duration; callers must ensure
793+
// CIMDRunConfig.Validate() has already been called so the string is well-formed.
794+
func resolveCIMDConfig(cfg *authserver.CIMDRunConfig) (enabled bool, cacheMaxSize int, cacheFallbackTTL time.Duration) {
795+
if cfg == nil {
796+
return false, 0, 0
797+
}
798+
var ttl time.Duration
799+
if cfg.CacheFallbackTTL != "" {
800+
var err error
801+
ttl, err = time.ParseDuration(cfg.CacheFallbackTTL)
802+
if err != nil {
803+
// Should not happen when called after CIMDRunConfig.Validate().
804+
slog.Warn("invalid cimd cache_fallback_ttl, zero will be replaced by default",
805+
"value", cfg.CacheFallbackTTL, "err", err)
806+
}
807+
}
808+
return cfg.Enabled, cfg.CacheMaxSize, ttl
809+
}
810+
785811
// resolveEnvVar reads a value from the named environment variable.
786812
func resolveEnvVar(envVar string) (string, error) {
787813
if envVar == "" {

0 commit comments

Comments
 (0)