Wire CIMD config through embedded AS and enable storage decorator#5348
Open
amirejaz wants to merge 4 commits into
Open
Wire CIMD config through embedded AS and enable storage decorator#5348amirejaz wants to merge 4 commits into
amirejaz wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5348 +/- ##
==========================================
- Coverage 68.81% 68.77% -0.04%
==========================================
Files 627 627
Lines 63594 63625 +31
==========================================
- Hits 43762 43759 -3
- Misses 16583 16612 +29
- Partials 3249 3254 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4978459 to
233b73a
Compare
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>
- 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>
233b73a to
1e0e00f
Compare
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>
683469f to
5d11a41
Compare
3 tasks
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 2 PR 3 of CIMD embedded AS support — depends on #5343.
This is the wiring PR. All CIMD logic lives in PRs 1 and 2; this PR threads the configuration through the existing authserver config chain and activates the decorator.
Config chain:
RunConfig.CIMD→Config.CIMD*→AuthorizationServerParams→AuthorizationServerConfig→ discovery handlerWith this PR, operators can enable CIMD by setting:
When enabled:
client_id_metadata_document_supported: trueon both/.well-known/oauth-authorization-serverand/.well-known/openid-configurationCIMDStorageDecoratorwraps the storage soGetClientintercepts HTTPSclient_idvalues, fetches the metadata document, and caches the resultCIMD is opt-in (disabled by default) to avoid introducing outbound HTTPS fetching in existing deployments without explicit operator action.
Key implementation note
The decorator is applied after
runLegacyMigration(which needs the unwrapped*RedisStoragefor type assertion) and beforecreateProvider(so fosite holds the decorated storage and HTTPSclient_idvalues are intercepted duringGetClient).Known limitations
MCPExternalAuthConfighas noauthServer.cimdfield yet. Kubernetes operators must configure CIMD viaRunConfigYAML directly. CRD support is deferred to a follow-up (tracked with// TODO(cimd)comments in config.go).Type of change
Test plan
go test ./pkg/authserver/...— all 13 packages passCIMDCacheMaxSize < 1rejected at boot when enabledclient_id_metadata_document_supportedpresent/absent based onCIMDEnabledresolveCIMDConfignil-guard testedGenerated with Claude Code