diff --git a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go index 3dd765bfa3..b92cad70d5 100644 --- a/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go @@ -1854,6 +1854,20 @@ type DCRUpstreamConfig struct { // +kubebuilder:validation:MaxLength=2048 RegistrationEndpoint string `json:"registrationEndpoint,omitempty"` + // RotationKey is an optional operator-chosen label that is part of the + // cached registration's identity but is never sent to the upstream. + // Change it to make the proxies register a fresh client at their next + // start instead of reusing the cached one: for example after the + // upstream has dropped the registration (it rejects the stale client_id + // in the user's browser, so ToolHive cannot detect it) or when a + // rotation is required. The previous registration's row stays in storage, + // and upstream refresh tokens bound to the previous client_id stop + // refreshing, so affected users sign in again. + // +optional + // +kubebuilder:validation:Pattern=`^[A-Za-z0-9._-]+$` + // +kubebuilder:validation:MaxLength=64 + RotationKey string `json:"rotationKey,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 diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 463e4efa4e..a58c61fd22 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -1446,6 +1446,7 @@ func buildDCRUpstreamRunConfig( rc := &authserver.DCRUpstreamConfig{ DiscoveryURL: dcr.DiscoveryURL, RegistrationEndpoint: dcr.RegistrationEndpoint, + RotationKey: dcr.RotationKey, SoftwareID: dcr.SoftwareID, SoftwareStatement: dcr.SoftwareStatement, } diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index f1a782df9e..28c6a8ff90 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -1745,6 +1745,7 @@ func TestBuildAuthServerRunConfig(t *testing.T) { UserInfo: &mcpv1beta1.UserInfoConfig{EndpointURL: "https://idp.example.com/userinfo"}, DCRConfig: &mcpv1beta1.DCRUpstreamConfig{ RegistrationEndpoint: "https://idp.example.com/register", + RotationKey: "2026-10", }, }, }, @@ -1761,6 +1762,7 @@ func TestBuildAuthServerRunConfig(t *testing.T) { assert.Equal(t, "https://idp.example.com/register", upstream.OAuth2Config.DCRConfig.RegistrationEndpoint) assert.Empty(t, upstream.OAuth2Config.DCRConfig.DiscoveryURL) + assert.Equal(t, "2026-10", upstream.OAuth2Config.DCRConfig.RotationKey) // No InitialAccessTokenRef set: env var name should stay empty. assert.Empty(t, upstream.OAuth2Config.DCRConfig.InitialAccessTokenEnvVar) }, diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index d296274cad..2bd36963f5 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1736,6 +1736,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -2116,6 +2130,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -4622,6 +4650,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -5002,6 +5044,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index 8f59b290bb..4551c3c251 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -1612,6 +1612,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -1992,6 +2006,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -6767,6 +6795,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -7147,6 +7189,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml index 67e5c60b6b..b3675abc64 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml @@ -1739,6 +1739,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -2119,6 +2133,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -4625,6 +4653,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -5005,6 +5047,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 9aafd8a097..bca8fb78cb 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -1615,6 +1615,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -1995,6 +2009,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -6770,6 +6798,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, @@ -7150,6 +7192,20 @@ spec: maxLength: 2048 pattern: ^https://[^\s?#]+[^/\s?#]$ type: string + rotationKey: + description: |- + RotationKey is an optional operator-chosen label that is part of the + cached registration's identity but is never sent to the upstream. + Change it to make the proxies register a fresh client at their next + start instead of reusing the cached one: for example after the + upstream has dropped the registration (it rejects the stale client_id + in the user's browser, so ToolHive cannot detect it) or when a + rotation is required. The previous registration's row stays in storage, + and upstream refresh tokens bound to the previous client_id stop + refreshing, so affected users sign in again. + maxLength: 64 + pattern: ^[A-Za-z0-9._-]+$ + type: string softwareId: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, diff --git a/docs/arch/11-auth-server-storage.md b/docs/arch/11-auth-server-storage.md index 53717bebee..aa3e960154 100644 --- a/docs/arch/11-auth-server-storage.md +++ b/docs/arch/11-auth-server-storage.md @@ -62,7 +62,7 @@ The storage layer implements multiple interfaces from the [fosite](https://githu - `PendingAuthorizationStorage` — In-flight authorization tracking. Interactive requests have a single-use `awaiting` consent stage before upstream login; only an `approved` record can complete a callback. Each `PendingAuthorization` carries a `BrowserBindingHash`, the SHA-256 of a random cookie value set on the user agent. The consent POST and `/oauth/callback` require the matching cookie before consuming their respective records. Only the hash is stored; an older row without a binding or approval stage cannot complete the flow and must be restarted. `PendingDeviceLogin` carries the same browser binding for the separate device-flow verification-page login, which shares `/oauth/callback`. - `RememberedConsentStorage` — Optional server-side browser sessions and one approval per `(userID, clientID)`. The browser holds only a random `__Host-thv_consent_` secret; storage holds its SHA-256 digest. Each approval records the redirect URI, scopes, and resource selected for that client. Memory and Redis implement this capability; decorators expose the underlying backend via `storage.Unwrap`. See [Interactive client approval](#interactive-client-approval). - `UserStorage` — Internal user accounts and provider identity linking -- `DCRCredentialStore` — DCR client metadata and credential persistence; intentionally NOT embedded in `Storage` (each backend implements it separately and call sites reach it via an explicit `stor.(DCRCredentialStore)` type assertion). Secret-based clients store hashed secrets; `private_key_jwt` clients store their inline public JWKS and signing algorithm. +- `DCRCredentialStore` — DCR client metadata and credential persistence; intentionally NOT embedded in `Storage` (each backend implements it separately and call sites reach it via an explicit `stor.(DCRCredentialStore)` type assertion). Secret-based clients store hashed secrets; `private_key_jwt` clients store their inline public JWKS and signing algorithm. Outbound (upstream) registrations resolved by `pkg/auth/dcr` are keyed by `(Issuer, UpstreamID, RedirectURI, ScopesHash)` plus the optional operator-set `dcrConfig.rotationKey`. Changing the rotation key addresses a new row, so the next start registers a fresh upstream client; the previous row stays in place and carries no TTL unless the upstream asserted `client_secret_expires_at`. - `AssertionJWTConsumer` — atomic single-use consumption for validated JWT assertions. It is deliberately separate from `Storage`, so only assertion-grant composition requires replay protection. **Implementation:** diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 57828d40eb..1df586739e 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -1937,6 +1937,7 @@ _Appears in:_ | --- | --- | --- | --- | | `discoveryUrl` _string_ | 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.
HTTPS is required because the registration endpoint resolved from this
document carries the initial access token and the issued client_secret
(RFC 7591 §3, RFC 8414 §3). MaxLength is a defensive size cap (etcd
object budget, regex evaluation cost) and matches the conventional URL
length cap. | | MaxLength: 2048
Pattern: `^https://[^\s?#]+[^/\s?#]$`
Optional: \{\}
| | `registrationEndpoint` _string_ | 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.
HTTPS is required because the registration endpoint carries the initial
access token and the issued client_secret (RFC 7591 §3, RFC 8414 §3).
MaxLength is a defensive size cap (etcd object budget, regex evaluation
cost) and matches the conventional URL length cap. | | MaxLength: 2048
Pattern: `^https://[^\s?#]+[^/\s?#]$`
Optional: \{\}
| +| `rotationKey` _string_ | RotationKey is an optional operator-chosen label that is part of the
cached registration's identity but is never sent to the upstream.
Change it to make the proxies register a fresh client at their next
start instead of reusing the cached one: for example after the
upstream has dropped the registration (it rejects the stale client_id
in the user's browser, so ToolHive cannot detect it) or when a
rotation is required. The previous registration's row stays in storage,
and upstream refresh tokens bound to the previous client_id stop
refreshing, so affected users sign in again. | | MaxLength: 64
Pattern: `^[A-Za-z0-9._-]+$`
Optional: \{\}
| | `initialAccessTokenRef` _[api.v1beta1.SecretKeyRef](#apiv1beta1secretkeyref)_ | 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: \{\}
| | `softwareId` _string_ | 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. | | MaxLength: 255
Optional: \{\}
| | `softwareStatement` _string_ | SoftwareStatement is the RFC 7591 "software_statement" JWT asserting
metadata about the client software, signed by a party the authorization
server trusts.
Stored inline on the CR. The JWT is signed but not encrypted, so its
contents are visible to anyone with get/list/watch on this resource and
appear in etcd backups in plaintext. Treat the value as non-confidential
(signed attestation, not a secret). Operators that rotate software
statements like bearer credentials should keep them at the authorization
server side and rely on the registration endpoint's initial access
token (see InitialAccessTokenRef) instead of placing them on the CR.
Bounded to 16384 characters as a defensive size cap (etcd object
budget, regex evaluation cost). Real-world signed statements with
embedded x5c certificate chains, JWKS keys, or OIDC-Federation
trust-framework metadata routinely exceed 4 KB. | | MaxLength: 16384
Optional: \{\}
| diff --git a/docs/server/docs.go b/docs/server/docs.go index 7536b61d25..3ab2827c04 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -157,6 +157,10 @@ const docTemplate = `{ "description": "RegistrationEndpoint is the RFC 7591 registration endpoint URL used\ndirectly, bypassing discovery. Because no discovery is performed,\nserver-capability fields (token_endpoint_auth_methods_supported,\nscopes_supported) are unavailable on this code path; the caller is\nexpected to also supply AuthorizationEndpoint, TokenEndpoint, and an\nexplicit Scopes list on the parent OAuth2UpstreamRunConfig. Auth\nmethod falls back to the resolver's default (client_secret_basic).\n\nMutually exclusive with DiscoveryURL.", "type": "string" }, + "rotation_key": { + "description": "RotationKey is an optional operator-chosen label (1-64 characters from\n[A-Za-z0-9._-]) that becomes part of the cached registration's identity\nwithout being sent to the upstream. Changing it makes the next start\nregister a fresh client instead of reusing the cached one. Use it when\nthe upstream has dropped the registration it issued — that rejection is\nonly visible in the user's browser, so the resolver cannot detect it —\nor when a registration must be rotated on request. The row written\nunder the previous value is left in place. Upstream refresh tokens\nbound to the previous client_id stop refreshing once the client\nchanges; affected users sign in again.", + "type": "string" + }, "software_id": { "description": "SoftwareID is the RFC 7591 \"software_id\" registration metadata value,\nidentifying the client software independent of any particular\nregistration instance.", "type": "string" diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 4068da9008..8ea58c4434 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -150,6 +150,10 @@ "description": "RegistrationEndpoint is the RFC 7591 registration endpoint URL used\ndirectly, bypassing discovery. Because no discovery is performed,\nserver-capability fields (token_endpoint_auth_methods_supported,\nscopes_supported) are unavailable on this code path; the caller is\nexpected to also supply AuthorizationEndpoint, TokenEndpoint, and an\nexplicit Scopes list on the parent OAuth2UpstreamRunConfig. Auth\nmethod falls back to the resolver's default (client_secret_basic).\n\nMutually exclusive with DiscoveryURL.", "type": "string" }, + "rotation_key": { + "description": "RotationKey is an optional operator-chosen label (1-64 characters from\n[A-Za-z0-9._-]) that becomes part of the cached registration's identity\nwithout being sent to the upstream. Changing it makes the next start\nregister a fresh client instead of reusing the cached one. Use it when\nthe upstream has dropped the registration it issued — that rejection is\nonly visible in the user's browser, so the resolver cannot detect it —\nor when a registration must be rotated on request. The row written\nunder the previous value is left in place. Upstream refresh tokens\nbound to the previous client_id stop refreshing once the client\nchanges; affected users sign in again.", + "type": "string" + }, "software_id": { "description": "SoftwareID is the RFC 7591 \"software_id\" registration metadata value,\nidentifying the client software independent of any particular\nregistration instance.", "type": "string" diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index c48331f45c..970aa2021a 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -203,6 +203,19 @@ components: Mutually exclusive with DiscoveryURL. type: string + rotation_key: + description: |- + RotationKey is an optional operator-chosen label (1-64 characters from + [A-Za-z0-9._-]) that becomes part of the cached registration's identity + without being sent to the upstream. Changing it makes the next start + register a fresh client instead of reusing the cached one. Use it when + the upstream has dropped the registration it issued — that rejection is + only visible in the user's browser, so the resolver cannot detect it — + or when a registration must be rotated on request. The row written + under the previous value is left in place. Upstream refresh tokens + bound to the previous client_id stop refreshing once the client + changes; affected users sign in again. + type: string software_id: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value, diff --git a/pkg/auth/dcr/request.go b/pkg/auth/dcr/request.go index 63094d9818..02746d150b 100644 --- a/pkg/auth/dcr/request.go +++ b/pkg/auth/dcr/request.go @@ -44,8 +44,9 @@ type Request struct { // exercised on this path. // // The resolver's cache key is (Issuer, UpstreamID, RedirectURI, - // ScopesHash), where UpstreamID is derived by the resolver from - // DiscoveryURL / RegistrationEndpoint (issue #5823). The two consumers + // ScopesHash) plus RotationKey when set, where UpstreamID is derived by + // the resolver from DiscoveryURL / RegistrationEndpoint (issue #5823). + // The two consumers // do not collide on the key because the embedded authserver's // RedirectURI lives on the authserver's origin and the CLI's lives on a // loopback (RFC 8252 §7.3) — even when Issuer happens to match between @@ -91,6 +92,15 @@ type Request struct { // Mutually exclusive with DiscoveryURL. RegistrationEndpoint string + // RotationKey is an optional operator-chosen label folded into the cache + // key (and the in-process singleflight key) when non-empty. It is never + // sent to the upstream. Changing it forces a fresh RFC 7591 registration + // on the next resolve, which is how an operator retires a registration + // the upstream no longer recognises without editing the credential store + // by hand. An empty value addresses the same cache row as a Request + // written before the field existed. + RotationKey string + // CodeChallengeMethodsSupported lists the PKCE challenge methods the // upstream advertises. Only consulted when RegistrationEndpoint is set; // on the DiscoveryURL branch the resolver reads this value from the diff --git a/pkg/auth/dcr/resolver.go b/pkg/auth/dcr/resolver.go index 05268b3f66..d432551418 100644 --- a/pkg/auth/dcr/resolver.go +++ b/pkg/auth/dcr/resolver.go @@ -34,9 +34,9 @@ // # Concurrency // // The package maintains a process-global singleflight keyed on the tuple -// (issuer, upstreamID, redirectURI, scopesHash) so concurrent +// (issuer, upstreamID, redirectURI, scopesHash, rotationKey) so concurrent // ResolveCredentials calls across all consumers in a single process -// coalesce when their cache keys match. Consumers that share all four +// coalesce when their cache keys match. Consumers that share all five // values will share a flight — the deduplication is a feature for the // embedded authserver but means callers cannot assume per-call-site flight // isolation. See the dcrFlight doc comment below for the rationale. @@ -127,7 +127,10 @@ var dcrFlight singleflight.Group // UpstreamID keeps two upstreams that share Issuer, RedirectURI, and // scopes from coalescing into a single flight (issue #5823) — the same // distinction the persisted DCRKey draws, so the singleflight and cache -// layers stay aligned. +// layers stay aligned. RotationKey, when non-empty, is appended +// length-prefixed exactly as redisDCRKey appends it, so a rotation also +// separates the in-flight registration from one still running under the +// previous key; an empty RotationKey leaves the flight key unchanged. // // PublicClient is intentionally NOT part of the flight key — the // dcrFlight doc above explains why: today's two consumers register on @@ -137,11 +140,15 @@ var dcrFlight singleflight.Group // DCRKey from cross-profile coalescence; the two layers are aligned // rather than asymmetric. func flightKeyOf(key Key) string { - return fmt.Sprintf("%d:%s:%d:%s:%d:%s:%s", + base := fmt.Sprintf("%d:%s:%d:%s:%d:%s:%s", len(key.Issuer), key.Issuer, len(key.UpstreamID), key.UpstreamID, len(key.RedirectURI), key.RedirectURI, key.ScopesHash) + if key.RotationKey == "" { + return base + } + return fmt.Sprintf("%s:%d:%s", base, len(key.RotationKey), key.RotationKey) } // defaultUpstreamRedirectPath is the redirect path derived from the issuer @@ -365,6 +372,7 @@ func ResolveCredentials( UpstreamID: upstreamID, RedirectURI: redirectURI, ScopesHash: storage.ScopesHash(scopes), + RotationKey: req.RotationKey, } // Cache lookup short-circuits before any network I/O. @@ -518,6 +526,7 @@ func registerAndCache( "local_issuer", req.Issuer, "upstream_id", key.UpstreamID, "redirect_uri", redirectURI, + "rotation_key", key.RotationKey, "client_id", authoritative.ClientID, ) return authoritative, nil @@ -746,6 +755,7 @@ func lookupCachedResolution( "local_issuer", localIssuer, "upstream_id", key.UpstreamID, "redirect_uri", redirectURI, + "rotation_key", key.RotationKey, "client_id", cached.ClientID, "dcr_age_days", ageDays, ) diff --git a/pkg/auth/dcr/resolver_test.go b/pkg/auth/dcr/resolver_test.go index 65d457fe40..4187928ca1 100644 --- a/pkg/auth/dcr/resolver_test.go +++ b/pkg/auth/dcr/resolver_test.go @@ -187,6 +187,85 @@ func TestResolveDCRCredentials_CacheHitShortCircuits(t *testing.T) { "cache hit must not issue any network I/O (discovery or registration)") } +// TestResolveDCRCredentials_RotationKeyForcesFreshRegistration pins the +// operator-driven rotation path: a Request whose RotationKey differs from the +// cached row's registers a new client, while the previous row stays +// reachable under its own key and neither row is re-registered on later +// resolves. +func TestResolveDCRCredentials_RotationKeyForcesFreshRegistration(t *testing.T) { + t.Parallel() + + var registrationCalls int32 + server := newDCRTestServer(t, dcrTestHandlerConfig{ + tokenEndpointAuthMethodsSupported: []string{"client_secret_basic"}, + observeRegistration: func(_ *http.Request, _ []byte) { + atomic.AddInt32(®istrationCalls, 1) + }, + }) + + cache := newMemoryDCRStore(t) + issuer := server.URL + request := func(rotationKey string) *Request { + return &Request{ + Issuer: issuer, + Scopes: []string{"openid"}, + DiscoveryURL: issuer + "/.well-known/oauth-authorization-server", + RotationKey: rotationKey, + } + } + + for i, step := range []struct { + rotationKey string + wantRegistrations int32 + }{ + {"", 1}, // first resolve registers + {"", 1}, // cache hit + {"r1", 2}, // rotation addresses a new row: fresh registration + {"r1", 2}, // cache hit under the rotation key + {"", 2}, // the pre-rotation row is still there + } { + _, err := ResolveCredentials(context.Background(), request(step.rotationKey), cache) + require.NoError(t, err, "step %d", i) + assert.Equal(t, step.wantRegistrations, atomic.LoadInt32(®istrationCalls), + "step %d (rotation_key=%q)", i, step.rotationKey) + } + + baseKey := Key{ + Issuer: issuer, + UpstreamID: issuer, + RedirectURI: issuer + "/oauth/callback", + ScopesHash: storage.ScopesHash([]string{"openid"}), + } + rotatedKey := baseKey + rotatedKey.RotationKey = "r1" + for _, key := range []Key{baseKey, rotatedKey} { + _, ok, err := cache.Get(context.Background(), key) + require.NoError(t, err) + assert.True(t, ok, "row for rotation_key=%q must be cached", key.RotationKey) + } +} + +// TestFlightKeyOf_RotationKey pins that the singleflight key tracks the +// persisted DCRKey: an empty RotationKey leaves the pre-rotation encoding +// untouched and a non-empty one appends a length-prefixed segment, so two +// rotations of the same upstream never coalesce into one in-flight +// registration. +func TestFlightKeyOf_RotationKey(t *testing.T) { + t.Parallel() + + key := Key{ + Issuer: "https://thv.example.com", + UpstreamID: "https://idp.example.com", + RedirectURI: "https://thv.example.com/oauth/callback", + ScopesHash: storage.ScopesHash([]string{"openid"}), + } + legacy := flightKeyOf(key) + assert.Equal(t, "23:https://thv.example.com:23:https://idp.example.com:38:https://thv.example.com/oauth/callback:"+key.ScopesHash, legacy) + + key.RotationKey = "r1" + assert.Equal(t, legacy+":2:r1", flightKeyOf(key)) +} + func TestResolveDCRCredentials_RegistersOnCacheMiss(t *testing.T) { t.Parallel() diff --git a/pkg/auth/dcr/store.go b/pkg/auth/dcr/store.go index d19a7b2842..eb67ee6685 100644 --- a/pkg/auth/dcr/store.go +++ b/pkg/auth/dcr/store.go @@ -17,7 +17,9 @@ import ( // dcrStaleAgeThreshold is the age beyond which a cached DCR resolution is // considered stale and logged as such by higher-level wiring. The store itself // does not expire or evict entries — RFC 7591 client registrations are -// long-lived and are only purged by explicit RFC 7592 deregistration. +// long-lived and are only purged by explicit RFC 7592 deregistration. An +// operator retires a cached registration by changing Request.RotationKey, +// which addresses a new row; the old row is left in place. const dcrStaleAgeThreshold = 90 * 24 * time.Hour // Key is a re-export of storage.DCRKey, kept as a package-local alias so @@ -30,8 +32,8 @@ const dcrStaleAgeThreshold = 90 * 24 * time.Hour type Key = storage.DCRKey // CredentialStore caches RFC 7591 Dynamic Client Registration resolutions -// keyed by the (Issuer, UpstreamID, RedirectURI, ScopesHash) tuple. -// Implementations must be safe for concurrent use. +// keyed by the (Issuer, UpstreamID, RedirectURI, ScopesHash) tuple plus the +// optional RotationKey. Implementations must be safe for concurrent use. // // This is the runner-facing interface used by the DCR resolver. It is a // narrow re-projection of storage.DCRCredentialStore that exchanges diff --git a/pkg/auth/dcr/store_test.go b/pkg/auth/dcr/store_test.go index bbcfe65c5b..e404940998 100644 --- a/pkg/auth/dcr/store_test.go +++ b/pkg/auth/dcr/store_test.go @@ -109,6 +109,10 @@ func TestStorageBackedStore_DistinctKeysDoNotCollide(t *testing.T) { RedirectURI: "https://toolhive.example.com/oauth/callback", ScopesHash: storage.ScopesHash([]string{"openid"}), } + // keyF is keyA under an operator-set RotationKey: the rotation must + // address its own entry while keyA's row stays reachable. + keyF := keyA + keyF.RotationKey = "r2" // The persisted *storage.DCRCredentials shape requires non-empty // AuthorizationEndpoint / TokenEndpoint per validateDCRCredentialsForStore; @@ -126,7 +130,7 @@ func TestStorageBackedStore_DistinctKeysDoNotCollide(t *testing.T) { key Key clientID string }{ - {keyA, "a"}, {keyB, "b"}, {keyC, "c"}, {keyD, "d"}, {keyE, "e"}, + {keyA, "a"}, {keyB, "b"}, {keyC, "c"}, {keyD, "d"}, {keyE, "e"}, {keyF, "f"}, } { _, err := store.PutIfAbsent(ctx, put.key, resolution(put.clientID)) require.NoError(t, err) @@ -141,6 +145,7 @@ func TestStorageBackedStore_DistinctKeysDoNotCollide(t *testing.T) { {keyC, "c"}, {keyD, "d"}, {keyE, "e"}, + {keyF, "f"}, } { got, ok, err := store.Get(ctx, tc.key) require.NoError(t, err) @@ -375,6 +380,7 @@ func TestResolutionCredentialsRoundTrip(t *testing.T) { UpstreamID: "https://upstream.example.com", RedirectURI: "https://thv.example.com/oauth/callback", ScopesHash: storage.ScopesHash([]string{"openid", "profile"}), + RotationKey: "2026-10", } original := &Resolution{ diff --git a/pkg/authserver/config.go b/pkg/authserver/config.go index fa204b1a46..3a0d1bd0e0 100644 --- a/pkg/authserver/config.go +++ b/pkg/authserver/config.go @@ -898,6 +898,18 @@ type DCRUpstreamConfig struct { // Mutually exclusive with DiscoveryURL. RegistrationEndpoint string `json:"registration_endpoint,omitempty" yaml:"registration_endpoint,omitempty"` + // RotationKey is an optional operator-chosen label (1-64 characters from + // [A-Za-z0-9._-]) that becomes part of the cached registration's identity + // without being sent to the upstream. Changing it makes the next start + // register a fresh client instead of reusing the cached one. Use it when + // the upstream has dropped the registration it issued — that rejection is + // only visible in the user's browser, so the resolver cannot detect it — + // or when a registration must be rotated on request. The row written + // under the previous value is left in place. Upstream refresh tokens + // bound to the previous client_id stop refreshing once the client + // changes; affected users sign in again. + RotationKey string `json:"rotation_key,omitempty" yaml:"rotation_key,omitempty"` + // InitialAccessTokenFile is the path to a file containing the RFC 7591 // initial access token presented to the registration endpoint. Mutually // exclusive with InitialAccessTokenEnvVar. Both may be omitted for open @@ -1625,9 +1637,19 @@ func (c *DCRUpstreamConfig) Validate() error { "dcr_config: initial_access_token_file and initial_access_token_env_var are mutually exclusive") } + // Mirrors the CRD's Pattern/MaxLength markers on rotationKey for the + // YAML and CLI run-config paths, which have no admission-time schema. + if c.RotationKey != "" && !dcrRotationKeyPattern.MatchString(c.RotationKey) { + return fmt.Errorf("dcr_config: rotation_key must be 1-64 characters from [A-Za-z0-9._-]") + } + return nil } +// dcrRotationKeyPattern bounds DCRUpstreamConfig.RotationKey to a short, +// log- and key-safe alphabet; the CRD enforces the same shape at admission. +var dcrRotationKeyPattern = regexp.MustCompile(`^[A-Za-z0-9._-]{1,64}$`) + func validateZeroUpstreamMode(upstreamCount, delegateClientCount int, issuers []tokenexchange.TrustedIssuer) error { if upstreamCount > 0 || delegateClientCount > 0 || JWTBearerGrantEnabled(issuers) { return nil diff --git a/pkg/authserver/config_test.go b/pkg/authserver/config_test.go index 96cf17e8d6..fa762f44d4 100644 --- a/pkg/authserver/config_test.go +++ b/pkg/authserver/config_test.go @@ -1293,6 +1293,31 @@ func TestDCRUpstreamConfigValidate(t *testing.T) { SoftwareStatement: "eyJhbGciOi...", }, }, + { + name: "rotation_key within the allowed alphabet passes validation", + config: DCRUpstreamConfig{ + RegistrationEndpoint: "https://idp.example.com/register", + RotationKey: "2026-10.ion_v2-A", + }, + }, + { + name: "rotation_key with characters outside the allowed alphabet rejects", + config: DCRUpstreamConfig{ + RegistrationEndpoint: "https://idp.example.com/register", + RotationKey: "rotate me", + }, + wantErr: true, + errMsg: "rotation_key must be 1-64 characters", + }, + { + name: "rotation_key longer than 64 characters rejects", + config: DCRUpstreamConfig{ + RegistrationEndpoint: "https://idp.example.com/register", + RotationKey: strings.Repeat("a", 65), + }, + wantErr: true, + errMsg: "rotation_key must be 1-64 characters", + }, { name: "both initial_access_token_file and initial_access_token_env_var rejects", config: DCRUpstreamConfig{ diff --git a/pkg/authserver/runner/dcr_adapter.go b/pkg/authserver/runner/dcr_adapter.go index 9a63b2a308..da9a00adba 100644 --- a/pkg/authserver/runner/dcr_adapter.go +++ b/pkg/authserver/runner/dcr_adapter.go @@ -87,6 +87,7 @@ func newDCRRequest(rc *authserver.UpstreamRunConfig, localIssuer string) (*dcr.R return &dcr.Request{ Issuer: localIssuer, RedirectURI: redirectURI, Scopes: scopes, DiscoveryURL: discoveryURL, RegistrationEndpoint: dcrConfig.RegistrationEndpoint, + RotationKey: dcrConfig.RotationKey, AuthorizationEndpoint: authorizationEndpoint, TokenEndpoint: tokenEndpoint, InitialAccessToken: initialAccessToken, CAFilePath: caFilePath, AllowPrivateIPs: allowPrivateIPs, }, nil diff --git a/pkg/authserver/runner/dcr_adapter_test.go b/pkg/authserver/runner/dcr_adapter_test.go index c65888ffd3..f2cc8b0682 100644 --- a/pkg/authserver/runner/dcr_adapter_test.go +++ b/pkg/authserver/runner/dcr_adapter_test.go @@ -242,6 +242,7 @@ func TestNewDCRRequest(t *testing.T) { wantRegistration string wantAllowPrivateIPs bool wantCAFilePath string + wantRotationKey string }{ { name: "discovery_url branch resolves file-based initial access token", @@ -289,6 +290,23 @@ func TestNewDCRRequest(t *testing.T) { wantAllowPrivateIPs: true, wantCAFilePath: "/var/run/toolhive/upstream-ca/ca.crt", }, + { + // Pins the RotationKey copy onto dcr.Request: without it a + // rotation configured on the run-config would silently keep + // addressing the pre-rotation cache row. + name: "rotation_key propagates from run-config", + rc: &authserver.OAuth2UpstreamRunConfig{ + Scopes: []string{"openid"}, + DCRConfig: &authserver.DCRUpstreamConfig{ + RegistrationEndpoint: "https://idp.example.com/register", + RotationKey: "2026-10", + }, + }, + localIssuer: "https://thv.example.com", + wantIssuer: "https://thv.example.com", + wantRegistration: "https://idp.example.com/register", + wantRotationKey: "2026-10", + }, { name: "nil run-config rejected", rc: nil, @@ -313,6 +331,7 @@ func TestNewDCRRequest(t *testing.T) { assert.Equal(t, tc.wantRegistration, req.RegistrationEndpoint) assert.Equal(t, tc.wantAllowPrivateIPs, req.AllowPrivateIPs) assert.Equal(t, tc.wantCAFilePath, req.CAFilePath) + assert.Equal(t, tc.wantRotationKey, req.RotationKey) }) } } diff --git a/pkg/authserver/storage/memory_test.go b/pkg/authserver/storage/memory_test.go index e0ebe20cd3..aae6dcee13 100644 --- a/pkg/authserver/storage/memory_test.go +++ b/pkg/authserver/storage/memory_test.go @@ -3086,6 +3086,13 @@ func TestMemoryStorage_DCRCredentials_DistinctKeysDoNotCollide(t *testing.T) { // inside one embedded authserver sharing issuer, redirect, and // scopes. Before UpstreamID joined the key these collided (#5823). mkCreds(mkKey("https://idp-a.example.com", "https://up-b", "https://x/cb", []string{"openid"}), "e"), + // Differs from entry "a" only by the operator-set RotationKey: a + // rotation addresses its own entry and leaves "a" reachable. + func() *DCRCredentials { + key := mkKey("https://idp-a.example.com", "https://up-a", "https://x/cb", []string{"openid"}) + key.RotationKey = "r2" + return mkCreds(key, "f") + }(), } for _, e := range entries { _, err := s.StoreDCRCredentialsIfAbsent(ctx, e) diff --git a/pkg/authserver/storage/redis.go b/pkg/authserver/storage/redis.go index c1ebfee205..ba4ac55045 100644 --- a/pkg/authserver/storage/redis.go +++ b/pkg/authserver/storage/redis.go @@ -1871,6 +1871,9 @@ type storedDCRCredentials struct { KeyUpstreamID string `json:"key_upstream_id"` KeyRedirectURI string `json:"key_redirect_uri"` KeyScopesHash string `json:"key_scopes_hash"` + // Omitted when empty so rows written before the field existed and rows + // without a rotation key share one wire shape. + KeyRotationKey string `json:"key_rotation_key,omitempty"` ProviderName string `json:"provider_name,omitempty"` @@ -1899,6 +1902,7 @@ func newStoredDCRCredentials(creds *DCRCredentials) storedDCRCredentials { KeyUpstreamID: creds.Key.UpstreamID, KeyRedirectURI: creds.Key.RedirectURI, KeyScopesHash: creds.Key.ScopesHash, + KeyRotationKey: creds.Key.RotationKey, ProviderName: creds.ProviderName, ClientID: creds.ClientID, ClientSecret: creds.ClientSecret, @@ -1935,6 +1939,7 @@ func (s *storedDCRCredentials) toDCRCredentials() *DCRCredentials { UpstreamID: s.KeyUpstreamID, RedirectURI: s.KeyRedirectURI, ScopesHash: s.KeyScopesHash, + RotationKey: s.KeyRotationKey, }, ProviderName: s.ProviderName, ClientID: s.ClientID, diff --git a/pkg/authserver/storage/redis_keys.go b/pkg/authserver/storage/redis_keys.go index 21c89162da..d883b59f96 100644 --- a/pkg/authserver/storage/redis_keys.go +++ b/pkg/authserver/storage/redis_keys.go @@ -134,6 +134,12 @@ func redisProviderKey(prefix, providerID, providerSubject string) string { // // Format: "{prefix}dcr:::::::" // +// When DCRKey.RotationKey is non-empty a length-prefixed +// "::" segment follows the scopes hash. An +// empty RotationKey yields exactly the format above, so rows written before +// the field existed keep resolving; the hex scopes hash contains no colon, so +// the two shapes cannot be confused. +// // The first three segments are length-prefixed to handle colons in UpstreamID // and RedirectURI (and, for symmetry, Issuer) without ambiguity, mirroring // redisProviderKey. ScopesHash is expected to be a SHA-256 hex digest produced @@ -163,12 +169,16 @@ func redisProviderKey(prefix, providerID, providerSubject string) string { // add the distinguishing component back to the key format alongside an // explicit migration story for existing Redis-cached entries. func redisDCRKey(prefix string, key DCRKey) string { - return fmt.Sprintf("%s%s:%d:%s:%d:%s:%d:%s:%s", + base := fmt.Sprintf("%s%s:%d:%s:%d:%s:%d:%s:%s", prefix, KeyTypeDCR, len(key.Issuer), key.Issuer, len(key.UpstreamID), key.UpstreamID, len(key.RedirectURI), key.RedirectURI, key.ScopesHash) + if key.RotationKey == "" { + return base + } + return fmt.Sprintf("%s:%d:%s", base, len(key.RotationKey), key.RotationKey) } // redisUpstreamKey generates a Redis key for a per-provider upstream token entry. diff --git a/pkg/authserver/storage/redis_test.go b/pkg/authserver/storage/redis_test.go index 459a969d3b..375e1322a7 100644 --- a/pkg/authserver/storage/redis_test.go +++ b/pkg/authserver/storage/redis_test.go @@ -3617,6 +3617,22 @@ func TestRedisKeyGeneration(t *testing.T) { "test:auth:dcr:23:https://thv.example.com:23:https://idp.example.com:38:https://thv.example.com/oauth/callback:abc123", result) }) + + t.Run("redisDCRKey with rotation key", func(t *testing.T) { + t.Parallel() + key := DCRKey{ + Issuer: "https://thv.example.com", + UpstreamID: "https://idp.example.com", + RedirectURI: "https://thv.example.com/oauth/callback", + ScopesHash: "abc123", + } + legacy := redisDCRKey("test:auth:", key) + key.RotationKey = "2026-10" + // A rotation key appends one length-prefixed segment after the scopes + // hash; the empty value must keep producing the pre-rotation key so + // rows written before the field existed stay reachable. + assert.Equal(t, legacy+":7:2026-10", redisDCRKey("test:auth:", key)) + }) } // TestRedisDCRKey_Distinct pins the colon-safe lookup contract: any pair of @@ -4106,6 +4122,33 @@ func TestRedisStorage_DCRCredentials_RoundTrip(t *testing.T) { // authoritative value, and the stored row is unchanged. This is the fix for // the concurrent-replica bug where two replicas racing a cache miss each // independently register a distinct RFC 7591 client for the same key. +// TestRedisStorage_DCRCredentials_RotationKeyRoundTrip pins that +// DCRKey.RotationKey survives the stored JSON shape and addresses its own +// Redis row: the same tuple without the rotation key must miss. +func TestRedisStorage_DCRCredentials_RotationKeyRoundTrip(t *testing.T) { + withRedisStorage(t, func(ctx context.Context, s *RedisStorage, _ *miniredis.Miniredis) { + key := dcrFixtureKey() + key.RotationKey = "2026-10" + + _, err := s.StoreDCRCredentialsIfAbsent(ctx, &DCRCredentials{ + Key: key, + ClientID: "rotated-client", + AuthorizationEndpoint: "https://idp.example.com/auth", + TokenEndpoint: "https://idp.example.com/token", + }) + require.NoError(t, err) + + got, err := s.GetDCRCredentials(ctx, key) + require.NoError(t, err) + assert.Equal(t, key, got.Key, "RotationKey must round-trip through the stored row") + assert.Equal(t, "rotated-client", got.ClientID) + + _, err = s.GetDCRCredentials(ctx, dcrFixtureKey()) + require.ErrorIs(t, err, ErrNotFound, + "the tuple without the rotation key must address a different row") + }) +} + func TestRedisStorage_DCRCredentials_FirstClaimWins(t *testing.T) { withRedisStorage(t, func(ctx context.Context, s *RedisStorage, _ *miniredis.Miniredis) { key := dcrFixtureKey() diff --git a/pkg/authserver/storage/types.go b/pkg/authserver/storage/types.go index 9cf571a699..c2bfc89c09 100644 --- a/pkg/authserver/storage/types.go +++ b/pkg/authserver/storage/types.go @@ -235,6 +235,17 @@ type DCRKey struct { // hand at call sites; the canonical form must be a single source of truth // so the key matches across processes and backends. ScopesHash string + + // RotationKey is an optional operator-chosen label that is part of the + // cache identity but carries no OAuth meaning and is never sent to the + // upstream. Changing it makes the resolver miss the cache and register a + // fresh client, which is the supported way to retire a registration the + // upstream has dropped (see authserver.DCRUpstreamConfig.RotationKey). + // Empty for every row written before the field existed; backends MUST + // encode an empty value exactly as they encoded a key without the field + // so those rows stay reachable. Rows written under a previous value are + // not deleted and follow the same lifetime rules as any superseded row. + RotationKey string } // ScopesHash returns the SHA-256 hex digest of the canonical OAuth scope set, @@ -364,7 +375,8 @@ func validateDCRCompareAndSet(creds, expected *DCRCredentials) error { // TestResolutionCredentialsRoundTrip in // pkg/auth/dcr/store_test.go. type DCRCredentials struct { - // Key is the canonical cache key: (Issuer, UpstreamID, RedirectURI, ScopesHash). + // Key is the canonical cache key: (Issuer, UpstreamID, RedirectURI, + // ScopesHash) plus the optional RotationKey. Key DCRKey // ProviderName is the upstream's UpstreamRunConfig.Name. Debug / audit diff --git a/sdk/go/client/oas_json_gen.go b/sdk/go/client/oas_json_gen.go index 982b632179..09724c42a1 100644 --- a/sdk/go/client/oas_json_gen.go +++ b/sdk/go/client/oas_json_gen.go @@ -1051,6 +1051,12 @@ func (s *AuthenticationDCRUpstreamConfig) encodeFields(e *jx.Encoder) { s.RegistrationEndpoint.Encode(e) } } + { + if s.RotationKey.Set { + e.FieldStart("rotation_key") + s.RotationKey.Encode(e) + } + } { if s.SoftwareID.Set { e.FieldStart("software_id") @@ -1065,13 +1071,14 @@ func (s *AuthenticationDCRUpstreamConfig) encodeFields(e *jx.Encoder) { } } -var jsonFieldsNameOfAuthenticationDCRUpstreamConfig = [6]string{ +var jsonFieldsNameOfAuthenticationDCRUpstreamConfig = [7]string{ 0: "discovery_url", 1: "initial_access_token_env_var", 2: "initial_access_token_file", 3: "registration_endpoint", - 4: "software_id", - 5: "software_statement", + 4: "rotation_key", + 5: "software_id", + 6: "software_statement", } // Decode decodes AuthenticationDCRUpstreamConfig from json. @@ -1122,6 +1129,16 @@ func (s *AuthenticationDCRUpstreamConfig) Decode(d *jx.Decoder) error { }(); err != nil { return errors.Wrap(err, "decode field \"registration_endpoint\"") } + case "rotation_key": + if err := func() error { + s.RotationKey.Reset() + if err := s.RotationKey.Decode(d); err != nil { + return err + } + return nil + }(); err != nil { + return errors.Wrap(err, "decode field \"rotation_key\"") + } case "software_id": if err := func() error { s.SoftwareID.Reset() diff --git a/sdk/go/client/oas_schemas_gen.go b/sdk/go/client/oas_schemas_gen.go index 100dfe2e66..be725515bf 100644 --- a/sdk/go/client/oas_schemas_gen.go +++ b/sdk/go/client/oas_schemas_gen.go @@ -622,6 +622,17 @@ type AuthenticationDCRUpstreamConfig struct { // method falls back to the resolver's default (client_secret_basic). // Mutually exclusive with DiscoveryURL. RegistrationEndpoint OptString `json:"registration_endpoint"` + // RotationKey is an optional operator-chosen label (1-64 characters from + // [A-Za-z0-9._-]) that becomes part of the cached registration's identity + // without being sent to the upstream. Changing it makes the next start + // register a fresh client instead of reusing the cached one. Use it when + // the upstream has dropped the registration it issued — that rejection is + // only visible in the user's browser, so the resolver cannot detect it — + // or when a registration must be rotated on request. The row written + // under the previous value is left in place. Upstream refresh tokens + // bound to the previous client_id stop refreshing once the client + // changes; affected users sign in again. + RotationKey OptString `json:"rotation_key"` // SoftwareID is the RFC 7591 "software_id" registration metadata value, // identifying the client software independent of any particular // registration instance. @@ -652,6 +663,11 @@ func (s *AuthenticationDCRUpstreamConfig) GetRegistrationEndpoint() OptString { return s.RegistrationEndpoint } +// GetRotationKey returns the value of RotationKey. +func (s *AuthenticationDCRUpstreamConfig) GetRotationKey() OptString { + return s.RotationKey +} + // GetSoftwareID returns the value of SoftwareID. func (s *AuthenticationDCRUpstreamConfig) GetSoftwareID() OptString { return s.SoftwareID @@ -682,6 +698,11 @@ func (s *AuthenticationDCRUpstreamConfig) SetRegistrationEndpoint(val OptString) s.RegistrationEndpoint = val } +// SetRotationKey sets the value of RotationKey. +func (s *AuthenticationDCRUpstreamConfig) SetRotationKey(val OptString) { + s.RotationKey = val +} + // SetSoftwareID sets the value of SoftwareID. func (s *AuthenticationDCRUpstreamConfig) SetSoftwareID(val OptString) { s.SoftwareID = val diff --git a/sdk/go/openapi.json b/sdk/go/openapi.json index fae661db51..d455c740fb 100644 --- a/sdk/go/openapi.json +++ b/sdk/go/openapi.json @@ -231,6 +231,10 @@ "description": "RegistrationEndpoint is the RFC 7591 registration endpoint URL used\ndirectly, bypassing discovery. Because no discovery is performed,\nserver-capability fields (token_endpoint_auth_methods_supported,\nscopes_supported) are unavailable on this code path; the caller is\nexpected to also supply AuthorizationEndpoint, TokenEndpoint, and an\nexplicit Scopes list on the parent OAuth2UpstreamRunConfig. Auth\nmethod falls back to the resolver's default (client_secret_basic).\n\nMutually exclusive with DiscoveryURL.", "type": "string" }, + "rotation_key": { + "description": "RotationKey is an optional operator-chosen label (1-64 characters from\n[A-Za-z0-9._-]) that becomes part of the cached registration's identity\nwithout being sent to the upstream. Changing it makes the next start\nregister a fresh client instead of reusing the cached one. Use it when\nthe upstream has dropped the registration it issued — that rejection is\nonly visible in the user's browser, so the resolver cannot detect it —\nor when a registration must be rotated on request. The row written\nunder the previous value is left in place. Upstream refresh tokens\nbound to the previous client_id stop refreshing once the client\nchanges; affected users sign in again.", + "type": "string" + }, "software_id": { "description": "SoftwareID is the RFC 7591 \"software_id\" registration metadata value,\nidentifying the client software independent of any particular\nregistration instance.", "type": "string" diff --git a/sdk/go/openapi.yaml b/sdk/go/openapi.yaml index b58933334d..0ef28479e0 100644 --- a/sdk/go/openapi.yaml +++ b/sdk/go/openapi.yaml @@ -279,6 +279,19 @@ components: Mutually exclusive with DiscoveryURL. type: string + rotation_key: + description: |- + RotationKey is an optional operator-chosen label (1-64 characters from + [A-Za-z0-9._-]) that becomes part of the cached registration's identity + without being sent to the upstream. Changing it makes the next start + register a fresh client instead of reusing the cached one. Use it when + the upstream has dropped the registration it issued — that rejection is + only visible in the user's browser, so the resolver cannot detect it — + or when a registration must be rotated on request. The row written + under the previous value is left in place. Upstream refresh tokens + bound to the previous client_id stop refreshing once the client + changes; affected users sign in again. + type: string software_id: description: |- SoftwareID is the RFC 7591 "software_id" registration metadata value,