Expose DCR config in operator CRD for OAuth2 upstreams

Implements changes for issue #5040 (Phase 2 DCR CRD surface):

- Add DCRUpstreamConfig CRD type (discoveryUrl, registrationEndpoint,
  initialAccessTokenRef, softwareId, softwareStatement) and a new
  dcrConfig field on OAuth2UpstreamConfig so Kubernetes users can
  configure RFC 7591 Dynamic Client Registration on upstream providers.
- Make OAuth2UpstreamConfig.clientId optional and add CEL validation
  requiring exactly one of clientId or dcrConfig, and exactly one of
  discoveryUrl or registrationEndpoint inside dcrConfig. Mirror the
  checks at runtime via validateOAuth2DCRConfig for defense-in-depth.
- Wire the conversion in controllerutil/authserver.go so DCRConfig is
  mapped onto authserver.DCRUpstreamConfig. InitialAccessTokenRef is
  resolved to an env var (TOOLHIVE_UPSTREAM_DCR_INITIAL_ACCESS_TOKEN_*)
  populated from the referenced Secret, mirroring the ClientSecretRef
  pattern. Extract small helpers for env-var generation to keep
  cyclomatic complexity within lint limits.
- Regenerate zz_generated.deepcopy.go, CRD YAML manifests, and CRD API
  reference docs.
- Add table-driven validation tests covering DCR+ClientID conflict,
  both endpoints set, neither endpoint set, valid single-endpoint
  cases, and neither-auth configuration. Add conversion tests covering
  DCR discoveryUrl/registrationEndpoint paths and initial-access-token
  env var wiring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address code review feedback

Fixed issues from code review of the DCR CRD surface commit:

- CRITICAL: CEL markers contained a Unicode smart quote (U+201D) that
  gofmt's doc-comment formatter reintroduced on every lint-fix. Rewrote
  both markers to use CEL's size(...) > 0 idiom instead of `!= ''`, which
  sidesteps the typographic normalization entirely and keeps regeneration
  idempotent. Verified no U+2018-U+201F characters remain in source or CRDs.
- HIGH: buildUpstreamRunConfig now calls the exported
  mcpv1beta1.ValidateOAuth2DCRConfig before producing a RunConfig, so
  malformed ClientID/DCRConfig pairs that bypass admission fail at
  reconcile time rather than at authserver startup. Error propagation
  threaded through BuildAuthServerRunConfig; split OIDC and OAuth2
  branches into helpers to stay under the gocyclo limit.
- HIGH: Added table case exercising validateUpstreamProvider rejection
  of an OIDC-typed provider whose OAuth2Config carries a DCRConfig.
- MEDIUM: Added kubebuilder CEL XValidation on UpstreamProviderConfig
  enforcing oidcConfig/oauth2Config mutual exclusivity paired to the
  declared type, closing the silent-pod-failure YAML-apply gap.
- MEDIUM: Added MaxLength=255 to SoftwareID and MaxLength=4096 to
  SoftwareStatement to prevent unbounded input from inflating CRs
  beyond etcd object limits.
- MEDIUM: Pinned the "neither ClientID nor DCRConfig" error assertion to
  the scoped `oauth2Config:` prefix; added a regression case exercising
  the non-DCR OAuth2 path (ClientID only, DCRConfig nil); added a new
  TestBuildAuthServerRunConfig_InvalidDCR suite covering all four
  invalid DCR/ClientID pairings at the conversion layer.
- MEDIUM: Renamed UpstreamDCRInitialAccessTokenEnvVar to
  UpstreamDCRInitialAccessTokenEnvVarPrefix and expanded the godoc on
  both prefix constants to show the resolved <prefix>_<PROVIDER> form.

All task lint/lint-fix/license-check pass; regenerated CRDs and
deepcopy are idempotent; affected unit tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address iteration-2 review feedback

Polish items raised in the second review pass:

- MEDIUM: Trim duplicate upstream name from reconcile-time DCR validation
  errors. Added scopedFieldPath() helper in
  cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go so
  ValidateOAuth2DCRConfig prepends a dotted prefix only when one is
  given, and the conversion call site now passes an empty prefix so
  BuildAuthServerRunConfig's outer "upstream %q: %w" wrap is the only
  mention of the upstream name. Strengthened
  TestBuildAuthServerRunConfig_InvalidDCR to assert the upstream name
  appears exactly once in the error string.
- MEDIUM: Make the UpstreamProviderConfig CEL rule fail closed for
  unrecognized future provider types. Restructured the rule from a
  binary discriminator into a chain of equality checks ending in an
  explicit `false`, and updated the message to "type must be 'oidc'
  or 'oauth2'; ...". Added a contributor-facing doc comment reminding
  future authors to extend both the rule and validateUpstreamProvider
  when adding a new UpstreamProviderType.
- MEDIUM: Refresh the godoc on extractUpstreamSecretRefs to describe
  the actual invariants that hold post-CEL: OIDC providers can only
  return a clientSecretRef; OAuth2 providers can return both
  independently; other (currently unreachable) types return nil/nil.
  Cross-linked to the CEL rule and noted that BuildAuthServerRunConfig
  is the reconcile-time backstop callers should not rely on this
  helper to enforce.

Regenerated CRD YAMLs and crd-api.md prose. task lint, lint-fix,
license-check, and the affected unit tests pass; regeneration is
idempotent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: tgrunnagle

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

@@ -273,6 +273,21 @@ const (
 )
 
 // UpstreamProviderConfig defines configuration for an upstream Identity Provider.
