Skip to content

Token-exchange grant is unreachable: no way to provision a confidential client #6082

Description

@jhrozek

Problem

The RFC 8693 token-exchange grant is implemented and tested, but unreachable in production. Two independent blockers:

1. No way to create a client that can use the grant

tokenexchange/handler.go:88 rejects public clients; :97 requires the grant. Nothing in production can produce a client satisfying both:

Path Why not
DCR (/oauth/register) handlers/dcr.go:108 hardcodes Public: true; registration/dcr.go:162-167 forces token_endpoint_auth_method: none; :94-97 limits grants to authorization_code/refresh_token; :118-123 makes redirect_uris mandatory, so a token-exchange-only client is inexpressible
CIMD Also public-only — storage/cimd_decorator.go:262 sets Public: true, :140 rejects any auth method but none
RunConfig No client-seeding field exists
registration.New(Public: false) Works, but called only from tests (integration_test.go withExtraClient) and server/doc.go:60

This predates #5989 and affects the self-issued subject-token path too, not just external issuers.

2. Discovery advertises neither the grant nor secret-based client auth

handlers/discovery.go:111-116 publishes grant_types_supported: [authorization_code, refresh_token] and token_endpoint_auth_methods_supported: ["none"]. The token-exchange handler is always registered (server_impl.go:248), so this is a pre-existing metadata bug: even with a confidential client, any metadata-driven OAuth library will refuse to attempt the grant or secret auth. Only a hand-rolled client ignoring discovery works.

Options

Four were designed. All four make the grant reachable; they differ in credential type and who may create a client.

A. Pre-provisioned clients in RunConfig

Operator declares client_id + a secret reference (file/env, never inline) + grants/scopes/audiences; registered at startup. Reuses registration.New(Public:false) + resolveSecret, both already exercised.

  • IDs: operator-chosen, stable
  • Serves: every deployment mode (thv run, proxyrunner, Compose, operator)
  • Cost: ~350–400 LOC; no new concepts
  • Gap: rotation and removal need a restart

B. Operator CRD field

delegateClients on the existing EmbeddedAuthServerConfig (embedded by both MCPExternalAuthConfig.Spec.EmbeddedAuthServer and VirtualMCPServer.Spec.AuthServerConfig), consuming a pre-existing Secret via the existing SecretKeyRef. Writes into option A's RunConfig field — it is an adapter on A, not an alternative.

  • IDs: operator-chosen, stable; enables an admission-time CEL cross-check that every allowedDelegateClients entry names a declared client
  • No new RBACget;list;watch on secrets already present on all three controllers
  • Explicitly not a new CRD: a client CRD needs a targetRef, but there is no auth-server object to point at — the embedded AS is a field on a workload, and one MCPExternalAuthConfig spawns N AS processes with different issuers, audiences and storage prefixes. targetRef is ambiguous by construction, and targetRef.namespace would create a cross-namespace escalation path an embedded field structurally cannot express.
  • Nothing for non-Kubernetes deployments

C. Confidential DCR (operator-provisioned initial access token)

Gate confidential registration behind a bearer IAT; add a confidential grant tier; generate a 32-byte secret returned once. RFC 7591 permits all of it (§3 open registration is only a SHOULD; §2 lists extension grant URIs; §2's redirect_uris MUST is conditional on using a redirect flow).

  • IDs: uuid.NewString() today — unknowable ahead of time. Fixable by honouring software_id as the client_id for IAT-authenticated requests.
  • Only option that works when the client set is unknown at deploy time (self-service fleets, third-party agents)
  • Introduces a standing root credential whose compromise mints delegation-capable clients
  • Should not ship before a revocation path or finite client TTL exists — "cannot revoke a leaked delegation-capable client" is worse than "cannot create one"

D. Non-secret client authentication (mTLS / SPIFFE) — prior art exists

The unmerged spiffee-authserver branch (62 commits, Apr 2026, no PR; partially harvested via #5713 and #5881) already solved both blockers, by sidestepping the secret entirely:

  • A custom fosite.ClientAuthenticationStrategy (pkg/authserver/spiffe/strategy.go:33) authenticates the client from a verified X.509-SVID and delegates to the default strategy when no SPIFFE ID is present, so browser flows are untouched.
  • Clients auto-register just-in-time with Public: false and no secret (strategy.go:155), enabled by a three-line seam in shared code: registration.Config.SkipSecretHash, changing if !cfg.Public to if !cfg.Public && !cfg.SkipSecretHash (registration/client.go:162).
  • The client ID is the SPIFFE ID — stable and writable into config before the client ever connects.
  • It also made discovery metadata dynamic (handlers/discovery.go:96-150), advertising the token-exchange grant and tls_client_auth/spiffe auth methods — i.e. it fixed blocker 2.
  • Fail-closed verified: a caller presenting client_id=spiffe://… without a client cert falls through to the default strategy and fails on bcrypt.CompareHashAndPassword(nil, …). The public-client rejection was not relaxed.
  • Two weaknesses to fix before lifting: policy == nil means allow-all, and every auto-registered client receives all supported scopes and all allowed audiences — no per-identity narrowing.

Recommendation

  1. Land A first. It is the substrate: B is an adapter on it, and A is the only option serving non-Kubernetes deployments. Both the CRD and DCR designs independently concluded A should ship first.
  2. Then B, for Kubernetes ergonomics and the admission-time cross-check.
  3. Fix blocker 2 alongside A — advertise the grant unconditionally (the handler is always registered) and secret-based auth methods only when a confidential client is configured, so public-only deployments' metadata stays byte-identical.
  4. D is the strongest answer for Kubernetes and is complementary rather than competing — SkipSecretHash is a three-line seam, and identity-as-client-ID is worth adopting regardless of credential type. Consider harvesting it as its own issue.
  5. C last, scoped to the dynamic-fleet case, and not before revocation or a finite TTL.

Effect on #5989

#5989 added an optional per-issuer allowed_delegate_clients (ToolHive client IDs permitted to exchange a given trusted issuer's tokens). It is optional because the IDs it names cannot currently be created.

  • Under A, B or D — all of which give stable, operator-chosen IDs — it can become required, or better: enforce the invariant from the other side, so every ID named must resolve to a declared client. A typo currently reverts silently to permissive behaviour.
  • Under C as it stands it cannot, since IDs don't exist until after registration.
  • It also has no CRD field or converter mapping yet (controllerutil/authserver.go:630 maps AllowedActors and stops), so it is reachable only from a hand-written RunConfig. That should land with B.

Separate bugs found during design — worth splitting out

All verified in shipped code, all independent of which option wins:

  1. ClientRegistry.RegisterClient documents ErrAlreadyExists but both backends silently overwritestorage/types.go:534 vs memory.go:345 (unconditional map assignment) and redis.go:220 (plain SET). Any caller trusting the contract replaces an existing client's secret instead of failing. Needs SET NX / check-under-lock, or an honest doc.
  2. No DeleteClient anywhere, and confidential clients are stored with ttl = 0 (redis.go:212-220) — a confidential client cannot be revoked and never expires.
  3. Every DCR client is registered for the server's entire AllowedAudiences (handlers/dcr.go:112), making the per-client audience ceiling vacuous for DCR clients.
  4. Confidential clients get a bare *fosite.DefaultClient (registration/client.go:182), which doesn't implement fosite.OpenIDConnectClient, so fosite skips auth-method negotiation and accepts either client_secret_basic or client_secret_post regardless of registration. Redis also persists no auth method (redis.go:167-190).
  5. In-memory storage loses confidential clients on restart (memory.go:342), so any config naming them goes stale every boot — effectively mandating Redis for options A–C.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions