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

[jhrozek]

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

Problem

DCRConfig exists on OAuth2UpstreamRunConfig (pkg/authserver/config.go:669) and its
CRD counterpart OAuth2UpstreamConfig (mcpexternalauthconfig_types.go:1205), but
OIDCUpstreamRunConfig / OIDCUpstreamConfig have no equivalent field at all. This is
structural, not a runtime gate:

• No field on either struct.
• No conversion code (buildOIDCUpstreamRunConfig copies nothing DCR-related).
• The trigger function needsDCR(rc *authserver.OAuth2UpstreamRunConfig) is typed to
  only accept the OAuth2 variant, so it can't even be called for an OIDC config.

Practical effect: an operator can only get DCR by declaring type: oauth2 with explicit
authorization_endpoint/token_endpoint. Any upstream configured as type: oidc
(relying on standard /.well-known/openid-configuration discovery) can never use DCR,
even when its discovery document publishes a registration_endpoint. This is the case
today for connector-gateway.stacklok.dev, which forces the more verbose explicit-endpoint
oauth2 form just to get DCR.

Why this is a real gap, not just an edge case

RFC 7591 Dynamic Client Registration is defined purely by its own request/response
contract — a POST of client metadata to a registration_endpoint, returning
client_id/client_secret. It has no dependency on how the caller learned that URL.
Both RFC 8414 (Authorization Server Metadata) and OpenID Connect Discovery 1.0 publish
registration_endpoint with identical semantics, and OIDC discovery is a strict superset
of the OAuth2 metadata fields DCR already reads (registration_endpoint,
authorization_endpoint, token_endpoint, token_endpoint_auth_methods_supported,
scopes_supported). There is no separate "OIDC DCR" wire protocol — DCR is
discovery-source-agnostic by spec. OpenID Connect Dynamic Client Registration 1.0 adds a
handful of OPTIONAL extra metadata fields (subject_type, id_token_signed_response_alg,
etc.) that a bare RFC 7591 registration can simply omit — the OP applies its own defaults,
and interop is unaffected. So there's no spec reason DCR should remain OAuth2-only.

Recommended fix (scoping fix, not a new feature)

Reuse the discovery-fetch/field-extraction logic DCRUpstreamConfig already has. Do
not introduce a shared interface or common embed for this — dcr.Request is already
the profile-neutral shape both paths should converge on, and a second neutral type would
just duplicate it and drift. Concretely:

1. pkg/authserver/config.go

   • Add DCRConfig *DCRUpstreamConfig to OIDCUpstreamRunConfig.
   • For OIDC, DiscoveryURL becomes optional — it defaults to
     IssuerURL + "/.well-known/openid-configuration". This round-trips cleanly with
     deriveExpectedIssuerFromDiscoveryURL (pkg/auth/dcr/resolver.go:1040), so no
     changes are needed in pkg/auth/dcr itself.
   • Update the DCRUpstreamConfig doc comment: OAuth2 upstreams must set
     DiscoveryURL or explicit endpoints (no issuer to derive from); OIDC upstreams may
     omit both and derive from IssuerURL.
   • Add OIDCUpstreamRunConfig.Validate() enforcing ClientID XOR DCRConfig (OIDC
     currently has no such validator — mirror buildPureOAuth2Config's validation call
     in embeddedauthserver.go:652).

2. pkg/authserver/runner/dcr_adapter.go

   • Delete needsDCR (:38); replace with a newDCRRequest(rc *authserver.UpstreamRunConfig, localIssuer string) (*dcr.Request, error)
     that switches on rc.Type and returns nil, nil when no DCR is needed. The OIDC
     branch synthesizes the discovery URL as above and leaves
     AuthorizationEndpoint/TokenEndpoint for the OIDC provider to discover.
   • Add consumeOIDCResolution / applyResolutionToOIDCConfig mirroring the existing
     OAuth2 versions (writes ClientID, ClientSecret, RedirectURI if empty, nils
     DCRConfig). Note: upstream.OIDCConfig has no TokenEndpointAuthMethod field —
     leave that gap as-is (already documented as OAuth2-only in upstream/oauth2.go:150).

3. pkg/authserver/runner/embeddedauthserver.go:506-548

   • Replace the single OAuth2-only call site with a per-type branch that pairs
     newDCRRequest + consume*/apply* for both types.

4. cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go

   • Add DCRConfig *DCRUpstreamConfig to OIDCUpstreamConfig (:982); drop the
     +kubebuilder:validation:Required on ClientID (:990); add CEL rules mirroring
     the OAuth2 ones (:1103-1104), relaxed to "at most one of DiscoveryURL/explicit
     endpoints" for OIDC vs. "exactly one" for OAuth2.
   • Add ValidateOIDCDCRConfig, called from ValidateUpstreams (:2145), mirroring
     ValidateOAuth2DCRConfig (:2214).

5. cmd/thv-operator/pkg/controllerutil/authserver.go

   • buildOIDCUpstreamRunConfig (:1159) gains an error return, calls the new
     validator, and sets DCRConfig via the same helper the OAuth2 builder uses.
   • Extend the initial-access-token env var emitter (:181) — currently reads the ref
     only off OAuth2Config.DCRConfig — to also check OIDCConfig.DCRConfig.

6. Regenerate: task operator-generate && task operator-manifests && task crdref-gen.

Tradeoffs / what NOT to do

• Don't build a shared HasDCR interface or UpstreamAuthConfigCommon embed. The only
  duplication this introduces is one field + a couple of CEL rules on two structs
  (~10 lines) — cheaper than a shared type that would change the JSON/CRD shape of both
  configs for no behavioral gain. This mirrors existing duplication of ClientID,
  Scopes, AllowPrivateIPs across the two upstream types already.
• Don't touch pkg/auth/dcr — the resolver is already discovery-source-agnostic; the
  gap is entirely in the config plumbing above it.

Verification notes for whoever picks this up

• Confirm CEL validation cost budget on OIDCUpstreamConfig doesn't blow the K8s CEL
  cost limit — check with a real kubectl apply against a max-length upstreams list,
  not just envtest.
• End-to-end test against a real OIDC discovery document that publishes
  registration_endpoint (e.g. a local Keycloak realm, or connector-gateway.stacklok.dev
  itself) — this is exactly the "kind e2e, no exceptions" case per team convention;
  RunConfig-level reachability isn't sufficient proof by itself.

Related

Unrelated question raised alongside this one (re: whether DCR re-registration reuses a
possibly single-use InitialAccessToken) was investigated and found not to be a bug:
the token is re-read fresh from its configured source on every registration call,
including staleness-triggered re-registrations. No action needed there.