Skip to content

Support OIDC dynamic client registration - #6544

Merged
jhrozek merged 2 commits into
mainfrom
6538-oidc-dcr
Sep 8, 2026
Merged

jhrozek merged 2 commits into
mainfrom
6538-oidc-dcr

Conversation

@jhrozek

@jhrozek jhrozek commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • OIDC-typed upstreams could not use Dynamic Client Registration (DCR) even
    when their discovery metadata exposed a registration_endpoint, forcing
    operators onto the more verbose explicit-endpoint OAuth2 form for
    providers such as connector-gateway.stacklok.dev that publish standard
    OIDC discovery. DCRConfig existed only on the OAuth2 upstream types; the
    gap was structural (missing field, no conversion path, no call site), not
    a runtime restriction — RFC 7591 DCR is discovery-source-agnostic and does
    not distinguish OIDC-discovered vs. OAuth2-declared endpoints.
  • Add DCRConfig to OIDCUpstreamRunConfig / OIDCUpstreamConfig (runtime
    and CRD), deriving the discovery URL from IssuerURL + /.well-known/openid-configuration when the operator doesn't set one
    explicitly — no changes needed in pkg/auth/dcr itself.
  • Route both OAuth2 and OIDC upstreams through the shared DCR resolver via a
    type-agnostic newDCRRequest, add consumeOIDCResolution /
    applyResolutionToOIDCConfig mirroring the existing OAuth2 helpers, add
    OIDC XOR validation (ClientID vs DCRConfig), extend CRD CEL validation
    and the initial-access-token secret-ref plumbing to the OIDC path, and
    regenerate manifests/docs.

Fixes #6538

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

OIDCUpstreamConfig.ClientID changes from required to optional (was
+kubebuilder:validation:Required), and a new optional dcrConfig field is
added — both additive, backward compatible with existing manifests.

Changes