+//
+// Exactly one of OIDCConfig or OAuth2Config must be set and must match the
+// declared Type: oidc-typed providers set OIDCConfig, oauth2-typed providers
+// set OAuth2Config. The CEL rule below enforces the pairing at admission; the
+// matching Go-level check in validateUpstreamProvider provides defense-in-depth
+// for stored objects.
+//
+// The rule is structured as a chain of equality checks ending in an explicit
+// `false`, so adding a new UpstreamProviderType value without extending this
+// rule fails admission instead of silently demanding the OAuth2 shape. When
+// adding a new type, extend both this rule and validateUpstreamProvider.
+//
+// +kubebuilder:validation:XValidation:rule="self.type == 'oidc' ? (has(self.oidcConfig) && !has(self.oauth2Config)) : self.type == 'oauth2' ? (has(self.oauth2Config) && !has(self.oidcConfig)) : false",message="type must be 'oidc' or 'oauth2'; oidcConfig must be set when type is 'oidc' and oauth2Config must be set when type is 'oauth2' (and the other must not be set)"
+//
+//nolint:lll // CEL validation rules exceed line length limit
 type UpstreamProviderConfig struct {
 	// Name uniquely identifies this upstream provider.
 	// Used for routing decisions and session binding in multi-upstream scenarios.
@@ -354,6 +369,14 @@ type OIDCUpstreamConfig struct {
 
 // OAuth2UpstreamConfig contains configuration for pure OAuth 2.0 providers.
 // OAuth 2.0 providers require explicit endpoint configuration.
+//
+// Exactly one of ClientID or DCRConfig must be set: ClientID is used when the
+// client is pre-provisioned out of band, DCRConfig enables RFC 7591 Dynamic
+// Client Registration at runtime.
+//
+// +kubebuilder:validation:XValidation:rule="(has(self.clientId) && size(self.clientId) > 0) ? !has(self.dcrConfig) : has(self.dcrConfig)",message="exactly one of clientId or dcrConfig must be set"
+//
+//nolint:lll // CEL validation rules exceed line length limit
 type OAuth2UpstreamConfig struct {
 	// AuthorizationEndpoint is the URL for the OAuth authorization endpoint.
 	// +kubebuilder:validation:Required
@@ -374,8 +397,10 @@ type OAuth2UpstreamConfig struct {
 	UserInfo *UserInfoConfig `json:"userInfo,omitempty"`
 
 	// ClientID is the OAuth 2.0 client identifier registered with the upstream IDP.
-	// +kubebuilder:validation:Required
-	ClientID string `json:"clientId"`
+	// Mutually exclusive with DCRConfig: when DCRConfig is set, ClientID is obtained
+	// at runtime via RFC 7591 Dynamic Client Registration and must be left empty.
+	// +optional
+	ClientID string `json:"clientId,omitempty"`
 
 	// ClientSecretRef references a Kubernetes Secret containing the OAuth 2.0 client secret.
 	// Optional for public clients using PKCE instead of client secret.
@@ -410,6 +435,81 @@ type OAuth2UpstreamConfig struct {
 	// +kubebuilder:validation:MaxProperties=16
 	// +optional
 	AdditionalAuthorizationParams map[string]string `json:"additionalAuthorizationParams,omitempty"`
+
+	// DCRConfig enables RFC 7591 Dynamic Client Registration against the upstream
+	// authorization server. When set, the client credentials are obtained at
+	// runtime rather than being pre-provisioned, and ClientID must be left empty.
+	// Mutually exclusive with ClientID.
+	// +optional
+	DCRConfig *DCRUpstreamConfig `json:"dcrConfig,omitempty"`
+}
+
+// DCRUpstreamConfig configures RFC 7591 Dynamic Client Registration for an
+// OAuth 2.0 upstream. When present on an OAuth2 upstream, the authserver
+// performs registration at runtime to obtain client credentials, replacing
+// the need to pre-provision a ClientID.
+//
+// Exactly one of DiscoveryURL or RegistrationEndpoint must be set. DiscoveryURL
+// points at an RFC 8414 / OIDC Discovery document from which the registration
+// endpoint is resolved; RegistrationEndpoint is used directly when the upstream
+// does not publish discovery metadata.
+//
+// The XOR rule uses has() alone (not has() + size() > 0) to keep the rule's
+// estimated CEL cost under the apiserver's per-rule static budget. With
+// `omitempty` on both fields, an unset field is absent on the wire and has()
+// returns false; the explicit-empty-string edge case is rejected at reconcile
+// time by ValidateOAuth2DCRConfig.
+//
+// +kubebuilder:validation:XValidation:rule="has(self.discoveryUrl) != has(self.registrationEndpoint)",message="exactly one of discoveryUrl or registrationEndpoint must be set"
+//
+//nolint:lll // CEL validation rules exceed line length limit
+type DCRUpstreamConfig struct {
+	// DiscoveryURL is the RFC 8414 / OIDC Discovery document URL. The resolver
+	// issues a single GET against this URL (no well-known-path fallback) and
+	// reads registration_endpoint, authorization_endpoint, token_endpoint,
+	// token_endpoint_auth_methods_supported, and scopes_supported from the
+	// response.
+	// Mutually exclusive with RegistrationEndpoint.
+	// MaxLength bounds CEL cost estimation on the parent struct's
+	// XValidation rule and matches the conventional URL length cap.
+	// +optional
+	// +kubebuilder:validation:Pattern=`^https?://.*$`
+	// +kubebuilder:validation:MaxLength=2048
+	DiscoveryURL string `json:"discoveryUrl,omitempty"`
+
+	// RegistrationEndpoint is the RFC 7591 registration endpoint URL used
+	// directly, bypassing discovery. When using this field, the caller is
+	// expected to also supply AuthorizationEndpoint, TokenEndpoint, and an
+	// explicit Scopes list on the parent OAuth2UpstreamConfig.
+	// Mutually exclusive with DiscoveryURL.
+	// MaxLength bounds CEL cost estimation on the parent struct's
+	// XValidation rule and matches the conventional URL length cap.
+	// +optional
+	// +kubebuilder:validation:Pattern=`^https?://.*$`
+	// +kubebuilder:validation:MaxLength=2048
+	RegistrationEndpoint string `json:"registrationEndpoint,omitempty"`
+
+	// InitialAccessTokenRef is an optional reference to a Kubernetes Secret
+	// carrying an RFC 7591 §3 initial access token. When set, the resolver
+	// presents the token value as a Bearer credential on the registration
+	// request. Mirrors the ClientSecretRef pattern.
+	// +optional
+	InitialAccessTokenRef *SecretKeyRef `json:"initialAccessTokenRef,omitempty"`
+
+	// SoftwareID is the RFC 7591 "software_id" registration metadata value,
+	// identifying the client software independent of any particular
+	// registration instance. Typically a UUID or short identifier.
+	// +optional
+	// +kubebuilder:validation:MaxLength=255
+	SoftwareID string `json:"softwareId,omitempty"`
+
+	// SoftwareStatement is the RFC 7591 "software_statement" JWT asserting
+	// metadata about the client software, signed by a party the authorization
+	// server trusts. Bounded to 4096 characters to prevent unbounded
+	// user input from inflating the CR beyond etcd object limits.
+	// +optional
+	// +kubebuilder:validation:MaxLength=4096
+	SoftwareStatement string `json:"softwareStatement,omitempty"`
 }
 
 // TokenResponseMapping maps non-standard token response fields to standard OAuth 2.0 fields
@@ -956,10 +1056,62 @@ func (*MCPExternalAuthConfig) validateUpstreamProvider(index int, provider *Upst
 		return fmt.Errorf("%s: unsupported provider type: %s", prefix, provider.Type)
 	}
 
+	// Validate OAuth2-specific DCR / ClientID constraints (defense-in-depth with CEL).
+	if provider.Type == UpstreamProviderTypeOAuth2 && provider.OAuth2Config != nil {
+		if err := ValidateOAuth2DCRConfig(prefix, provider.OAuth2Config); err != nil {
+			return err
+		}
+	}
+
 	// Validate additionalAuthorizationParams does not contain reserved keys
 	return ValidateAdditionalAuthorizationParams(prefix, provider.AdditionalAuthorizationParams())
 }
 
+// ValidateOAuth2DCRConfig enforces the mutual exclusivity between ClientID and
+// DCRConfig and, when DCRConfig is present, between DiscoveryURL and
+// RegistrationEndpoint. These rules mirror the CEL validation on
+// OAuth2UpstreamConfig and DCRUpstreamConfig and provide defense-in-depth for
+// stored objects (e.g., objects stored before CEL rules were added or
+// validated through code paths that bypass admission).
+//
+// The prefix is embedded in error messages to identify the offending upstream
+// (e.g., "upstreamProviders[i]"). Pass an empty string when the caller already
+// wraps the error with the upstream identifier, to avoid duplicating it.
+// Exported so the controllerutil conversion layer can reuse the same
+// invariants when building runtime configs, rejecting malformed objects at
+// reconcile time rather than waiting until the authserver process parses them.
+func ValidateOAuth2DCRConfig(prefix string, cfg *OAuth2UpstreamConfig) error {
+	hasClientID := cfg.ClientID != ""
+	hasDCR := cfg.DCRConfig != nil
+
+	scoped := scopedFieldPath(prefix, "oauth2Config")
+	if hasClientID == hasDCR {
+		return fmt.Errorf("%s: exactly one of clientId or dcrConfig must be set", scoped)
+	}
+
+	if !hasDCR {
+		return nil
+	}
+
+	hasDiscoveryURL := cfg.DCRConfig.DiscoveryURL != ""
+	hasRegistrationEndpoint := cfg.DCRConfig.RegistrationEndpoint != ""
+	if hasDiscoveryURL == hasRegistrationEndpoint {
+		return fmt.Errorf("%s.dcrConfig: exactly one of discoveryUrl or registrationEndpoint must be set", scoped)
+	}
+	return nil
+}
+
+// scopedFieldPath joins a non-empty prefix to a field name with a dot. When
+// the prefix is empty, it returns just the field name so callers that already
+// supply their own scope (e.g., a wrapping `fmt.Errorf("upstream %q: %w", ...)`)
+// don't end up with a leading dot.
+func scopedFieldPath(prefix, field string) string {
+	if prefix == "" {
+		return field
+	}
+	return prefix + "." + field
+}
+
 // AdditionalAuthorizationParams returns the additional authorization parameters
 // from whichever upstream config is set, or nil if none.
 func (p *UpstreamProviderConfig) AdditionalAuthorizationParams() map[string]string {

cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go

@@ -571,6 +571,125 @@ func TestMCPExternalAuthConfig_validateUpstreamProvider(t *testing.T) {
 			},
 			expectErr: false,
 		},
+		{
+			name: "OAuth2 provider with valid DCRConfig (discoveryUrl only)",
+			provider: UpstreamProviderConfig{
+				Name: "dcr-discovery",
+				Type: UpstreamProviderTypeOAuth2,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+					DCRConfig: &DCRUpstreamConfig{
+						DiscoveryURL: "https://idp.example.com/.well-known/openid-configuration",
+					},
+				},
+			},
+			expectErr: false,
+		},
+		{
+			name: "OAuth2 provider with valid DCRConfig (registrationEndpoint only)",
+			provider: UpstreamProviderConfig{
+				Name: "dcr-endpoint",
+				Type: UpstreamProviderTypeOAuth2,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+					Scopes:                []string{"openid"},
+					DCRConfig: &DCRUpstreamConfig{
+						RegistrationEndpoint: "https://idp.example.com/register",
+					},
+				},
+			},
+			expectErr: false,
+		},
+		{
+			name: "OAuth2 provider with DCRConfig and non-empty ClientID",
+			provider: UpstreamProviderConfig{
+				Name: "dcr-with-clientid",
+				Type: UpstreamProviderTypeOAuth2,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+					ClientID:              "pre-provisioned-id",
+					DCRConfig: &DCRUpstreamConfig{
+						DiscoveryURL: "https://idp.example.com/.well-known/openid-configuration",
+					},
+				},
+			},
+			expectErr: true,
+			errMsg:    "exactly one of clientId or dcrConfig must be set",
+		},
+		{
+			name: "OAuth2 provider with DCRConfig specifying both endpoints",
+			provider: UpstreamProviderConfig{
+				Name: "dcr-both-endpoints",
+				Type: UpstreamProviderTypeOAuth2,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+					DCRConfig: &DCRUpstreamConfig{
+						DiscoveryURL:         "https://idp.example.com/.well-known/openid-configuration",
+						RegistrationEndpoint: "https://idp.example.com/register",
+					},
+				},
+			},
+			expectErr: true,
+			errMsg:    "exactly one of discoveryUrl or registrationEndpoint must be set",
+		},
+		{
+			name: "OAuth2 provider with DCRConfig specifying neither endpoint",
+			provider: UpstreamProviderConfig{
+				Name: "dcr-neither-endpoint",
+				Type: UpstreamProviderTypeOAuth2,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+					DCRConfig:             &DCRUpstreamConfig{},
+				},
+			},
+			expectErr: true,
+			errMsg:    "exactly one of discoveryUrl or registrationEndpoint must be set",
+		},
+		{
+			name: "OAuth2 provider with neither ClientID nor DCRConfig",
+			provider: UpstreamProviderConfig{
+				Name: "oauth2-missing-auth",
+				Type: UpstreamProviderTypeOAuth2,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+				},
+			},
+			expectErr: true,
+			// Pin the scoped prefix so a rename of the oauth2Config label surfaces as a test failure.
+			errMsg: "oauth2Config: exactly one of clientId or dcrConfig must be set",
+		},
+		{
+			// Regression guard: conversion only maps DCRConfig in the OAuth2
+			// branch, so an OIDC-typed provider carrying OAuth2Config/DCRConfig
+			// must be rejected at validate time rather than silently dropped.
+			name: "OIDC provider with OAuth2Config carrying DCRConfig is rejected",
+			provider: UpstreamProviderConfig{
+				Name: "mismatched-oidc",
+				Type: UpstreamProviderTypeOIDC,
+				OAuth2Config: &OAuth2UpstreamConfig{
+					AuthorizationEndpoint: "https://idp.example.com/authorize",
+					TokenEndpoint:         "https://idp.example.com/token",
+					UserInfo:              &UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"},
+					DCRConfig: &DCRUpstreamConfig{
+						DiscoveryURL: "https://idp.example.com/.well-known/openid-configuration",
+					},
+				},
+			},
+			expectErr: true,
+			errMsg:    "oidcConfig must be set when type is 'oidc' and must not be set otherwise",
+		},
 	}
 
 	for _, tt := range tests {