File Change
pkg/authserver/config.go Add DCRConfig to OIDCUpstreamRunConfig; add Validate() enforcing ClientID/DCRConfig exclusivity
pkg/authserver/runner/dcr_adapter.go Generalize newDCRRequest to both upstream types; add OIDC consume/apply helpers
pkg/authserver/runner/embeddedauthserver.go Branch DCR request/consume/apply per upstream type
pkg/authserver/runner/dcr_adapter_test.go Cover the new OIDC DCR path
cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go Add DCRConfig to OIDCUpstreamConfig; relax ClientID to optional; add CEL rules and ValidateOIDCDCRConfig
cmd/thv-operator/pkg/controllerutil/authserver.go Validate and populate OIDC DCR config; extend initial-access-token secret-ref extraction to OIDC
deploy/charts/**, docs/operator/crd-api.md, zz_generated.deepcopy.go Regenerated from the CRD type changes

Does this introduce a user-facing change?

Yes — operators can now set dcrConfig on an OIDC-typed upstream
(type: oidc) to use Dynamic Client Registration, instead of being
required to use the more verbose type: oauth2 form with explicit
authorizationEndpoint/tokenEndpoint just to get DCR.

Special notes for reviewers

  • The discovery URL for OIDC DCR is derived from IssuerURL + "/.well-known/openid-configuration" when the operator doesn't set
    dcrConfig.discoveryUrl/registrationEndpoint explicitly. This round-trips
    correctly through the existing pkg/auth/dcr resolver's
    deriveExpectedIssuerFromDiscoveryURL without any changes there.
  • Deliberately did not introduce a shared interface/embed between the OAuth2
    and OIDC DCR wiring — the two config types already duplicate several
    fields (ClientID, Scopes, AllowPrivateIPs), and a shared abstraction
    here would churn the CRD/run-config JSON shape for no behavioral gain.
  • Follow-up (not in this PR): end-to-end verification against a real OIDC
    discovery document that publishes registration_endpoint (e.g. Keycloak,
    or connector-gateway.stacklok.dev itself) in a kind cluster — RunConfig-level
    reachability isn't sufficient proof by itself for this class of change.

Implementation plan

Approved implementation plan

Design produced by an oauth-expert + go-architect review pair prior to
implementation:

Spec review (oauth-expert): RFC 7591 DCR is discovery-source-agnostic —
the registration_endpoint POST/response contract doesn't depend on whether
the endpoint was learned via OIDC discovery or explicit OAuth2 config. OIDC
Dynamic Client Registration 1.0 adds only OPTIONAL extra metadata fields
(subject_type, id_token_signed_response_alg, etc.); omitting them is
spec-compliant. No spec reason to keep OIDC and OAuth2 DCR separate.

Architecture (go-architect): Reuse the existing DCRUpstreamConfig
shape rather than introducing a shared interface/embed. Add the field
directly to OIDCUpstreamRunConfig/OIDCUpstreamConfig (mirroring the
OAuth2 struct, ~10 lines of duplication — cheaper than a shared type that
would churn the CRD/run-config JSON shape). Make DiscoveryURL optional for
OIDC, defaulting to IssuerURL + "/.well-known/openid-configuration"
(round-trips through deriveExpectedIssuerFromDiscoveryURL unmodified).
Collapse needsDCR into a type-switching newDCRRequest; add
consumeOIDCResolution/applyResolutionToOIDCConfig mirroring the OAuth2
versions; extend CRD CEL validation from "exactly one" to "at most one" of
discoveryUrl/registrationEndpoint at the shared struct level, with OAuth2
re-asserting "exactly one" at its own struct level.

OIDC-typed upstreams could not use Dynamic Client Registration, even
when their discovery metadata exposed a registration endpoint. This
forced operators to use the more verbose OAuth2 form and prevented
issuer-based OIDC configurations from working with providers such as
connector-gateway.

Add DCR configuration and validation to OIDC runtime and CRD types,
derive the standard discovery URL from the issuer when needed, and
route both OAuth2 and OIDC upstreams through the shared DCR resolver.
Propagate registered credentials into the OIDC provider, update
secret bindings and generated manifests, and cover the adapter
behavior with tests.

Fixes #6538
@github-actions github-actions Bot added the size/XL Extra large PR: 1000+ lines changed label Sep 8, 2026
@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.47368% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.66%. Comparing base (c4f6cb4) to head (8229473).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
pkg/authserver/runner/embeddedauthserver.go 63.26% 18 Missing ⚠️
...perator/api/v1beta1/mcpexternalauthconfig_types.go 31.81% 15 Missing ⚠️
pkg/authserver/config.go 27.77% 13 Missing ⚠️
cmd/thv-operator/pkg/controllerutil/authserver.go 66.66% 4 Missing ⚠️
pkg/authserver/runner/dcr_adapter.go 92.15% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6544      +/-   ##
==========================================
- Coverage   78.67%   78.66%   -0.02%     
==========================================
  Files         777      777              
  Lines       76686    76925     +239     
==========================================
+ Hits        60334    60512     +178     
- Misses      16347    16408      +61     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

buildUpstreamConfigs exceeded the cyclomatic complexity limit (19 > 15);
split its per-upstream loop body into buildOneUpstreamConfig,
validateUpstreamRunConfig, resolveUpstreamDCR, and
applyDCRResolutionToConfig. Also wrap two lines that exceeded the
line-length limit, and regenerate docs/server/swagger.* to include the
new dcr_config field on OIDCUpstreamConfig.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Sep 8, 2026
@jhrozek
jhrozek merged commit f8c54a9 into main Sep 8, 2026
49 of 50 checks passed
@jhrozek
jhrozek deleted the 6538-oidc-dcr branch September 8, 2026 20:27
@github-actions github-actions Bot mentioned this pull request Sep 10, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Dynamic Client Registration (DCR) for OIDC-typed upstreams

2 participants