From 0e1c1eaee9fcb65f555ab5ebcc5f3cfd37df485f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 15 Jul 2026 11:02:30 +0100 Subject: [PATCH 1/6] Add stateless transport proxy RFC Proposes dual-protocol operation in the transparent and streamable transport proxies so ToolHive serves both the current (2025-11-25) and the stateless 2026-07-28 MCP revision on one endpoint, discriminating per request. Covers the revision classifier, the client x backend dual-era matrix (diagonal cells in scope, off-diagonal deferred to vMCP), security (per-request response correlation), and the relationship to the existing --stateless flag. Co-Authored-By: Claude Opus 4.8 --- rfcs/THV-XXXX-stateless-transport-proxies.md | 373 +++++++++++++++++++ 1 file changed, 373 insertions(+) create mode 100644 rfcs/THV-XXXX-stateless-transport-proxies.md diff --git a/rfcs/THV-XXXX-stateless-transport-proxies.md b/rfcs/THV-XXXX-stateless-transport-proxies.md new file mode 100644 index 0000000..8a81189 --- /dev/null +++ b/rfcs/THV-XXXX-stateless-transport-proxies.md @@ -0,0 +1,373 @@ +# RFC-XXXX: Stateless Transport Proxy Support (MCP 2026-07-28) + +- **Status**: Draft +- **Author(s)**: Jakub Hrozek (@jhrozek) +- **Created**: 2026-07-15 +- **Last Updated**: 2026-07-15 +- **Target Repository**: toolhive +- **Related Issues**: [toolhive#5755](https://github.com/stacklok/toolhive/issues/5755) + +## Summary + +The upcoming MCP 2026-07-28 revision is stateless: it removes the `initialize` +handshake, protocol sessions, and the GET SSE stream, replacing them with +per-request `_meta` metadata and a `server/discover` RPC. This RFC proposes making +ToolHive's transparent and streamable transport proxies serve both the current +(2025-11-25) and the stateless revision on the same endpoint, discriminating per +request rather than at a cutover. + +## Problem Statement + +The 2026-07-28 revision removes exactly the mechanisms the transport proxies are +built around: + +- the `initialize` handshake (replaced by per-request `_meta` carrying + `protocolVersion`, `clientInfo`, `clientCapabilities`, plus a `server/discover` RPC), +- protocol sessions and the `Mcp-Session-Id` header (traffic becomes stateless), and +- the standalone GET SSE stream. + +ToolHive's proxies currently treat the `initialize` request as a trigger for session +creation, backend-pod pinning (Kubernetes multi-replica affinity), and health-check +enablement. Under the new revision that trigger never arrives. + +Crucially, the ecosystem will straddle both revisions for a long time. SDKs (go-sdk +v1.7, TS SDK v2) serve both on one endpoint, and clients, servers, and backends will +upgrade independently. ToolHive therefore cannot do a flag-day cutover; it must +discriminate the revision **per request** so a 2025-11-25 peer keeps working unchanged +while a 2026-07-28 peer is served natively. Everyone running ToolHive as a proxy in +front of MCP servers is affected once either side of the connection upgrades. + +## Goals + +- Serve 2025-11-25 (Legacy) and 2026-07-28 (Modern) traffic on the same proxy endpoint, + chosen per request. +- Zero behavior change for existing 2025-11-25 traffic. +- Add the minimum new machinery: a per-request revision classifier and revision-aware + handling in the two proxies. +- Preserve the existing per-request response-correlation confidentiality invariant. +- Remove, for Modern traffic, the Kubernetes cross-replica affinity burden that only + exists to support session-ful backends. + +## Non-Goals + +- vMCP stateless support and the cross-generation bridge (a client on either revision + reaching a backend on either revision) — tracked separately ([toolhive#5756]). +- The go-sdk v1.7 adoption / `mcpcompat` stateless plumbing ([toolhive#5754]). This RFC + argues the transport-proxy work is independent of it (see Compatibility / Alternatives). +- OAuth RFC 9207 `iss` validation ([toolhive#5760]) — genuine but unrelated work in + ToolHive's own OAuth client. +- MRTR, Tasks, the `x-mcp-header` tool-parameter mirroring extension, and caching metadata. +- Off-diagonal era translation in the transport proxies (see Proposed Solution) — deferred. + +## Proposed Solution + +### High-Level Design + +Each request is classified per-request as **Modern** (2026-07-28, stateless) or +**Legacy** (≤2025-11-25, `initialize`-based). Legacy traffic follows today's code +paths unchanged; Modern traffic takes a stateless path. Classification is a pure +function called at the points where the proxies already decode the JSON-RPC body — the +proxies parse bodies themselves (`golang.org/x/exp/jsonrpc2`), so this lives in +ToolHive-owned code rather than in an SDK. + +```mermaid +flowchart TD + Req[Incoming request] --> Meta{_meta has
protocolVersion?} + Meta -->|yes| Modern[Modern: stateless path
no session, per-request token] + Meta -->|no| Init{method ==
initialize?} + Init -->|yes| LegacyNew[Legacy: create session] + Init -->|no| LegacyExisting[Legacy: existing-session path] +``` + +The proxy fronts one backend and serves whatever client connects; the two sides have +independent revisions, giving a client × backend matrix: + +| Client | Backend | Proxy behavior | +|--------|---------|----------------| +| Legacy | Legacy | Today's path, unchanged. | +| Modern | Modern | Stateless pass-through. | +| Legacy | Modern | Gateway must terminate the handshake itself (deferred). | +| Modern | Legacy | Gateway must synthesize `initialize` toward the backend (deferred). | + +The two diagonal cells are the scope of this RFC. The off-diagonal cells require the +gateway to translate between eras; they are deferred (see Alternatives). The MCP +specification's probe-then-fallback mechanism is normative but only protects a client +talking *directly* to a server — its compatibility matrix marks the Legacy-client / +Modern-server case as "Fails" — so only a gateway could bridge it. The agentgateway +project reached the same conclusion: it ships the diagonals and the Modern-client / +Legacy-server synthetic-initialize path, and deliberately declined the Legacy-client / +Modern-server case as unsound. + +### Detailed Design + +#### Component Changes + +**Classifier (`pkg/mcp`).** A pure function, called at the existing decode points +(the transparent proxy's `detectInitialize` inside `RoundTrip`, the streamable proxy's +`resolveSessionForRequest`). Neither proxy reads the shared `ParsedMCPRequest` on its +routing path, so classification is not threaded through a context object. A connection +does not change revision mid-stream, so the transparent proxy classifies once and +stores the result in session metadata. + +**Streamable proxy (HTTP→stdio).** For Modern requests, unconditionally mint a fresh +per-request routing token and ignore any client-supplied `Mcp-Session-Id`. The proxy +already has a sessionless branch, but it is guarded on the absence of a session header; +Modern is sessionless by definition, so the header must be ignored rather than trusted +(see Security Considerations). No `Mcp-Session-Id` is minted and the session manager is +not consulted. + +**Transparent proxy (HTTP→HTTP).** Modern requests never mint a session, so the +sticky-session machinery (session guard, pod-pinning, init-body storage, re-init replay, +backend-SID rewrite) simply does not engage. Because there is no shared pipe, Go's +`http.Transport` correlates responses to requests natively and the Modern path collapses +to a plain reverse proxy. The internal health-monitor gate must not "default ready" for +Modern (that would start the monitor before the backend is warm and risk self-stopping +the proxy); it should gate off the first successful backend contact. + +**Parser vocabulary.** Register `server/discover` (currently unlabeled, yielding an +empty resource ID) and the mandatory Modern request headers `Mcp-Method` / `Mcp-Name` +so they are labeled for authz/audit; source `clientInfo`/protocolVersion from `_meta` +for Modern requests. + +#### API Changes + +Internal API only; no user-facing or CRD API changes. + +```go +// pkg/mcp +type Revision int // Legacy, Modern + +// ClassifyRevision decides the MCP era of a single request from its method, +// decoded _meta, and (on Streamable HTTP) the MCP-Protocol-Version header. +func ClassifyRevision(method string, meta map[string]any, protoHeader string) (Revision, error) +``` + +Classification rule: **Modern** iff `_meta` carries +`io.modelcontextprotocol/protocolVersion` (required on every Modern request); on +Streamable HTTP that value is mirrored into the `MCP-Protocol-Version` header, and a +header/body mismatch is rejected with `400` and JSON-RPC error `-32020` (the body is +authoritative). **Legacy** otherwise, with `method == "initialize"` marking session +start. A malformed or absent `_meta` classifies as Legacy — the safe direction. + +#### Configuration Changes + +None. The classifier is auto-detected from request content and needs no CRD, operator, +or CLI configuration. The existing `--stateless` run flag is orthogonal (see +Compatibility). + +#### Data Model Changes + +None. The session manager (`pkg/transport/session`) is trigger-agnostic and unchanged; +only the proxy call sites that create sessions move behind the Legacy branch. + +## Security Considerations + +### Threat Model + +- **Attacker**: an authenticated client of the proxy (authentication is unaffected — + see below), attempting to read another client's responses or ride another client's + routing state. +- **Primary threat**: response cross-delivery in the streamable proxy. Responses arriving + over the single shared stdio pipe are demultiplexed to the waiting HTTP handler by a + composite key of (routing token, JSON-RPC id). If two concurrent requests share a + routing token and reuse a JSON-RPC id (trivial — ids commonly start at `1`), the second + overwrites the first's waiter entry and one client's response payload is delivered to + the other. + +### Authentication and Authorization + +- Authentication is **not** on the session/initialize axis. It runs in the middleware + chain before request routing, so making Modern traffic sessionless does not move any + request off the authenticated path. +- The per-request `_meta` fields (`protocolVersion`, `clientInfo`, `clientCapabilities`) + and the mirrored headers are client-controlled and now appear on every request. They + are telemetry and labels, **never a security principal**; authorization must continue + to key off the authenticated identity, not client-asserted `_meta`. +- Registering `server/discover` in the parser method tables is partly a security measure: + an unknown method currently yields an empty resource ID, so authorization must + default-deny rather than match a broad allow rule. + +### Data Security + +The routing token is an in-process correlation nonce, not persisted and not exposed to +clients (the proxy never returns it as a session header on the Modern path). Its +uniqueness is what prevents the cross-delivery leak described in the threat model. + +### Input Validation + +- `_meta` is decoded with guarded type assertions; malformed or absent metadata + classifies as Legacy, preserving the existing session guard (fail-safe direction). +- `Mcp-Method` / `Mcp-Name` (and `MCP-Protocol-Version`) are validated against the body; + mismatch is rejected with `-32020`. As an intermediary that reads bodies for + authz/audit, the proxy should verify that the protocol version indicates a + header-validation-required revision before trusting any mirrored header value. + +### Secrets Management + +Not applicable — this change handles no secrets or credentials. + +### Audit and Logging + +`clientInfo` moves from a once-per-session handshake value to a per-request `_meta` +field. Audit labeling must source it per request. It remains a structured log field +(not interpolated into messages), so it introduces no log-forging vector. + +### Mitigations + +- **Streamable proxy**: for Modern requests, unconditionally mint a fresh UUID routing + token and ignore any client-supplied `Mcp-Session-Id`. A unique token guarantees a + unique composite key even when clients reuse JSON-RPC ids, closing the cross-delivery + leak by construction. Ignoring the header is safe because Modern has no legitimate + session header. +- **Transparent proxy**: gate the backend-SID rewrite on Legacy so a Modern request's + session header can never be rewritten onto a pinned backend. +- **Verification**: a concurrency test asserting that two concurrent Modern requests + sharing a JSON-RPC id — one carrying a foreign session header — never cross-deliver. + +## Alternatives Considered + +### Alternative 1: Extend the existing `--stateless` flag as the axis + +- Description: reuse ToolHive's per-server `--stateless` flag ("POST-only, no SSE") as + the stateless switch instead of a per-request classifier. +- Pros: reuses existing mechanism (GET/DELETE method gate, POST-ping health check). +- Cons: `--stateless` is a per-server, operator-set declaration; it cannot express a + dual-era endpoint where a Legacy client (which needs GET SSE) and a Modern request + arrive concurrently. Its server-global GET/DELETE gate would reject a Legacy client's + SSE stream. It also does not touch the POST session/initialize path at all. +- Why not chosen: it is orthogonal to protocol era. The classifier composes with it + additively; the one interaction is that the global GET/DELETE gate must become + revision-aware under dual-era. + +### Alternative 2: Do stateless handling in the SDK / gate on go-sdk v1.7 + +- Description: rely on go-sdk v1.7's stateless mode instead of ToolHive proxy code, and + sequence this work behind the SDK bump. +- Pros: less ToolHive-side code; SDK handles discover/MRTR/caching internally. +- Cons: the transport proxies' routing/session/forwarding path never constructs an + mcpcompat/go-sdk client or server — it is hand-rolled JSON-RPC over + `golang.org/x/exp/jsonrpc2`. The SDK's stateless mode governs SDK-built servers/clients, + not ToolHive's proxy data path. Gating on a prerelease SDK would also delay work that + has no actual dependency on it. +- Why not chosen: the proxy work is SDK-independent and can proceed in parallel with the + bump. (The SDK bump remains relevant for ToolHive's client-role code and for vMCP.) + +### Alternative 3: Full bidirectional era bridge in the transport proxies + +- Description: implement all four matrix cells, including gateway-side translation for + Legacy-client / Modern-server and Modern-client / Legacy-server. +- Pros: any client reaches any backend regardless of era. +- Cons: the Legacy-client / Modern-server cell requires the gateway to synthesize and + hold session state the spec has no client-side fallback for; agentgateway assessed the + equivalent as "theoretically unsound" and declined it. It is also more naturally a + gateway-aggregation concern than a transport-proxy one. +- Why not chosen: deferred; the cross-generation bridge belongs to vMCP ([toolhive#5756]). + This RFC scopes to the diagonal cells and relies on the spec-normative client fallback + for Modern-client / Legacy-server. + +## Compatibility + +### Backward Compatibility + +Fully backward compatible. All traffic classifies as Legacy unless a request carries +Modern `_meta`, so 2025-11-25 clients, servers, and backends behave exactly as today. +No CRD or operator change is required (the classifier is auto-detected), so a rolling +proxy upgrade is safe: old and new proxy pods can share one Service and session store, +and Modern traffic adds no session writes. + +**Dependency note**: [toolhive#5755] currently lists a dependency on the go-sdk v1.7 +adoption issue ([toolhive#5754]). Per Alternative 2, the transport-proxy work does not +depend on it and this RFC proposes decoupling that edge. + +### Forward Compatibility + +The binary Modern/Legacy classification suffices while there is a single Modern revision. +When a second ships, `DiscoverResult.supportedVersions` and +`UnsupportedProtocolVersionError.data.supported` are version lists, so the classifier +would need real version-string negotiation rather than a boolean; the `ClassifyRevision` +signature accommodates this by returning a richer `Revision` if needed. + +## Implementation Plan + +### Phase 1: Classifier and parser vocabulary (no behavior change) + +- Add `ClassifyRevision`; wire it at the decode points; store `Revision` in transparent-proxy + session metadata. +- Register `server/discover`, `Mcp-Method`, `Mcp-Name` in the parser; source `clientInfo` + from `_meta` for Modern requests. + +### Phase 2: Streamable proxy — Modern is stateless + +- Unconditionally mint a per-request routing token and ignore client `Mcp-Session-Id` + for Modern; do not mint a session. + +### Phase 3: Transparent proxy — gate the initialize-triggered machinery + +- Gate the session guard, pod-pinning, re-init replay, and backend-SID rewrite on Legacy; + gate the health monitor off first backend contact rather than defaulting ready. + +### Phase 4 (deferred): Off-diagonal translation + +- Only if the cross-era cells are taken on for the transport proxies; otherwise this stays + with vMCP. + +### Dependencies + +None blocking. Independent of [toolhive#5754] (see Compatibility). Off-diagonal work, if +pursued, needs backend-revision detection (probe `server/discover` on stdio backends; +inspect the `400` body on HTTP backends). + +## Testing Strategy + +- **Unit**: `ClassifyRevision` truth table (Modern `_meta`, absent/malformed `_meta`, + `initialize`, header/body mismatch → `-32020`). +- **Integration**: dual-era matrix — Legacy and Modern requests against the same endpoint; + verify Legacy session behavior is unchanged and Modern requests mint no session and emit + no `Mcp-Session-Id`. +- **Security**: concurrency test asserting no response cross-delivery for concurrent Modern + requests sharing a JSON-RPC id, one carrying a foreign session header. +- **Kubernetes**: Modern request to a multi-replica backend routes without pod-pinning and + does not re-initialize on backend `404`; the internal monitor does not self-stop on a + cold-starting Modern backend. + +## Documentation + +- Architecture doc: `docs/arch/stateless-transport-proxies-design.md` (already drafted, + [toolhive#5808]) — this RFC is its decision-level counterpart. +- Update `docs/arch/03-transport-architecture.md` when Phase 2/3 land. + +## Open Questions + +1. Should the revision-aware GET/DELETE gate on the transparent proxy be a refinement of + the existing `statelessMethodGate`, or a distinct mechanism keyed on `MCP-Protocol-Version`? +2. Timing: file the Phase 1–3 implementation issues on acceptance, or after go-sdk v1.7 + final (given the spec is still finalizing)? +3. Do we want the optional future tie-in where `--stateless` implies an "assume Modern" + fast-path, or keep the two strictly orthogonal? + +## References + +- MCP draft specification: `basic/versioning` (era terminology, compatibility matrix, + fallback), `server/discover`, `basic/transports/{stdio,streamable-http}`. +- Prior art — agentgateway: `crates/agentgateway/src/mcp/README.md` and `session.rs` + (`stateless_send_and_initialize`); evolution via issue #221 and PRs #262, #2365, #2477. +- ToolHive architecture doc: `docs/arch/stateless-transport-proxies-design.md` + ([toolhive#5808]). +- Related: [toolhive#5755] (this work), [toolhive#5756] (vMCP stateless), [toolhive#5754] + (go-sdk v1.7), [toolhive#5760] (OAuth `iss`). + +--- + +## RFC Lifecycle + +### Review History + +| Date | Reviewer | Decision | Notes | +|------|----------|----------|-------| +| 2026-07-15 | — | Draft | Initial submission | + +### Implementation Tracking + +| Repository | PR | Status | +|------------|-----|--------| +| toolhive | — | Not started | From 29d433d32eb5985cbebbd2ebd12a9b0af136dd4d Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 15 Jul 2026 11:34:48 +0100 Subject: [PATCH 2/6] Refine stateless transport RFC after verification review Fixes claims found inaccurate when verified against the MCP draft spec, ToolHive source, and agentgateway: - Correct the agentgateway prior-art attribution: it does not assess Legacy-client/Modern-server as unsound; cite the spec compatibility matrix ("Fails", no client fall-forward) instead. - Reconcile the client x backend matrix: Modern-client/Legacy-server resolves via the client's own probe-then-fallback (no proxy work), leaving only Legacy-client/Modern-server unbridged. - Qualify Mcp-Name as conditionally required (tools/call, resources/read, prompts/get) rather than universally mandatory. - Reframe the health-monitor note: neither existing readiness trigger fires on the Modern path, so it must gate off first backend contact. - Correct the forward-compat note: version negotiation is a Revision type/signature change, not drop-in. Co-Authored-By: Claude Opus 4.8 --- rfcs/THV-XXXX-stateless-transport-proxies.md | 52 +++++++++++--------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/rfcs/THV-XXXX-stateless-transport-proxies.md b/rfcs/THV-XXXX-stateless-transport-proxies.md index 8a81189..cdc3e4b 100644 --- a/rfcs/THV-XXXX-stateless-transport-proxies.md +++ b/rfcs/THV-XXXX-stateless-transport-proxies.md @@ -86,17 +86,19 @@ independent revisions, giving a client × backend matrix: |--------|---------|----------------| | Legacy | Legacy | Today's path, unchanged. | | Modern | Modern | Stateless pass-through. | -| Legacy | Modern | Gateway must terminate the handshake itself (deferred). | -| Modern | Legacy | Gateway must synthesize `initialize` toward the backend (deferred). | - -The two diagonal cells are the scope of this RFC. The off-diagonal cells require the -gateway to translate between eras; they are deferred (see Alternatives). The MCP -specification's probe-then-fallback mechanism is normative but only protects a client -talking *directly* to a server — its compatibility matrix marks the Legacy-client / -Modern-server case as "Fails" — so only a gateway could bridge it. The agentgateway -project reached the same conclusion: it ships the diagonals and the Modern-client / -Legacy-server synthetic-initialize path, and deliberately declined the Legacy-client / -Modern-server case as unsound. +| Legacy | Modern | Unbridged — spec marks this "Fails" (no client fall-forward). Deferred to vMCP. | +| Modern | Legacy | Client's probe-then-fallback downgrades to `initialize`; the pass-through proxy forwards and classifies it Legacy. No new proxy work. | + +The two diagonal cells are the scope of this RFC. The **Modern-client / Legacy-server** +cell needs no proxy work either: the MCP-normative probe-then-fallback runs client-side, +so the client downgrades to an `initialize` handshake that the transparent proxy forwards +to the Legacy backend and classifies as Legacy. That leaves only the **Legacy-client / +Modern-server** cell genuinely unbridged — the spec's compatibility matrix marks it +"Fails" ("Legacy clients have no fall-forward mechanism"), so only a gateway could bridge +it, and that work belongs to vMCP. The agentgateway project, as an aggregator that cannot +rely on transparent pass-through, instead actively synthesizes the handshake for the +Modern-client / Legacy-server cell (`stateless_send_and_initialize`); it likewise leaves +Legacy-client / Modern-server unbridged. ### Detailed Design @@ -120,13 +122,16 @@ not consulted. sticky-session machinery (session guard, pod-pinning, init-body storage, re-init replay, backend-SID rewrite) simply does not engage. Because there is no shared pipe, Go's `http.Transport` correlates responses to requests natively and the Modern path collapses -to a plain reverse proxy. The internal health-monitor gate must not "default ready" for -Modern (that would start the monitor before the backend is warm and risk self-stopping -the proxy); it should gate off the first successful backend contact. +to a plain reverse proxy. The internal health-monitor gate today flips ready on the `initialize` 200 or a +`Mcp-Session-Id` response header — neither of which occurs on the Modern path, so the +gate would never open. The Modern path must therefore acquire an alternate readiness +trigger: gate off the first successful backend contact (and never default the gate ready, +which would start the monitor before the backend is warm and risk self-stopping the proxy). **Parser vocabulary.** Register `server/discover` (currently unlabeled, yielding an -empty resource ID) and the mandatory Modern request headers `Mcp-Method` / `Mcp-Name` -so they are labeled for authz/audit; source `clientInfo`/protocolVersion from `_meta` +empty resource ID) and the Modern request headers `Mcp-Method` (required on all requests) +/ `Mcp-Name` (required for `tools/call`, `resources/read`, `prompts/get`) so they are +labeled for authz/audit; source `clientInfo`/protocolVersion from `_meta` for Modern requests. #### API Changes @@ -197,7 +202,8 @@ uniqueness is what prevents the cross-delivery leak described in the threat mode - `_meta` is decoded with guarded type assertions; malformed or absent metadata classifies as Legacy, preserving the existing session guard (fail-safe direction). -- `Mcp-Method` / `Mcp-Name` (and `MCP-Protocol-Version`) are validated against the body; +- `Mcp-Method` (all requests) / `Mcp-Name` (`tools/call`, `resources/read`, `prompts/get`) + and `MCP-Protocol-Version` are validated against the body; mismatch is rejected with `-32020`. As an intermediary that reads bodies for authz/audit, the proxy should verify that the protocol version indicates a header-validation-required revision before trusting any mirrored header value. @@ -258,9 +264,10 @@ field. Audit labeling must source it per request. It remains a structured log fi Legacy-client / Modern-server and Modern-client / Legacy-server. - Pros: any client reaches any backend regardless of era. - Cons: the Legacy-client / Modern-server cell requires the gateway to synthesize and - hold session state the spec has no client-side fallback for; agentgateway assessed the - equivalent as "theoretically unsound" and declined it. It is also more naturally a - gateway-aggregation concern than a transport-proxy one. + hold session state the spec has no client-side fallback for — the spec's own + compatibility matrix marks this cell "Fails" ("Legacy clients have no fall-forward + mechanism"). It is also more naturally a gateway-aggregation concern than a + transport-proxy one. - Why not chosen: deferred; the cross-generation bridge belongs to vMCP ([toolhive#5756]). This RFC scopes to the diagonal cells and relies on the spec-normative client fallback for Modern-client / Legacy-server. @@ -284,8 +291,9 @@ depend on it and this RFC proposes decoupling that edge. The binary Modern/Legacy classification suffices while there is a single Modern revision. When a second ships, `DiscoverResult.supportedVersions` and `UnsupportedProtocolVersionError.data.supported` are version lists, so the classifier -would need real version-string negotiation rather than a boolean; the `ClassifyRevision` -signature accommodates this by returning a richer `Revision` if needed. +would need real version-string negotiation rather than a boolean. `ClassifyRevision` is +the single extension point for that: `Revision` would grow from an enum into a type that +carries the negotiated version string — a signature-level change, not a drop-in. ## Implementation Plan From 4b02f574170ba37dce4982785c69adff01fc344a Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 15 Jul 2026 11:37:59 +0100 Subject: [PATCH 3/6] Renumber RFC to THV-0081 to match PR number Co-Authored-By: Claude Opus 4.8 --- ...58-rekey-upstream-token-storage-by-user.md | 442 +++++++ ...> THV-0081-stateless-transport-proxies.md} | 2 +- rfcs/THV-XXXX-upstream-token-refresh.md | 639 ++++++++++ rfcs/THV-XXXX-user-to-agent-delegation.md | 1090 +++++++++++++++++ 4 files changed, 2172 insertions(+), 1 deletion(-) create mode 100644 rfcs/THV-0058-rekey-upstream-token-storage-by-user.md rename rfcs/{THV-XXXX-stateless-transport-proxies.md => THV-0081-stateless-transport-proxies.md} (99%) create mode 100644 rfcs/THV-XXXX-upstream-token-refresh.md create mode 100644 rfcs/THV-XXXX-user-to-agent-delegation.md diff --git a/rfcs/THV-0058-rekey-upstream-token-storage-by-user.md b/rfcs/THV-0058-rekey-upstream-token-storage-by-user.md new file mode 100644 index 0000000..39c1f4e --- /dev/null +++ b/rfcs/THV-0058-rekey-upstream-token-storage-by-user.md @@ -0,0 +1,442 @@ +# RFC-0058: Re-key Upstream Token Storage by Stable User Identity + +- **Status**: Draft +- **Author(s)**: Jakub Hrozek (@jhrozek) +- **Created**: 2026-04-28 +- **Last Updated**: 2026-04-28 +- **Target Repository**: toolhive +- **Related Issues**: [toolhive#5046](https://github.com/stacklok/toolhive/issues/5046), [toolhive#5047](https://github.com/stacklok/toolhive/issues/5047) +- **Related RFCs**: THV-0019 (auth server design), THV-0031 (auth server integration), THV-0035 (auth server Redis storage), THV-0052 (multi-upstream IDP) + +## Summary + +Re-key `UpstreamTokenStorage` from `(SessionID, providerName)` to `(UserID, providerName)`. Switch the JWT lookup from the custom `tsid` claim to the standard `sub` claim, then drop `tsid` from newly-issued JWTs after a bounded transition window. Fix the callback's overwrite-with-empty-refresh-token bug atomically inside the storage layer. Address newly-discovered `User.ID` stability gaps amplified by the rekey. + +## Problem Statement + +Two bugs share one structural cause: + +- **toolhive#5046** — Tokens from providers that omit `expires_in` (GitHub PATs, Vault, service tokens) were given a fabricated 1h expiry, then evicted, triggering refresh attempts on tokens that have no refresh capability. The provider was omitted from downstream responses every hour. *Fixed in branch `upstream-expiring-issues`, ships separately as a focused PR.* +- **toolhive#5047** — A fresh `/authorize` mints a new SessionID via `rand.Text()` (`pkg/authserver/server/handlers/authorize.go:91`). The previous session's upstream tokens — including a working refresh token — become unreachable from the new JWT's `tsid` claim. Acutely visible with Google IdP, which only re-issues `refresh_token` on first consent without `prompt=consent`. + +The structural cause is that `UpstreamTokens` carry **user-lifetime semantics** (a Google refresh token is valid for ~6 months) but are stored under a **flow-scoped key** that rotates on every authorize call. The stable identifier (`User.ID`, resolved from `(providerID, providerSubject)` in `pkg/authserver/server/handlers/user.go`) exists on the row but is never used as a lookup key. + +Affected users: anyone using ToolHive with an IdP that omits `refresh_token` on re-auth (notably Google), and anyone whose access pattern triggers a fresh `/authorize` rather than a `grant_type=refresh_token` flow. + +## Goals + +- Eliminate orphaning of valid upstream refresh tokens on re-authentication. +- Make storage lookups deterministic for a returning user across `/authorize` calls. +- Retire the `tsid` claim cleanly (it is purely a lookup index, not load-bearing for security — see investigation in §References). +- Fix the callback's verbatim overwrite of an empty `refresh_token` (toolhive#5047 root cause B) atomically. +- Address `User.ID` stability gaps that the rekey amplifies (chain reordering, missing subsequent-leg identity links). +- Implement the dormant `ErrInvalidBinding` defence-in-depth check that has been designed but never wired up. + +## Non-Goals + +- **Account linking across providers.** Alice via Google ≠ Alice via GitHub remains true; auto-linking is deferred per existing `pkg/authserver/storage/types.go:284-288` TODO. +- **RFC 7009 upstream refresh-token revocation on AS logout.** Separate workstream. +- **JWE encryption of stored tokens at rest.** Orthogonal hardening. +- **MCP transport-layer session IDs.** ToolHive has at least four "session" concepts (MCP transport session via `Mcp-Session-Id`, Fosite OAuth session, AS upstream-storage session, transport proxy session). Only the third is in scope. +- **Renaming the upstream provider in config.** Reflects an orphaned-data trade-off documented as a known limitation. + +## Proposed Solution + +### High-Level Design + +```mermaid +flowchart TB + subgraph Before + JWT1[JWT: sub=userUUID, tsid=randomPerFlow] + JWT1 -- tsid lookup --> S1[(UpstreamTokens
keyed by sessionID,providerName)] + end + subgraph After + JWT2[JWT: sub=userUUID] + JWT2 -- sub lookup --> S2[(UpstreamTokens
keyed by userID,providerName)] + end +``` + +The JWT already carries `User.ID` in `sub`. The read path switch is a one-line change in `pkg/auth/token.go:1190`. The interface and backend changes are internal to `pkg/authserver`. Most external consumers only touch `Identity.UpstreamTokens` (a map) and are unaffected. + +### Detailed Design + +#### Storage Interface Change + +```go +// Before +StoreUpstreamTokens(ctx, sessionID, providerName, tokens) error +GetUpstreamTokens(ctx, sessionID, providerName) (*UpstreamTokens, error) +GetAllUpstreamTokens(ctx, sessionID) (map[string]*UpstreamTokens, error) +DeleteUpstreamTokens(ctx, sessionID) error + +// After +StoreUpstreamTokens(ctx, userID, providerName, tokens) error // merging semantics built in +GetUpstreamTokens(ctx, userID, providerName) (*UpstreamTokens, error) +GetAllUpstreamTokens(ctx, userID) (map[string]*UpstreamTokens, error) +DeleteUpstreamTokens(ctx, userID, providerName) error // gain providerName +DeleteAllForUser(ctx, userID) error // user deletion / "sign out everywhere" +``` + +`Store` gains atomic merge semantics: if the inbound `RefreshToken` is empty and an existing record has a non-empty one, the storage layer (Lua script in Redis, mutex-guarded read-then-write in memory) preserves the existing refresh token. This fixes toolhive#5047 root cause B without introducing application-layer races. + +#### JWT Changes + +- Read path (`pkg/auth/token.go:1190`) reads `claims["sub"]` instead of `claims["tsid"]`. +- Write path (`pkg/authserver/server/session/session.go:120-123`) stops emitting `tsid` after the transition window. +- **No fallback to `tsid`-keyed lookup.** A read fallback is a downgrade-attack vector; a JWT carrying only `tsid` (issued before cutover) must fail validation and trigger re-auth. + +To bound the in-flight pool: rotate the AS signing key on cutover, or reduce access-token lifespan to <1h ahead of the migration so the in-flight pool drains within hours. + +#### Storage Field Rename: `SessionExpiresAt` → `LastAccessedAt` + +Field rename with semantic change: sliding-window timestamp updated by both `Get` (read-bump) and `Store`. Backends derive eviction as `LastAccessedAt + idleTTL` (default 24h, configurable). + +Optimization: only update on `Get` if the remaining TTL is below a threshold (e.g., 50%) to avoid every read becoming a write. + +#### Multi-Upstream Chain Handling + +`SessionID` (from `authorize.go:91`) stays as the **chain correlation key** in `PendingAuthorization`. It no longer flows into upstream token storage. `UserID` (resolved on leg 1's callback) is threaded through subsequent legs via `pending.ResolvedUserID` (current behavior) and is now the storage key. + +#### User.ID Stability Fixes + +Investigation found `User.ID` is stable for the common case but unstable in three scenarios that the rekey newly amplifies: + +1. **Chain reordering**. The first leg is hardcoded as `h.upstreams[0]`. Reordering config produces a different `User.ID` for the same human. +2. **Subsequent legs don't auto-link**. `UpdateLastAuthenticated` returns `ErrNotFound` if the `(providerID, providerSubject)` row doesn't exist; the caller only logs a warning. The provider identity for legs 2+ is never persisted. +3. **Different providers, no linking**. Alice via Google → User.ID = X; Alice via GitHub → User.ID = Y. Auto-linking is explicitly out of scope per `types.go:284-288` TODO. + +The rekey adds: + +- **Chain-reorder warning**. On startup, persist the first-leg `providerName`. If it changes between runs, log a loud warning that returning users will resolve to fresh User.IDs. +- **Subsequent-leg auto-linking**. `UpdateLastAuthenticated` upserts: if `(providerID, providerSubject)` doesn't exist, create the `ProviderIdentity` row and link it to `pending.ResolvedUserID`. This is not "account linking" in the security-sensitive sense — it happens inside an authenticated chain where consent is implicit by chain completion. +- **Identity-mismatch check runs on every callback**, not only when `len(h.upstreams) > 1`. After rekey, single-leg flows write to a globally-readable slot; the cross-check is now load-bearing in single-leg too. + +Cross-provider auto-linking remains out of scope. + +#### Multi-IDP Authorization Scoping + +Under user-keying, an attacker who logs in via a "weak" linked provider could request `(UserID, "atlassian")` and pull tokens issued by a "strong" provider. The fix: the read path filters by the provider authorized in the current AS session. Add an `auth_provider` JWT claim or scope-based gating (e.g., `upstream:atlassian` granted at auth time). Refuse cross-provider reads. + +For ToolHive's current single-provider deployments this is dormant. For vMCP multi-upstream scenarios (THV-0052) it is load-bearing. + +#### `ErrInvalidBinding` Implementation + +The binding infrastructure (`UpstreamTokens.ClientID`, `UpstreamTokens.UpstreamSubject`, the `ErrInvalidBinding` sentinel) was designed in PR #3358 as defence-in-depth and never wired up. Investigation confirms `ErrInvalidBinding` is defined but never returned by any code path. + +On `Get`, the storage layer should compare: + +- `claims["client_id"]` vs `tokens.ClientID` — catches tokens being read from the wrong client's session. + +`tokens.UserID` vs `claims["sub"]` becomes tautological after the rekey (the lookup key is the same value), so that check disappears naturally. + +This is technically separate hardening, but the rekey is the moment we reread this code anyway. ~20 LOC across two backends. + +#### Logout Semantics + +Logout terminates the AS session only. Upstream tokens persist (other devices keep working). "Sign out everywhere" becomes an explicit user action that calls `DeleteAllForUser` and best-effort revokes upstream RTs via RFC 7009. **This is a behaviour change** — today's `DeleteUpstreamTokens(sessionID)` evicts the user's tokens on logout because per-session keying made one row equal one session. + +#### Redis Schema Changes + +Existing layout: + +``` +{prefix}upstream:{sessionID}:{providerName} # primary +{prefix}upstream:idx:{sessionID} # session index set +{prefix}user:upstream:{userID} # user reverse index +``` + +New layout: + +``` +{prefix}upstream:{userID}:{providerName} # primary (was idx member) +{prefix}user:upstream:{userID} # promoted to primary index +``` + +The `user:upstream:{userID}` index already exists and is currently used only for `DeleteUser` cascade — promoting it is a small change, not a new build. + +The hash tag `{:}` is unchanged → cluster slot routing unchanged. Do **not** introduce `{userID}` as a new hash tag (would scatter different users across slots and break per-tenant batch operations). + +The new Lua script is ~60 lines (smaller than current ~80) because the old-vs-new userID compare/repair logic disappears (userID is structural, not payload). + +```lua +-- Pseudocode for new storeUpstreamTokensScript (atomicity-critical) +KEYS[1] = "{prefix}upstream::" +KEYS[2] = "{prefix}user:upstream:" +ARGV[1] = newTokenJSON +ARGV[2] = ttlMs + +existing = GET KEYS[1] +new = cjson.decode(ARGV[1]) +if existing and existing != "null": + old = cjson.decode(existing) + if isEmpty(new.refresh_token) and notEmpty(old.refresh_token): + new.refresh_token = old.refresh_token -- toolhive#5047 fix B + +if ttlMs > 0: + SET KEYS[1] cjson.encode(new) PX ttlMs +else: + SET KEYS[1] cjson.encode(new) + +SADD KEYS[2] KEYS[1] +if ttlMs > 0: + if PTTL(KEYS[2]) < ttlMs: PEXPIRE KEYS[2] ttlMs +else: + PERSIST KEYS[2] +``` + +#### Migration Strategy: Big-Bang + +No dual-write, no online migration. Old keys self-evict via TTL within `max(refresh-token-TTL)` (typically minutes to hours). + +Justification: upstream tokens are bounded-lifetime caches, not durable user state. The worst-case user experience at cutover is one OAuth re-consent on the next downstream request — no worse than what users hit when an IdP revokes a refresh token (already a normal event). Dual-write trades complexity (extra Lua, dual indices, longer rollback rehearsal) for a UX outcome the system already handles gracefully. + +PR #4198 previously re-keyed Redis storage from `upstream:{sessionID}` to `upstream:{sessionID}:{providerName}` and shipped one-shot startup migration code. The precedent exists if a migration approach proves needed, but for this RFC the trade-off favours big-bang. + +## Security Considerations + +### Threat Model + +Attackers in scope: +- A user holding an old (pre-cutover) JWT. +- A user with multiple linked providers attempting to read across providers. +- A malicious or compromised IdP returning empty `refresh_token` on re-auth. +- Concurrent re-auths racing to overwrite the same user's tokens. +- An adversary attempting to enumerate or guess storage keys. + +### Authentication and Authorization + +- JWT signature validation remains the trust root. +- The lookup key changes from `tsid` (per-flow random, ~130 bits) to `User.ID` (UUIDv4, ~122 bits). Both far exceed brute-force reach. Forging requires breaking JWT signing, at which point all claims are forgeable regardless. +- The dormant `ErrInvalidBinding` check is wired up: `claims["client_id"]` vs `tokens.ClientID` becomes a hard read-side check. +- Multi-IDP scoping (see §Detailed Design): an `auth_provider` claim or scope gates which provider's tokens a session can read. + +### Data Security + +- Eliminates orphaned-token leakage class. Today, post-re-auth refresh tokens sit unreachable in storage with TTL until they age out. Post-rekey: single row per `(UserID, providerName)`, overwritten in place. +- GDPR right-to-erasure becomes one DELETE per user, deterministic. With SessionID keying erasure required scanning by `UserID` field — slower and error-prone. +- Bug B fix preserves an upstream `refresh_token` when the IdP omits it on re-auth. Failure mode: if the IdP just revoked the RT (e.g., user clicked "Revoke app" on Google), the AS keeps using a now-invalid RT until the next refresh attempt. Mitigation: on `invalid_grant` from upstream, the storage row MUST be deleted or dead-marked, forcing re-auth. + +### Input Validation + +- `providerID` (used as part of the storage key and identity lookup) is the **logical** ToolHive upstream name from config, not an upstream-asserted value. Renaming the upstream in config orphans data — documented as a known limitation. +- `providerSubject` is asserted by the upstream IdP. Trust in `(providerID, providerSubject)` uniqueness is the foundational assumption. +- `User.ID` is generated by `uuid.New()` (UUIDv4 from `crypto/rand`-backed `google/uuid`). Birthday-bound collision probability is negligible. + +### Secrets Management + +- Refresh tokens are stored in Redis (or memory). The rekey does not change the at-rest threat model — JWE encryption is out of scope. +- The merge-on-empty pattern for refresh tokens MUST happen inside the Lua script, not in Go application code, to be safe under concurrent `Store` for the same user. + +### Audit and Logging + +- `tsid` was useful as per-flow correlation in logs. Replace with a dedicated `jti` (JWT ID) claim or `flow_id` claim, logged but not used as a storage key. +- Audit log field rename: `session_id` → `user_id` in upstream-token-service log lines. Release-note item. + +### Mitigations + +| Threat | Mitigation | +|--------|------------| +| `tsid` fallback as downgrade vector | Hard switch: no fallback. JWT-version-claim-gated. Bounded JWT drainage. | +| Multi-IDP cross-provider read | `auth_provider` JWT claim or scope; storage read filtered by authorized provider. | +| Stale RT after revocation | `invalid_grant` from upstream MUST delete the row and force re-auth. | +| Concurrent `Store` race | Atomic Lua merge; mutex in memory backend. | +| Concurrent `ResolveUser` race | Atomic upsert (`INSERT ... ON CONFLICT DO NOTHING + re-read`). | + +## Alternatives Considered + +### Alternative 1: Reuse SessionID across authorize calls for the same user + +- Description: Don't mint fresh SessionID on every `/authorize`. Look up the user's existing SessionID from a side index and reuse it. +- Pros: Minimal interface change. +- Cons: User.ID isn't known at `/authorize` time. Breaks chain-leg routing (legs lookup by SessionID). Regresses multi-device (devices compete for same row). +- Why not chosen: Not viable. Contorts the auth flow to dodge a storage problem. + +### Alternative 2: Side index `(userID, providerID) → currentSessionID` + +- Description: Keep storage keyed by SessionID. Add a side index that maps stable `(userID, providerID)` to the current SessionID. Callback merges old→new at the index pointer. +- Pros: No primary-key change. JWT `tsid` continues to work. +- Cons: Lua CAS complexity for an index that should just be the primary key. Eviction-window hole (if old SessionID's row was already TTL'd, merge is a no-op and refresh token is lost). Stale rows linger until TTL. +- Why not chosen: Adds Lua atomicity surface area for the index-that-should-be-primary. Worst of both worlds. + +### Alternative 3: Half-rekey: `GetByUser` on top of session-keyed storage + +- Description: Add `GetByUser(userID, providerID)` method that reads the side index, then fetches by SessionID. Read path switches to `sub`, write path unchanged. +- Pros: Backwards compat for in-flight JWTs (still readable via `tsid` if needed). +- Cons: Once `sub` is the read key, the primary `SessionID` key is dead weight. Pays most of the rekey cost without the structural cleanup. +- Why not chosen: A half-step that would need a follow-up cleanup that "won't happen." + +### Alternative 4: Callback-only merge (Bug B fix without rekey) + +- Description: Apply the refresher's preserve-RT-on-empty pattern to the callback. Don't rekey. +- Pros: Tiny change. +- Cons: Doesn't fix root cause A. Old tokens still orphan on every re-auth. +- Why not chosen: Solves the visible Google symptom without solving the structural problem. + +### Alternative 5: OIDC `id_token_hint` carrying old tsid + +- Description: Client includes `id_token_hint` on `/authorize`. Server validates and reuses old SessionID. +- Pros: Standards-aligned mechanism. +- Cons: Most callers won't send `id_token_hint`. Hostile clients could collide sessions. Only optimisation on top of a real solution. +- Why not chosen: Not standalone. + +### Alternative 6: Explicit session merge + +- Description: Like Alternative 4 but with audit logging and explicit merge semantics. +- Pros: Cleaner code review story. +- Cons: Same shortcomings as Alt 4 — write-side only, doesn't fix Bug A. +- Why not chosen: Same. + +## Compatibility + +### Backward Compatibility + +- **JWT format**: `sub` claim was always populated. Old JWTs containing only `tsid` (issued pre-cutover) will not resolve under new code → re-auth required. Bound the in-flight window via short access-token lifespan or signing-key rotation. +- **Storage data**: old `(SessionID, providerName)` keys are not migrated. They self-evict via TTL within `max(refresh-token-TTL)`. Worst case at cutover: every active user re-auths once on their next downstream request. +- **External consumers of `tsid`**: Investigation found only three production usages — `pkg/auth/token.go:1190` (read path, switching), `pkg/authserver/server/session/session.go:120-123` (write site, stopping), `pkg/auth/context.go:99` (filter, harmless to leave). No external operators or sidecars should depend on the claim. Verify before PR 7. +- **Audit log field name** changes from `session_id` to `user_id` in upstream-token-service log lines. Release-note item; dashboards greppping for `session_id=` need updating. +- **Logout semantics change** (AS session only, upstream tokens persist) — release-note item; behaviour change. + +### Forward Compatibility + +- Account linking across providers is enabled by the data model but not the workflow. A future RFC can build on `User.ID` as the unified identity without further storage changes. +- Multi-client per user (`(userID, clientID, providerName)` keying) is a clean follow-up if scenarios warrant — additive. +- ID-JAG / token exchange flows (memory: XAA PoC) get `User.ID` as the obvious identity anchor instead of session-scoped state. + +## Implementation Plan + +### Phase 0: Ship `upstream-expiring-issues` standalone (current branch) + +Per PR-creation rules, the existing branch ships as its own focused PR for toolhive#5046. Independent of this RFC. + +### Phase 1: Storage interface refactor + +- Rename `sessionID` → `userID` parameter throughout `UpstreamTokenStorage`. +- Rename `SessionExpiresAt` → `LastAccessedAt`. +- Add `DeleteAllForUser`. +- Add atomic merge semantics to `Store` (Lua script for Redis, mutex-guarded RMW for memory). +- Backends keep current Redis keys internally — no behaviour change yet. +- ~250 LOC. Reversible. + +### Phase 2: Big-bang cutover + +- Switch read path to `claims["sub"]`. +- Switch write path to `userID`. +- New Lua script. +- Drop `upstream:idx:{sessionID}` set; promote `user:upstream:{userID}` to primary index. +- Multi-IDP scoping enforced. +- Identity-mismatch check runs single-leg too. +- ~200 LOC. Medium risk. + +### Phase 3: User.ID stability fixes + +- Subsequent-leg auto-linking (`UpdateLastAuthenticated` upserts). +- Chain-reorder startup warning. +- Document config rename / `sub` rotation as known limitations. +- ~80 LOC. + +### Phase 4: Logout semantics change + +- Stop deleting upstream tokens on AS session logout. +- Add admin "Delete all tokens for user" surface. +- Release note. +- ~50 LOC. + +### Phase 5: Concurrency hardening (independent) + +- Atomic `ResolveUser`: `INSERT ... ON CONFLICT DO NOTHING + re-read`. +- Regression test for concurrent identical callbacks. +- Can land in parallel with Phase 1. + +### Phase 6: `ErrInvalidBinding` implementation + +- `claims["client_id"]` vs `tokens.ClientID` check on `Get`. +- ~20 LOC across two backends. + +### Phase 7: Drop `tsid` (one release after Phase 2) + +- Stop populating in JWT. +- Remove constant and `internalClaims` filter entry. +- ~30 LOC. + +### Dependencies + +- Phase 1 must land first. Phases 2–4 serialize. Phase 5 is independent. Phase 6 is independent of the rekey but logically grouped. Phase 7 lands one release after Phase 2. + +## Testing Strategy + +### Unit Tests + +- `Store` merge matrix: fresh has RT × existing has RT, fresh has RT × existing empty, fresh empty × existing RT, fresh empty × existing empty, no existing record. +- Idle-TTL eviction: store, advance clock, verify eviction; sliding-window keep-alive on `Get`. +- `DeleteAllForUser` removes all of a user's records, none of another user's. +- Concurrent `Store` for same `(UserID, providerName)`: refresh-token preservation invariant under contention. +- Concurrent `ResolveUser` for same `(providerID, providerSubject)`: exactly one User row created. +- Non-expiring token (`ExpiresAt.IsZero()`) retained until idle timeout (regression for existing #5046 fix). + +### Integration Tests + +- The toolhive#5047 regression: same client re-runs full authorize flow; tokens remain retrievable. +- Two distinct users, same provider — no cross-contamination. +- Same user, parallel chains — concurrent Store + per-chain Delete. +- Refresh-token-rotation cross-flow. +- Logout on device A leaves device B's access intact. +- Multi-IDP scoping: JWT auth'd via provider X cannot read `(UserID, providerY)`. +- Chain reorder produces different User.IDs (regression for known limitation). +- JWT with only `tsid` (issued pre-cutover, post-cutover validation) fails closed. + +### Real-Redis Integration + +- Sentinel failover during write: no torn writes, no lost user-index entries. +- Cluster slot routing: hash tag `{:}` keeps everything in one slot; Lua script does not raise `CROSSSLOT`. +- Post-migration coexistence: pre-seed old-layout keys, verify reads return `ErrNotFound` and writes go to new layout. + +## Documentation + +- `docs/middleware.md` lines 169-209: replace "extracts `tsid`" with "extracts `sub`". +- `docs/arch/11-auth-server-storage.md`: describe the rekey, the chain-correlation vs storage-key separation, idle-TTL eviction. +- `pkg/authserver/server/doc.go:76` and `pkg/authserver/storage/doc.go:113`: update claim references. +- Operator advisory comments at `cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go:358` and `cmd/thv-operator/controllers/virtualmcpserver_controller.go:507`: drop `tsid` from claim-namespace warning. +- Release notes: logout semantics change, `tsid` removal, audit log field rename. + +## Open Questions + +1. **Chain-reorder handling**: warning-only (cheap) or explicit `primaryIdentityProvider` config field (cleaner)? Current proposal: warning-only; promote to explicit field if reorder scenarios prove common. +2. **Composite key dimensions**: `(userID, providerName)` is the simple proposal. Other dimensions that could legitimately differentiate stored tokens for the same user+provider: + - **`clientID`**: same user logging in from multiple OAuth clients. Each gets its own session today; under the rekey they share a row. + - **Scope set**: re-auth with broader scopes typically issues a token covering the union, so overwriting is usually correct — but providers that don't combine scopes would silently lose access. + - **Audience / RFC 8707 `resource`**: if ToolHive ever passes `resource` upstream, tokens with different audiences should not share a row. + + Oathkeeper's `MutatorIDToken` cache key (`pipeline/mutate/mutator_id_token.go:82`) hashes five dimensions (`IssuerURL | TTL | JWKSURL | claims | subject`) precisely to avoid this class of leak. Current proposal: `(userID, providerName)` only, with all three additional dimensions deferred until concrete evidence requires them. +3. **Idle TTL default value and config surface**: 24h matches the current `SessionExpiresAt` cap, but it's now a separate knob. Default and config surface (env var? CRD field?) need product input. +4. **Does anything outside this repo read `tsid`?** External operators, sidecars, observability — verify before Phase 7. +5. **`UpstreamTokenRefresher.RefreshAndStore(sessionID, ...)` interface**: signature must change to `userID`. Confirm no external implementations exist outside `pkg/authserver`. +6. **Cluster mode usage**: codebase uses `redis.UniversalClient` but configured `redis.NewFailoverClient` (Sentinel). Verify whether any tenant runs against Cluster mode; if not, hash-tag analysis is precautionary. + +## References + +- toolhive#5046 — fabricated 1h expiry on non-expiring upstream tokens +- toolhive#5047 — orphaned upstream refresh tokens on re-authorization +- THV-0019 — original auth server design (defines session-scoped storage model) +- THV-0031 — auth server integration +- THV-0035 — auth server Redis storage +- THV-0052 — multi-upstream IDP authserver +- Hydra reference: `flow/consent_types.go`, `persistence/sql/persister_consent.go` — subject-keyed indexes pattern; `LoginSession` UPSERT-by-cookie-id at `persister_consent.go:217-228`; `FindGrantedAndRememberedConsentRequest` keyed by `(subject, client_id, nid)` at `persister_consent.go:286-331`. +- Oathkeeper reference: `pipeline/mutate/mutator_id_token.go:82` — cache key hashes `IssuerURL | TTL | JWKSURL | claims | session.Subject`, the closest structural analog to our problem; `pipeline/authn/authenticator_oauth2_client_credentials.go:180-194` — `TTL = min(configuredTTL, time.Until(token.Expiry))` pattern. Both Ory products independently picked subject-stable keying for caches that must survive across requests for the same user. +- Prior commits: `86ebd0f49` (`tsid` introduction), `2613d6e7f` (JWT binding security rationale), `bbaee66bf`/PR #3358 (`UserID` field added as metadata), `f52b6d075`/PR #4198 (Redis migration precedent), `e73512271`/PR #4283 (multi-leg chain SessionID threading) + +--- + +## RFC Lifecycle + +### Review History + +| Date | Reviewer | Decision | Notes | +|------|----------|----------|-------| +| 2026-04-28 | @jhrozek | Draft | Initial submission, not yet pushed | + +### Implementation Tracking + +| Repository | PR | Status | +|------------|-----|--------| +| toolhive | (TBD Phase 0 — `upstream-expiring-issues`) | In review | +| toolhive | (TBD Phase 1) | Not started | diff --git a/rfcs/THV-XXXX-stateless-transport-proxies.md b/rfcs/THV-0081-stateless-transport-proxies.md similarity index 99% rename from rfcs/THV-XXXX-stateless-transport-proxies.md rename to rfcs/THV-0081-stateless-transport-proxies.md index cdc3e4b..5fb0688 100644 --- a/rfcs/THV-XXXX-stateless-transport-proxies.md +++ b/rfcs/THV-0081-stateless-transport-proxies.md @@ -1,4 +1,4 @@ -# RFC-XXXX: Stateless Transport Proxy Support (MCP 2026-07-28) +# RFC-0081: Stateless Transport Proxy Support (MCP 2026-07-28) - **Status**: Draft - **Author(s)**: Jakub Hrozek (@jhrozek) diff --git a/rfcs/THV-XXXX-upstream-token-refresh.md b/rfcs/THV-XXXX-upstream-token-refresh.md new file mode 100644 index 0000000..68570bd --- /dev/null +++ b/rfcs/THV-XXXX-upstream-token-refresh.md @@ -0,0 +1,639 @@ +# RFC-XXXX: Upstream Token Refresh for Auth Server + +- **Status**: Draft +- **Author(s)**: @tgrunnagle +- **Created**: 2026-02-20 +- **Last Updated**: 2026-02-20 +- **Target Repository**: toolhive +- **Related Issues**: TBD + +## Summary + +Add transparent upstream token refresh to the embedded authorization server. When a stored upstream access token expires, the auth server refreshes it via the upstream IdP before returning it to the middleware, extending session lifetime beyond the upstream AT's ~1-hour window. When upstream refresh permanently fails, the service revokes the internal session to force a clean re-authentication. + +## Problem Statement + +Upstream refresh tokens are stored and `RefreshTokens()` is fully implemented on both OAuth2 and OIDC providers, but no code calls it yet. The upstream swap middleware (`pkg/auth/upstreamswap/middleware.go`) detects expiry but logs a warning and passes the expired token through. This means sessions are effectively limited to the upstream AT lifetime (~1 hour for most IdPs). + +To support long-lived sessions, the auth server needs to use stored upstream refresh tokens to obtain fresh access tokens transparently. Additionally, when an upstream refresh token is permanently invalidated by the IdP, the auth server should revoke the internal session so the client is directed into a clean re-authentication rather than retrying with stale credentials. + +### Current Token Flow + +``` +Client -> GET /oauth/authorize -> Internal AS -> 302 to upstream IdP + | +Client <- redirect with auth code <- Internal AS <- GET /oauth/callback (code) + | + ExchangeCode -> upstream AT, RT, IDT stored (keyed by sessionID) + Issue internal JWT (with tsid=sessionID claim) + Issue internal opaque refresh token + | +Client -> proxied request -> upstream swap middleware + | + Read upstream tokens by tsid from storage + Replace Authorization header with upstream AT + | + Forward to backend +``` + +### Current Token Lifetimes + +| Token | Default | Configurable Range | +|-------|---------|-------------------| +| Internal access token (JWT) | 1h | 1min-24h | +| Internal refresh token (opaque) | 7d | 1h-30d | +| Auth code | 10min | 30s-10min | +| Upstream AT storage TTL | upstream `ExpiresAt` (or 1h fallback) | - | +| Storage cleanup interval | 5min | - | +| Expiry detection buffer | 30s | - | + +### Opportunities + +1. **Upstream refresh tokens stored but never used** - `RefreshTokens()` is fully implemented (`pkg/authserver/upstream/oauth2.go`, `oidc.go`) but no code calls it yet +2. **Storage TTL tied to AT expiry** - the upstream token entry expires when the AT expires, which garbage-collects the refresh token even though it may be valid for 30+ days +3. **No session revocation on upstream failure** - when upstream tokens expire, the internal session (7-day RT) continues without a mechanism to direct the client into re-authentication +4. **Direct storage access from middleware** - the middleware calls raw `storage.UpstreamTokenStorage` directly, which is an implementation detail of the AS, not a proper service boundary + +## Goals + +- Transparently refresh expired upstream access tokens on read, so users never see a 401 due to upstream AT expiry +- Revoke the internal session when upstream refresh permanently fails, forcing the client into a clean re-authentication +- Introduce a service interface (`upstreamtoken.Service`) that encapsulates all upstream token logic behind a clean API boundary +- Decouple storage TTL from upstream AT expiry so refresh tokens are preserved across AT renewal cycles +- Deduplicate concurrent refresh attempts for the same session using singleflight +- Prepare the architecture for out-of-process AS extraction (the service interface becomes a single RPC) + +## Non-Goals + +- **Background refresh loop** - deferred to a future phase; can be added as an internal optimization without changing the `Service` interface +- **Persisting refresh failure state** - the service handles errors transparently and revokes the session on permanent failure; no persistent failure flag is needed +- **Multi-provider refresh** - the current system supports a single upstream provider per auth server instance +- **Redis storage implementation** - this RFC only modifies the in-memory storage; Redis implementation (THV-0035) will implement the same interface methods +- **Upstream token encryption at rest** - tokens remain in-memory with short TTLs + +## Proposed Solution + +### High-Level Design + +Introduce an `upstreamtoken.Service` interface that the middleware calls instead of raw storage. The service transparently refreshes expired tokens on read, deduplicates concurrent refreshes via singleflight, and revokes the internal session when upstream refresh permanently fails. + +```mermaid +flowchart TB + subgraph "Proxy Runner" + MW["Upstream Swap
Middleware"] + SVC["upstreamtoken.Service
(GetValidTokens)"] + SF["singleflight.Group"] + STOR["UpstreamTokenStorage"] + PROV["OAuth2Provider
(RefreshTokens)"] + REV["SessionRevoker
(RevokeSessionByTSID)"] + + MW -->|"GetValidTokens(ctx, tsid)"| SVC + SVC -->|"1. Read tokens"| STOR + SVC -->|"2. Refresh if expired"| SF + SF -->|"One IdP call"| PROV + SVC -->|"3. Revoke on permanent failure"| REV + SVC -->|"4. Update tokens"| STOR + end + + IDP["Upstream IdP"] + PROV -->|"RefreshTokens()"| IDP +``` + +### Detailed Design + +#### Design Decision: Service Interface with On-Read Refresh + +The middleware currently calls `storage.UpstreamTokenStorage.GetUpstreamTokens()` directly. This is the wrong abstraction: storage is an implementation detail of the AS. The middleware should call a service-level interface that represents the business operation: "give me valid upstream tokens for this session." + +Three approaches were evaluated: proactive background refresh, lazy on-demand refresh in the middleware, and on-read refresh behind a service interface. + +**On-read refresh behind the service interface wins** because: + +- **Architecture boundary**: The middleware calls a service, not raw storage. The service owns all IdP interaction. When the AS goes out-of-process, `GetValidTokens()` becomes a single RPC. The middleware code does not change. +- **Simplicity**: No background goroutines, no `Start`/`Stop` lifecycle, no `ListUpstreamTokenSessions()` method. The service is just storage + provider + singleflight. +- **Correctness without optimization**: Every request gets valid tokens or a clear error. No edge cases around idle sessions that the background loop missed. +- **Minimal interface**: One method (`GetValidTokens`) returning an opaque `UpstreamCredential`. Clean for in-process and network use. + +**The latency cost is acceptable**: On-read refresh adds ~200ms (one IdP round-trip) to the first request after AT expiry, once per hour per session. MCP tool calls typically take 500ms-2s. The 200ms is imperceptible. + +#### Service Interface + +**Package**: `pkg/authserver/upstreamtoken/` + +```go +package upstreamtoken + +type Service interface { + // GetValidTokens returns a valid upstream credential for the given session. + // If stored tokens are expired and a refresh token is available, the + // implementation transparently refreshes them before returning. + // + // If refresh permanently fails (e.g. refresh token revoked), the + // implementation revokes the internal session to force re-authentication. + GetValidTokens(ctx context.Context, sessionID string) (UpstreamCredential, error) +} + +// UpstreamCredential is an opaque type representing a valid upstream token. +// The middleware does not need to know internal structure - it only needs +// a bearer token string to inject into upstream requests. +type UpstreamCredential struct { + accessToken string +} + +// NewUpstreamCredential creates an UpstreamCredential. Only the service +// package should call this. +func NewUpstreamCredential(accessToken string) UpstreamCredential { + return UpstreamCredential{accessToken: accessToken} +} + +// BearerToken returns the access token string for injection into HTTP headers. +func (c UpstreamCredential) BearerToken() string { + return c.accessToken +} + +var ( + ErrSessionNotFound = errors.New("upstreamtoken: session not found") + ErrRefreshFailed = errors.New("upstreamtoken: refresh failed") + ErrNoRefreshToken = errors.New("upstreamtoken: no refresh token") +) +``` + +#### In-Process Implementation + +```go +type inProcessService struct { + storage storage.UpstreamTokenStorage + provider upstream.OAuth2Provider + revoker SessionRevoker + group singleflight.Group +} +``` + +Three dependencies: +- `storage.UpstreamTokenStorage` - read/write upstream token entries +- `upstream.OAuth2Provider` - call IdP to refresh tokens +- `SessionRevoker` - revoke internal session when upstream refresh permanently fails + +#### GetValidTokens Flow + +```mermaid +flowchart TD + A["GetUpstreamTokens(sessionID)"] -->|"not found"| B["Return ErrSessionNotFound"] + A -->|"found"| C{"AT expired?"} + C -->|"no"| D["Return UpstreamCredential(AT)"] + C -->|"yes"| E{"Has RT?"} + E -->|"no"| F["Return ErrNoRefreshToken"] + E -->|"yes"| G["singleflight.Do"] + G --> H["Double-check: re-read from storage"] + H -->|"already refreshed"| D + H -->|"still expired"| I["provider.RefreshTokens()"] + I -->|"success"| J["UpdateUpstreamTokens
(reset TTL)"] + J --> D + I -->|"invalid_grant"| K["RevokeSessionByTSID
DeleteUpstreamTokens"] + K --> L["Return ErrRefreshFailed (401)"] + I -->|"transient error"| M["Return ErrRefreshFailed (502)
Session NOT revoked"] +``` + +The singleflight group ensures that concurrent requests for the same expired session result in a single IdP refresh call. The double-check after acquiring the singleflight lock avoids redundant refreshes when another goroutine has already refreshed the tokens. + +When storing refreshed tokens, refresh token rotation is handled via coalesce: if the IdP returns a new refresh token, it is stored; otherwise the existing one is preserved. + +#### Session Revocation on Permanent Failure + +When upstream refresh fails with `invalid_grant` (RT revoked, expired, or invalidated by the IdP), the service must break the internal session to prevent the user from being stuck in a loop. Without this, the client would: + +1. Get 401 from the middleware +2. Use its internal refresh token to get a new internal JWT +3. New JWT has the same `tsid` (fosite preserves session claims on refresh) +4. Next request hits the same dead upstream tokens +5. Infinite loop - user is stuck + +The fix: `RevokeSessionByTSID(tsid)` deletes all internal access tokens and refresh tokens where `session.UpstreamSessionID == tsid`. The next time the client tries to use its internal refresh token, fosite returns `invalid_grant`, which is the standard signal that forces the client into a full re-authentication (new OAuth flow, new upstream tokens, new `tsid`). + +#### Session Revocation Interface + +```go +// SessionRevoker revokes all internal tokens associated with a session. +// The UpstreamTokenService calls this when upstream refresh permanently fails, +// to force the client into re-authentication. +type SessionRevoker interface { + RevokeSessionByTSID(ctx context.Context, tsid string) error +} +``` + +Implementation on `MemoryStorage`: + +```go +func (s *MemoryStorage) RevokeSessionByTSID(ctx context.Context, tsid string) error { + s.mu.Lock() + defer s.mu.Unlock() + + // Delete all refresh tokens for this session + for sig, entry := range s.refreshTokens { + session, ok := entry.value.GetSession().(*session.Session) + if ok && session.UpstreamSessionID == tsid { + delete(s.refreshTokens, sig) + } + } + + // Delete all access tokens for this session + for sig, entry := range s.accessTokens { + session, ok := entry.value.GetSession().(*session.Session) + if ok && session.UpstreamSessionID == tsid { + delete(s.accessTokens, sig) + } + } + + return nil +} +``` + +This follows the existing pattern. `RevokeRefreshToken(requestID)` and `RevokeAccessToken(requestID)` already do O(n) scans over the same maps. The service never touches fosite internals (signatures, request IDs). It only knows about `tsid`. + +#### Storage Layer Changes + +**New method on `UpstreamTokenStorage`**: + +```go +// UpdateUpstreamTokens atomically updates token values for an existing session +// and resets the sliding-window TTL (2h inactivity timeout). Used by refresh logic. +// Since there is no background refresh loop, every call to UpdateUpstreamTokens +// is triggered by a user request, so resetting the TTL here is correct. +UpdateUpstreamTokens(ctx context.Context, sessionID string, tokens *UpstreamTokens) error +``` + +**Behavioral change to `StoreUpstreamTokens`**: + +This is a change to an existing method's behavior, not just a new method: + +``` +BEFORE: expiresAt = tokens.ExpiresAt (AT expiry, ~1h) + +AFTER: if tokens.RefreshToken != "" -> expiresAt = now + DefaultUpstreamInactivityTimeout (2h) + if tokens.RefreshToken == "" -> expiresAt = tokens.ExpiresAt (unchanged) +``` + +New constant: `DefaultUpstreamInactivityTimeout = 2 * time.Hour` + +**Why 2 hours?** +- Long enough that a briefly idle user (lunch, meeting) does not lose their session +- Short enough that abandoned sessions are reaped quickly (not 7 days) +- TTL is reset by `StoreUpstreamTokens` (initial auth) and `UpdateUpstreamTokens` (refresh). Since there is no background loop, every write is user-triggered, so resetting TTL on write correctly tracks user activity +- Without a background refresh loop, there are zero unnecessary IdP refreshes for abandoned sessions: the entry simply expires and gets reaped + +No `LastAccessedAt` field is needed. The `timedEntry.expiresAt` sliding window (reset by `UpdateUpstreamTokens`) already tracks user activity. No `RefreshFailed` flag is needed. The service handles errors transparently and returns them to the caller. + +#### Error Mapping in Middleware + +| Service Error | HTTP | When | +|---|---|---| +| `ErrSessionNotFound` | 401 | No upstream tokens for session | +| `ErrNoRefreshToken` | 401 | AT expired, no RT to refresh with | +| `ErrRefreshFailed` + `invalid_grant` | 401 | RT permanently dead (internal session also revoked) | +| `ErrRefreshFailed` + transient | 502 | IdP temporarily down | + +#### Wiring Chain + +The `EmbeddedAuthServer` constructs the `upstream.OAuth2Provider` directly (it already builds the upstream config), keeps a reference, and passes it to both the Server and the Service. The `authserver.Server` interface does not change. + +```go +// In NewEmbeddedAuthServer: +provider := upstream.NewOAuth2Provider(upstreamConfig) +server := authserver.New(provider, ...) + +tokenService := upstreamtoken.NewInProcessService( + server.IDPTokenStorage(), // UpstreamTokenStorage + provider, // OAuth2Provider for refresh calls + server.IDPTokenStorage(), // SessionRevoker (MemoryStorage implements both) +) +``` + +**`MiddlewareRunner` interface change**: + +```go +// BEFORE: +GetUpstreamTokenStorage() func() storage.UpstreamTokenStorage + +// AFTER: +GetUpstreamTokenService() func() upstreamtoken.Service +``` + +**Middleware change**: + +```go +// BEFORE: +stor := storageGetter() +tokens, err := stor.GetUpstreamTokens(ctx, tsid) +if tokens.IsExpired(time.Now()) { + logger.Warn("upstreamswap: upstream tokens expired") + // Continue with expired token +} +injectToken(r, tokens.AccessToken) + +// AFTER: +svc := serviceGetter() +cred, err := svc.GetValidTokens(r.Context(), tsid) +if err != nil { + // map error to 401 or 502 + return +} +injectToken(r, cred.BearerToken()) +``` + +### Token Lifecycle Scenarios + +#### A. Active user (continuous requests) + +``` +T+0:00 Auth. AT stored (exp T+1:00). Entry TTL = T+2:00. +T+0:30 Request. GetValidTokens -> AT valid. TTL unchanged (T+2:00). +T+1:00 Request. AT expired. GetValidTokens refreshes via IdP (~200ms). + UpdateUpstreamTokens stores new AT (exp T+2:00), resets TTL -> T+3:00. +T+1:30 Request. AT valid. TTL unchanged (T+3:00). +T+2:00 Request. AT expired. Refresh again. TTL resets -> T+4:00. +... continues indefinitely. +``` + +#### B. User idle 30 min, returns + +``` +T+0:00 Last request. TTL = T+2:00. +T+0:30 User returns. AT still valid (expires T+1:00). TTL unchanged (T+2:00). +``` +Result: Seamless. No refresh needed. + +#### C. User idle 90 min, returns + +``` +T+0:00 Last request. TTL = T+2:00. +T+1:30 User returns. AT expired (expired at T+1:00). TTL still alive (T+2:00). + GetValidTokens refreshes via IdP (~200ms). UpdateUpstreamTokens resets TTL -> T+3:30. +``` +Result: 200ms latency on first request. Transparent to user. + +#### D. User idle 2+ hours, returns + +``` +T+0:00 Last request. TTL = T+2:00. +T+2:00 Cleanup loop reaps entry. +T+2:15 User returns. GetValidTokens -> ErrSessionNotFound -> 401. Re-authenticate. +``` +Result: Must re-authenticate. Expected for 2h+ idle. + +#### E. Abandoned session + +``` +T+0:00 Last request. TTL = T+2:00. +T+2:00 Entry reaped. Zero unnecessary IdP calls. +``` + +#### F. Refresh token revoked by IdP + +``` +T+1:00 User request. AT expired. GetValidTokens attempts refresh. + IdP returns invalid_grant. + Service calls revoker.RevokeSessionByTSID(tsid) -> deletes internal AT + RT. + Service calls storage.DeleteUpstreamTokens(tsid) -> deletes upstream entry. + Returns ErrRefreshFailed -> middleware returns 401. +T+1:01 Client tries internal refresh token -> fosite returns invalid_grant. + Client forced into full re-authentication. + New OAuth flow -> new upstream tokens -> new tsid -> user is recovered. +``` +Result: Clean recovery. User re-authenticates and gets a fresh session. + +#### G. IdP temporarily down + +``` +T+1:00 User request. AT expired. GetValidTokens attempts refresh. + IdP returns connection error. + Return ErrRefreshFailed wrapping transient error -> middleware returns 502. + Internal session NOT revoked (transient, not permanent). + Client retries. +T+1:01 Client retries. GetValidTokens attempts refresh again. + IdP is back -> refresh succeeds. New AT stored. Request proceeds. +``` +Result: Brief 502, then recovery on retry. + +#### H. Concurrent requests for same expired session + +``` +T+1:00 5 requests arrive. All call GetValidTokens(sessionID). + singleflight deduplicates: 1 actual refresh call to IdP. + All 5 callers block, all get same result. +``` + +#### I. IdP rotates refresh token + +``` +T+1:00 Refresh returns new AT + new RT. UpdateUpstreamTokens stores both. + Old RT replaced atomically. coalesce() keeps old RT if IdP doesn't rotate. +``` + +### Future: Out-of-Process AS + +When the AS moves to a separate service: + +**HTTP endpoint** (on the AS): +``` +POST /internal/upstream-tokens/{sessionID} + +200: { "access_token": "..." } +404: { "error": "session_not_found" } +401: { "error": "refresh_failed" } +422: { "error": "no_refresh_token" } +``` + +**Network client**: Implements `upstreamtoken.Service` by making HTTP calls to the AS. `GetValidTokens()` becomes a single HTTP request. Error types are serializable. + +**Middleware code does not change** - same interface, different implementation behind it. + +**Background refresh loop** (Phase 2 optimization): When added, it lives inside the AS process as an internal optimization of the concrete `Service` implementation. It can be added without changing the `Service` interface. + +## Security Considerations + +### Threat Model + +| Threat | Description | Likelihood | Impact | +|--------|-------------|------------|--------| +| Stolen upstream refresh token | Attacker obtains RT from memory and uses it to get new ATs | Low (requires process memory access) | Access to upstream resources for one user | +| IdP refresh token replay | Attacker replays an old RT after rotation | Low | Blocked by IdP RT rotation policies | +| Stale session exploitation | Attacker uses internal session after upstream RT revocation | Medium (before this fix) | Currently possible; this RFC eliminates it | + +### Authentication and Authorization + +- No changes to authentication flows. The service operates within an already-authenticated session. +- The service validates that a session exists before attempting refresh. It does not create new sessions. +- Session revocation on permanent failure is an authorization enforcement: if the upstream IdP revokes access, the internal session is terminated. + +### Data Security + +- Upstream access tokens and refresh tokens remain in-memory only, matching the current storage model +- Refreshed tokens overwrite the previous values atomically; no history is retained +- The `UpstreamCredential` type uses an unexported field (`accessToken`) to prevent accidental logging or serialization + +### Input Validation + +- `sessionID` is an internal identifier (UUID) generated during the OAuth flow; it is not user-supplied +- The service does not accept external input beyond the session ID +- Upstream IdP responses are validated by the existing `upstream.OAuth2Provider` implementation + +### Secrets Management + +- No new secrets are introduced +- Upstream refresh tokens are already stored in the existing `UpstreamTokenStorage` +- The 2-hour inactivity TTL ensures tokens are reaped more aggressively than before (previously tied to AT expiry only) + +### Audit and Logging + +- Token refresh attempts logged at INFO level (session ID, provider, success/failure) +- Permanent refresh failures (invalid_grant) logged at WARN level with session revocation noted +- Transient refresh failures logged at WARN level +- Token values are never logged + +### Mitigations + +| Threat | Mitigation | +|--------|------------| +| Stolen upstream RT | In-memory storage with 2h inactivity TTL; tokens reaped when session idle | +| Stale session loop | Session revocation on `invalid_grant`; internal RT deleted, forcing re-auth | +| IdP outage causing cascading failures | Transient errors return 502 without revoking session; client retries naturally | +| Concurrent refresh thundering herd | singleflight deduplication; one IdP call per session per expiry window | +| RT replay after rotation | `coalesce()` stores new RT when IdP rotates; old RT is replaced atomically | + +## Alternatives Considered + +### Alternative 1: Background Refresh Loop + +- **Description**: A background goroutine periodically scans all sessions and refreshes tokens before they expire +- **Pros**: Zero latency on requests (tokens always pre-refreshed), IdP outage resilience (grace period with pre-refreshed tokens) +- **Cons**: Requires `Start`/`Stop` lifecycle, `ListUpstreamTokenSessions()` method, background goroutine management, and refreshes tokens for abandoned sessions (unnecessary IdP calls) +- **Why not chosen**: Adds significant complexity for minimal benefit. The ~200ms on-read refresh cost (once per hour per session) is imperceptible given MCP tool call latencies of 500ms-2s. Can be added as a Phase 2 optimization without changing the `Service` interface. + +### Alternative 2: Lazy Refresh in Middleware + +- **Description**: Add refresh logic directly in the upstream swap middleware +- **Pros**: Simpler, fewer files changed +- **Cons**: The middleware would need to know about IdP providers, storage internals, and session revocation. This violates the architecture boundary: storage and IdP interaction are AS concerns, not middleware concerns. When the AS goes out-of-process, the middleware would need a major rewrite. +- **Why not chosen**: Wrong abstraction layer. The service interface provides a clean boundary that works for both in-process and future out-of-process AS deployments. + +### Alternative 3: No Service Interface (Direct Storage Enhancement) + +- **Description**: Add refresh logic to the storage layer itself (e.g., auto-refresh on `GetUpstreamTokens`) +- **Pros**: Minimal API change +- **Cons**: Storage should not call IdP providers. It would couple the storage layer to network calls, making testing difficult and violating the single-responsibility principle. +- **Why not chosen**: Storage is a persistence concern; token refresh is a business logic concern. Mixing them creates an untestable, tightly-coupled design. + +## Compatibility + +### Backward Compatibility + +- **`MiddlewareRunner` interface change**: `GetUpstreamTokenStorage()` is renamed to `GetUpstreamTokenService()`. This is an internal interface; no external consumers are affected. +- **`StoreUpstreamTokens` TTL change**: Sessions with refresh tokens now have a 2-hour sliding-window TTL instead of AT-expiry TTL. This is strictly better behavior (tokens live longer, not shorter). Sessions without refresh tokens are unchanged. +- **No client-facing changes**: The middleware still injects a bearer token into upstream requests. Clients see no API changes. + +### Forward Compatibility + +- The `upstreamtoken.Service` interface is designed for out-of-process AS extraction. `GetValidTokens()` maps directly to a single HTTP endpoint. +- Background refresh can be added to the concrete implementation without changing the interface. +- Redis storage (THV-0035) will implement `UpdateUpstreamTokens` and `RevokeSessionByTSID` using the same interface contracts. + +## Implementation Plan + +### Phase 1: Storage Layer Extensions + +**Files**: +- `pkg/authserver/storage/types.go` - Add `UpdateUpstreamTokens` to `UpstreamTokenStorage` interface. Add `SessionRevoker` interface. +- `pkg/authserver/storage/memory.go` - Implement new methods including `RevokeSessionByTSID`. Change `StoreUpstreamTokens` TTL logic (RT present -> 2h sliding window). +- `pkg/authserver/storage/config.go` - Add `DefaultUpstreamInactivityTimeout = 2h` +- `pkg/authserver/storage/mocks/` - Regenerate + +**Tests**: New tests for `UpdateUpstreamTokens` (including TTL reset behavior), `RevokeSessionByTSID`, and the `StoreUpstreamTokens` TTL behavior change. + +### Phase 2: Service Package + +**Files**: +- `pkg/authserver/upstreamtoken/service.go` - `Service` interface, `UpstreamCredential`, sentinel errors +- `pkg/authserver/upstreamtoken/inprocess.go` - `inProcessService` with `GetValidTokens` (on-read refresh, singleflight, session revocation on permanent failure) +- `pkg/authserver/upstreamtoken/inprocess_test.go` - Unit tests +- `pkg/authserver/upstreamtoken/mocks/` - Generated mock for `Service` + +**Tests**: +- Happy path: valid tokens returned as UpstreamCredential +- Expired AT with RT: successful refresh, tokens updated +- Expired AT without RT: `ErrNoRefreshToken` +- Refresh failure (invalid_grant): `ErrRefreshFailed`, internal session revoked, upstream tokens deleted +- Refresh failure (transient): `ErrRefreshFailed`, internal session NOT revoked +- Singleflight dedup: concurrent calls, only one refresh +- RT rotation: new RT stored when IdP rotates + +### Phase 3: Wiring Chain + +**Files**: +- `pkg/authserver/runner/embeddedauthserver.go` - Construct provider + service, add `UpstreamTokenService()` accessor +- `pkg/transport/types/transport.go` - `GetUpstreamTokenStorage()` -> `GetUpstreamTokenService()` +- `pkg/runner/runner.go` - Updated implementation +- `pkg/transport/types/mocks/` - Regenerate + +### Phase 4: Middleware Rewrite + +**Files**: +- `pkg/auth/upstreamswap/middleware.go` - Replace `StorageGetter` with `ServiceGetter`, simplify to `GetValidTokens()` + error mapping +- `pkg/auth/upstreamswap/middleware_test.go` - Rewrite tests with mock `upstreamtoken.Service` + +### Phase 5: Cleanup and Verification + +- Remove dead imports and unused code +- Full test suite (unit + e2e) +- Lint, generate, build + +Each phase leaves the codebase compilable. Phases 3+4 should ship together if in separate PRs. + +### Dependencies + +- No new external dependencies. Uses the standard library's `golang.org/x/sync/singleflight`. +- Depends on existing `pkg/authserver/upstream/` (already implements `RefreshTokens()`) +- Depends on existing `pkg/authserver/storage/` (extended, not replaced) + +## Testing Strategy + +- **Unit tests**: Service implementation with mocked storage, provider, and revoker. Cover all error paths, singleflight deduplication, and RT rotation. +- **Unit tests**: Storage layer extensions (`UpdateUpstreamTokens`, `RevokeSessionByTSID`, TTL behavior change) +- **Unit tests**: Middleware error mapping (service errors to HTTP status codes) +- **Integration tests**: Full refresh flow with real storage and mocked IdP +- **End-to-end tests**: OAuth flow through token expiry, refresh, and re-authentication +- **Concurrency tests**: Verify singleflight deduplication under concurrent load + +## Documentation + +- Update `docs/arch/` in toolhive with upstream token refresh design +- GoDoc for new `pkg/authserver/upstreamtoken/` package +- Update operational guides with token lifecycle behavior changes + +## Open Questions + +None. All design questions resolved. + +## References + +- [THV-0019: OAuth Authorization Server Design](rfcs/THV-0019-auth-server-design.md) - Auth server architecture +- [THV-0031: Embedded Authorization Server in Proxy Runner](rfcs/THV-0031-auth-server-integration.md) - Auth server integration +- [THV-0035: Redis-Backed Storage for Auth Server](rfcs/THV-0035-auth-server-redis-storage.md) - Redis storage (implements same interfaces) +- [OAuth 2.0 RFC 6749](https://datatracker.ietf.org/doc/html/rfc6749) - Token refresh grant type (Section 6) +- [Fosite OAuth2 Library](https://github.com/ory/fosite) - OAuth2 implementation +- [golang.org/x/sync/singleflight](https://pkg.go.dev/golang.org/x/sync/singleflight) - Request deduplication + +--- + +## RFC Lifecycle + +### Review History + +| Date | Reviewer | Decision | Notes | +|------|----------|----------|-------| +| 2026-02-20 | TBD | Draft | Initial draft | + +### Implementation Tracking + +| Repository | PR | Status | +|------------|-----|--------| +| toolhive | TBD | Not Started | diff --git a/rfcs/THV-XXXX-user-to-agent-delegation.md b/rfcs/THV-XXXX-user-to-agent-delegation.md new file mode 100644 index 0000000..1390dc8 --- /dev/null +++ b/rfcs/THV-XXXX-user-to-agent-delegation.md @@ -0,0 +1,1090 @@ +# RFC-XXXX: User-to-Agent Delegation via OAuth Token Exchange + +- **Status**: Draft +- **Author(s)**: Jakub Hrozek (@jhrozek) +- **Created**: 2026-04-24 +- **Last Updated**: 2026-04-24 +- **Target Repository**: toolhive +- **Related Issues**: TBD + +> **Note on numbering**: this file uses the placeholder `THV-XXXX` until the +> PR is opened, at which point the number will be set to the PR number per +> the [naming convention](../CONTRIBUTING.md#rfc-file-naming-format). + +## Summary + +Extend ToolHive's embedded authorization server with the [RFC 8693 OAuth 2.0 +Token Exchange](https://datatracker.ietf.org/doc/html/rfc8693) grant so an +authenticated *agent* can present a user's JWT and receive a delegated JWT +that names the user as the principal (`sub`) and the agent as the acting +party (`act`). The exchange supports both same-trust-domain tokens (subject +tokens issued by ToolHive's own AS) and federated tokens issued by an +external OIDC provider, configured via a new trust-only upstream type +(`oidc-trust`). Cedar authorization is extended to read the `act` claim so +policies can scope what an agent may do *on behalf of* a user. + +## Problem Statement + +### What is "user-to-agent delegation"? + +In an *agentic* deployment of MCP, a non-human caller — typically an LLM agent +— calls an MCP server and ultimately a backend resource. Two distinct +identities are present on every such call: + +- the **user** who originally requested the work (e.g. "Alice") +- the **agent** that is performing the work on the user's behalf + (e.g. "coding-agent") + +Today, the embedded auth server (RFC-0019, RFC-0031) issues a JWT that +identifies *the user* and nothing else. The agent is invisible. This forces +a binary choice at the MCP server / backend: + +1. **Pretend the agent is the user.** The agent sends the user's JWT + verbatim. Authorization, audit, and rate-limiting see only the user. An + audit log entry that says "Alice deleted the repo" cannot tell whether + Alice typed the command herself or whether her agent did it on her behalf + from a tool call that may itself have come from a malicious prompt + injection. + +2. **Treat the agent as the only identity.** The agent uses its own + `client_credentials` token. The user is invisible. Per-user policy and + audit are lost; the agent looks like a single super-user account. + +Neither option is acceptable for production. We need a token format that +carries **both** identities at once, with a clear semantic distinction +between *principal* (whose data) and *actor* (who is acting). + +### Why the existing token exchange middleware (RFC-0007) does not solve it + +RFC-0007 (`pkg/oauth/tokenexchange`) is **outbound** middleware: it takes the +incoming token to the proxy and exchanges it at an *external* token endpoint +for an upstream-bound token before forwarding the request to a backend. It +does not change the identity of the principal — the result still names the +same user — and it never adds an actor. It cannot create a token that says +"this is Alice, acted upon by coding-agent" because the AS at the other end +of the exchange has no notion of the agent. + +This RFC works on the **inbound** side: the *AS itself* learns to issue +delegated tokens. The outbound middleware then continues to work unchanged on +top. + +### Who is affected + +- **Platform operators** running ToolHive with an embedded AS in front of + agentic workloads, who need per-agent audit and per-agent policy on top of + per-user policy. +- **MCP server authors** who want to express authorization rules of the form + "Alice may write to the repository, but coding-agent acting on Alice's + behalf may only read". +- **Compliance / security teams** who require an auditable record of which + agent acted on which user's behalf, distinct from the user's own actions. + +### Why now + +ToolHive is increasingly used to front agentic systems, and the existing +single-identity JWT model is the largest gap in our authorization story. +RFC 8693 has been an IETF standard since 2020 and is supported by every +major OAuth library, so the solution is well-trodden ground; the only novel +part is the integration with ToolHive's existing AS, upstream-provider +model, and Cedar authorization. + +## Background + +This section is for readers unfamiliar with RFC 8693 or with ToolHive's +auth-server architecture. Readers comfortable with both can skip ahead to +[Goals](#goals). + +### RFC 8693 in 30 seconds + +A token exchange request is an HTTP POST to the token endpoint with +`grant_type=urn:ietf:params:oauth:grant-type:token-exchange`. Its core +parameters are: + +| Parameter | Meaning | +|-----------|---------| +| `subject_token` | The token representing **the principal** the new token is *for* | +| `subject_token_type` | The format of `subject_token` (typically `access_token` or `id_token`) | +| `actor_token` *(optional)* | The token representing **the acting party** | +| `actor_token_type` *(optional)* | The format of `actor_token` | +| `audience`, `scope`, `resource` | Standard scoping parameters | + +The token endpoint validates both tokens, applies its own policy, and issues +a new token. RFC 8693 §4.1 specifies that delegated tokens carry an **`act` +claim**: a nested object whose `sub` field names the actor. + +```json +{ + "iss": "https://auth.example.com", + "sub": "alice@example.com", + "act": { "sub": "coding-agent" }, + "exp": 1714000000 +} +``` + +The principal is `alice`. The actor is `coding-agent`. Backends authorise +against `sub`; audit records both. + +### What the AS already does (after RFC-0019, RFC-0031, RFC-0052) + +- Speaks OAuth 2.0 / OIDC against MCP clients on `/oauth/{authorize,token, + callback,register}` and `/.well-known/{openid-configuration, + oauth-authorization-server,jwks.json}`. +- Authenticates the user against one or more configured **upstream IDPs** + (`oidc` for full OIDC discovery, `oauth2` for explicit-endpoint OAuth 2.0) + and stores the resulting upstream tokens. +- Issues short-lived signed JWTs whose `tsid` claim links to the stored + upstream tokens for later retrieval by vMCP outbound middleware. +- Runs **embedded** inside the proxy runner, configured by an + `MCPExternalAuthConfig` of `type: embeddedAuthServer`. + +Today the AS supports the `authorization_code`, `refresh_token`, and +`client_credentials` grants. It does **not** support token exchange. + +## Goals + +- Add the RFC 8693 token-exchange grant to the embedded AS so an + authenticated client can exchange a user JWT for a delegated JWT that + carries both identities. +- Support **two trust models** for the subject token: + 1. *Same-domain*: the subject token is an AS-issued JWT, validated + against the AS's own JWKS. + 2. *Federated*: the subject token is issued by an external OIDC provider, + trusted via a new `oidc-trust` upstream type and validated against + that provider's JWKS. +- Bind the actor identity to the authenticated OAuth client to prevent + replay of leaked actor tokens by a different client. +- Extend Cedar authorization so policies can read `act.sub` and write rules + that depend on which agent is acting. +- Plumb operational details for in-cluster IDPs: a CA-bundle reference for + private TLS and a flag to permit private-IP issuer endpoints. +- Stay strictly additive: deployments that do not configure delegation or + trust upstreams are unaffected, and the existing `authorization_code`, + `refresh_token`, and `client_credentials` flows continue to work + unchanged. + +## Non-Goals + +- **Multiple actor chaining (`act.act.…`).** RFC 8693 §4.1 allows nested + `act` claims to record an agent acting on behalf of another agent acting + on behalf of a user. This RFC issues a single-level `act` claim only. +- **Issuing OIDC ID tokens or refresh tokens from the exchange.** Only an + access token is returned. The exchange is one-shot; the agent re-exchanges + on its own schedule. +- **Token Exchange between two different ToolHive AS instances** (trust + federation across deployments). All federated trust is one-way: the AS + trusts an external OIDC provider, not vice versa. +- **Redirect-flow login against `oidc-trust` upstreams.** The new upstream + type contributes trust material only; it does not participate in browser + redirect flows. +- **mTLS / SPIFFE-based client authentication.** A separate effort explored + using the agent's mTLS identity as the actor; this RFC uses the + authenticated OAuth client identity (HTTP Basic, `client_secret_post`, or + similar). Adding mTLS-derived actors is left to a follow-up. +- **Step-up authentication** when the subject token's authentication context + is insufficient for the requested scope. +- **CLI-mode (`thv proxy`) integration.** Like RFC-0031, this is an + embedded-AS feature; the CLI does not expose an embedded AS to extend. + +## Proposed Solution + +### High-Level Design + +The AS gains a new fosite handler for the token-exchange grant. The handler +sits alongside the existing handlers in the same fosite provider; nothing +about the existing endpoints or grant types changes. + +```mermaid +flowchart TB + subgraph AuthServer["Embedded Auth Server"] + Token["/oauth/token"] + subgraph Handlers["Fosite token-endpoint handlers"] + AC["Authorization Code"] + RT["Refresh Token"] + CC["Client Credentials"] + TE["Token Exchange (NEW)"] + end + subgraph Validators["Subject-token validators"] + SV["Self-issued (own JWKS)"] + MV["Multi-issuer
(self + trusted external)"] + end + subgraph Upstreams["Upstream provider types"] + OIDC["oidc"] + OAUTH2["oauth2"] + TRUST["oidc-trust (NEW)"] + end + end + + Token --> AC + Token --> RT + Token --> CC + Token --> TE + TE --> MV + MV --> SV + MV --> TRUST + TRUST -->|JWKS via OIDC discovery| External[External OIDC IdP] +``` + +#### Two flows: same-domain and federated + +The exchange has two flows that differ only in which JWKS validates the +subject token. Everything downstream of validation is identical. + +**Same-domain flow** — the user has an AS-issued JWT and the agent +exchanges it for a delegated AS-issued JWT. + +```mermaid +sequenceDiagram + participant User + participant Agent + participant AS as ToolHive AS + participant Backend as MCP Server + + User->>AS: 1. Authorization code flow + AS-->>User: 2. JWT-A (sub=alice) + User->>Agent: 3. Pass JWT-A to agent + Note over Agent: Agent authenticates as
OAuth client coding-agent + + Agent->>AS: 4. POST /oauth/token
grant_type=token-exchange
subject_token=JWT-A
(client_secret_basic auth) + Note over AS: Validate JWT-A against own JWKS
Verify client = coding-agent
Build delegated session + AS-->>Agent: 5. JWT-B
sub=alice, act={sub:coding-agent} + + Agent->>Backend: 6. Bearer JWT-B + Note over Backend: Auth middleware validates JWT-B
Cedar: principal=alice,
actor=coding-agent + Backend-->>Agent: 7. Response +``` + +**Federated flow** — the user authenticated against an external OIDC IDP +(say Keycloak) and holds a Keycloak-issued JWT. The agent presents that +token directly, without first exchanging it through the AS authorization +code flow. + +```mermaid +sequenceDiagram + participant User + participant Agent + participant Ext as External OIDC IdP
(Keycloak) + participant AS as ToolHive AS + participant Backend as MCP Server + + User->>Ext: Authenticate + Ext-->>User: JWT-X (iss=keycloak, sub=alice) + User->>Agent: Pass JWT-X + + Agent->>AS: POST /oauth/token
grant_type=token-exchange
subject_token=JWT-X + Note over AS: peek iss claim → keycloak
route to MultiIssuerValidator
fetch keycloak JWKS via discovery
verify JWT-X
verify client = coding-agent + AS-->>Agent: JWT-B
iss=toolhive-as
sub=alice
act={sub:coding-agent} + + Agent->>Backend: Bearer JWT-B + Backend-->>Agent: Response +``` + +The federated flow lets organisations keep their existing IDP as the user's +authentication source while still using ToolHive's per-agent delegation and +audit semantics. + +### Vocabulary + +This RFC and the implementation use the following terms consistently. They +match RFC 8693 wherever the spec defines them. + +| Term | Meaning | +|------|---------| +| **Principal** | The identity carried in `sub` of the issued token. The "on behalf of" party. | +| **Actor** | The identity carried in `act.sub` of the issued token. The party performing the action. | +| **Subject token** | The token presented as `subject_token`. Names the principal. | +| **Actor token** | The token presented as `actor_token`. Names the actor. Optional in this RFC. | +| **Authenticated client** | The OAuth client that authenticated to the token endpoint (HTTP Basic etc.). Used as the actor when no `actor_token` is supplied. | +| **Self-issued token** | A JWT whose `iss` matches the AS's own issuer URL. Validated against the AS's own JWKS. | +| **Trusted external token** | A JWT whose `iss` matches one of the configured `oidc-trust` upstreams. Validated against that upstream's JWKS. | +| **Delegated token** | The access token returned from a token exchange. Carries `sub` and `act`. | + +### Detailed Design + +#### 1. Token-exchange handler + +A new package `pkg/authserver/server/tokenexchange` implements +`fosite.TokenEndpointHandler` for the `urn:ietf:params:oauth:grant-type: +token-exchange` grant. + +The handler runs after fosite's client authentication step, so by the time +it executes, `requester.GetClient()` is the authenticated OAuth client. + +Algorithm: + +1. Reject if the client is public. Token exchange requires a confidential + client. +2. Parse and validate the RFC 8693 form parameters (`subject_token`, + `subject_token_type`, optional `actor_token`/`actor_token_type`). + - Accepted subject token types: `access_token`, `jwt`, `id_token`. + - Accepted actor token types: `access_token`, `jwt`. (`id_token` + intentionally excluded — see *Security Considerations*.) +3. Validate the subject token via the configured `SubjectTokenValidator` + (see §2). Returns canonical claims (`sub`, `name`, `email`, `exp`,…) on + success. +4. Resolve the actor identity (see §3). Result is a single string: either + the actor token's `sub` (after a binding check) or the authenticated + client's `client_id`. +5. Validate that every requested scope is allowed for the client and that + the requested audience is permitted. +6. Build a delegated `session.Session` with: + - `sub` = subject token's `sub` + - `act` = `{ "sub": }` + - `name`, `email` propagated from the subject token (UI surfaces) + - **No `tsid` claim.** Delegated tokens are not bound to a stored + upstream session; the agent is acting on the *user's* identity but + does not gain access to upstream IDP tokens stored under the user's + `tsid`. +7. Cap the lifetime to `min(subject_remaining, configured_delegation_max)`. + The configured maximum defaults to 15 minutes and is bounded above by 24 + hours. A delegated token may not outlive the user's own grant. +8. Hand off to fosite's standard `IssueAccessToken` to sign and return. + +Pseudocode for the central decision in step 6: + +```go +delegatedSession := session.New( + validatedClaims.Subject, // sub = principal (user) + "", // no upstream session link + actorSub, // session.client_id field for audit + session.UserClaims{Name, Email}, +) +delegatedSession.JWTClaims.Extra["act"] = map[string]any{"sub": actorSub} +``` + +#### 2. Subject-token validation: same-domain vs federated + +The handler depends on a `SubjectTokenValidator` interface: + +```go +type SubjectTokenValidator interface { + Validate(ctx context.Context, rawToken string) (*ValidatedClaims, error) +} +``` + +Two implementations: + +- **`SelfIssuedTokenValidator`** — validates against the AS's own JWKS. + Used when the deployment has only same-domain delegation. +- **`MultiIssuerTokenValidator`** — wraps a `SelfIssuedTokenValidator` plus + zero or more `TrustedIssuer` entries. On `Validate(ctx, raw)`: + 1. Parse the JWT *without* signature verification to peek at `iss`. + 2. If `iss` equals the AS's own issuer, delegate to the self-issued + validator. + 3. If `iss` matches one of the configured trusted issuers, fetch (and + cache, with a 5-minute TTL) the issuer's JWKS via OIDC discovery and + verify there. + 4. Otherwise, reject — unknown issuers are never trusted. + +The factory selects which validator to install based on whether the AS +config contains any `oidc-trust` upstreams. Same-domain-only deployments +get the simpler self-issued validator with no external HTTP traffic. + +```go +type TrustedIssuer struct { + IssuerURL string + ExpectedAudience string // required `aud` claim, may be empty to skip +} +``` + +The `MultiIssuerTokenValidator` uses an `*http.Client` injected at +construction time. The same client is used for both OIDC discovery and +JWKS retrieval, so a single CA bundle / private-IP-permission policy +applies to everything (see §6). + +The discovery document is parsed into the existing +`pkg/oauthproto.OIDCDiscoveryDocument` type. The JWKS URL is validated +against an SSRF guard that requires HTTPS and rejects private/loopback +addresses unless `AllowPrivateIP` is enabled. + +#### 3. Actor identity resolution + +The actor is resolved by the handler in this order: + +1. **If `actor_token` is supplied:** + - Validate it against the AS's *own* JWKS — actor tokens are *always* + self-issued in this RFC. This prevents an external IDP from + unilaterally asserting "this is the agent". + - Enforce the binding `actor_token.sub == authenticated_client.client_id`. + Without this, a leaked actor token could be presented by a different + client to claim its identity. The check is unconditional, regardless + of how the client authenticated (basic, post, future mTLS). + - On success the actor is `actor_token.sub`. + +2. **If `actor_token` is absent:** + - The actor is the authenticated client's `client_id`. + +The `actor_token` path exists because some deployments want a richer actor +representation than just a `client_id` string — e.g., a JWT carrying +additional claims (deployment, region, agent version) that downstream +authorization or audit might consume. Today only `actor.sub` is consumed +by Cedar, but the audit record can still include the full claim set. + +The `actor_token` validator is exposed on the handler as a separate +`SubjectTokenValidator` (`selfValidator`) so it can be unit-tested in +isolation. The factory wires `selfValidator = validator` for same-domain +deployments and reuses the same self-issued validator inside the +multi-issuer wrapper for federated deployments. + +#### 4. New upstream provider type: `oidc-trust` + +The existing upstream provider model (RFC-0019, RFC-0052) supports two +types: `oidc` (OIDC with discovery + redirect-flow login) and `oauth2` +(explicit-endpoint OAuth 2.0 with redirect-flow login). Both are designed +around participating in the user's *login* — they hold a `client_id` / +`client_secret` and a `redirect_uri`, and the AS uses them to exchange an +authorization code for tokens. + +A federated trust upstream is fundamentally different: the AS never logs +the user in through it, never holds a client secret for it, and never has +a redirect URI. The AS only needs to verify a signature and an audience. +Reusing `oidc` would force unnecessary fields and make the validation logic +ambiguous about whether redirect-flow is supported. + +Therefore: a third type `oidc-trust`. It holds: + +- `issuer` (required) — the OIDC issuer URL, used to construct the + discovery URL (`/.well-known/openid-configuration`) and to + match the `iss` claim of incoming tokens. +- `clientId` (optional) — used as the `ExpectedAudience` for incoming + token validation. Empty disables the audience check (some IDPs do not + populate `aud` on access tokens). +- `caBundleConfigMapRef` (optional) — see §6. +- `allowPrivateIP` (optional, default `false`) — see §6. + +`oidc-trust` upstreams implement the same `OAuth2Provider` interface as +the others for storage compatibility, but the redirect-flow methods +(`AuthorizationURL`, `ExchangeCodeForIdentity`, `RefreshTokens`) return an +explicit "not supported" error. + +The AS's `defaultUpstreamFactory` extracts every `oidc-trust` upstream +from the configured list and feeds them into the +`MultiIssuerTokenValidator` as `TrustedIssuer` entries. The remaining +upstreams (`oidc`, `oauth2`) continue to drive the redirect-flow login +chain established by RFC-0052. + +#### 5. Cedar authorization: the `act` claim + +ToolHive's Cedar authorizer (`pkg/authz/authorizers/cedar`) builds a +context map from the token's claims so policies can write rules like +`context.claim_email == "alice@example.com"`. Today the converter +silently drops nested-map claims, so the `act` claim — which is itself a +JSON object — is invisible to policies. + +This RFC adds a `map[string]interface{}` case to `convertToCedarValue` so +nested claims are converted to Cedar Records. After the change, policies +can be written like: + +```cedar +// Allow Alice to call any tool, but if she's acting through coding-agent, +// only the read-only tools are allowed. + +permit ( + principal, + action == Action::"call_tool", + resource +) +when { + context.claim_email == "alice@example.com" && + !(context has claim_act) +}; + +permit ( + principal, + action == Action::"call_tool", + resource +) +when { + context.claim_email == "alice@example.com" && + context has claim_act && + context.claim_act.sub == "coding-agent" && + resource.readOnlyHint == true +}; +``` + +The change is purely additive in Cedar: existing policies that do not +reference `claim_act` are unaffected. + +#### 6. Operational plumbing: CA bundle and `AllowPrivateIP` + +Two practical concerns appeared during implementation that do not change +the conceptual model but require new fields to surface to the operator. + +**CA bundle (`caBundleConfigMapRef`)**: an `oidc-trust` upstream is most +commonly an in-cluster Keycloak (or similar) using TLS certificates issued +by a private cluster CA. The system trust store does not contain that CA, +so OIDC discovery and JWKS fetching fail with a TLS error. The new field +points at a `ConfigMap` containing the CA in PEM form. The operator +mounts the ConfigMap into the proxy runner pod, the runner passes the +filesystem path through `RunConfig`, and the AS uses +`pkg/networking.NewHttpClientBuilder().WithCABundle(path)` to construct +the HTTP client used by the multi-issuer validator. When unset, the system +trust store is used. + +**`allowPrivateIP`**: an in-cluster issuer URL typically resolves to a +private (RFC 1918) cluster IP. ToolHive's networking package defaults to +rejecting private IPs as a defence-in-depth measure against SSRF. The new +boolean field opts a single `oidc-trust` upstream into permitting private +IP resolution for its discovery and JWKS fetches. When `false` (the +default), the existing rejection policy applies. The flag is per-upstream +because two different `oidc-trust` upstreams in the same AS may want +different policies (one in-cluster, one external). + +Both fields aggregate at AS startup: if any configured `oidc-trust` +upstream has a CA bundle or requests private IPs, the HTTP client is +built once with the union of those settings and shared by the validator +across all upstreams. A future refinement could give each upstream its +own client. + +#### 7. Wiring: `RunConfig` → AS + +The `EmbeddedAuthServerConfig` `RunConfig` gains: + +```go +type RunConfig struct { + // ... existing fields ... + + // DelegationTokenLifespan caps the lifetime of delegated tokens + // issued via RFC 8693 token exchange. Empty defaults to 15 minutes; + // bounded above by 24h. + DelegationTokenLifespan string + + Upstreams []UpstreamRunConfig // existing, but Type may now be "oidc-trust" +} + +type OIDCUpstreamRunConfig struct { + // ... existing fields ... + + // CABundlePath is the absolute filesystem path to a CA-bundle PEM. + // Used only by oidc-trust upstreams for TLS verification of the + // OIDC discovery and JWKS endpoints. Empty means "use the system + // trust store". + CABundlePath string + + // AllowPrivateIP permits OIDC discovery and JWKS endpoints to + // resolve to private IP addresses. Off by default. + AllowPrivateIP bool +} +``` + +The `authserver.Config.UpstreamProviderType` enum gains +`UpstreamProviderTypeOIDCTrust` alongside the existing `oidc` and +`oauth2`. Validation requires `oidc_config` (with at least an issuer URL) +and forbids `oauth2_config` for the new type. + +The runner derives the `extraFactories` slice for the fosite provider: + +```go +extraFactories := []oauthserver.Factory{ + tokenexchange.Factory(tokenexchange.FactoryConfig{ + DelegationLifespan: cfg.DelegationTokenLifespan, + TrustedIssuers: trustedIssuersFromOIDCTrustUpstreams(upstreams), + HTTPClient: buildHTTPClientFromCAandPrivateIPFlags(upstreams), + }), +} +``` + +When no `oidc-trust` upstreams are configured, `TrustedIssuers` is empty +and the validator collapses back to the self-issued path. + +The token-exchange grant type is advertised in +`/.well-known/oauth-authorization-server` unconditionally. (RFC 8414 +discovery has no notion of "this client may use this grant"; the +authorization decision is made per-client by fosite at the token endpoint.) + +#### 8. Operator / CRD changes + +`MCPExternalAuthConfig.spec.embeddedAuthServer.upstreamProviders[].type` +gains the enum value `oidc-trust`. The validation function is reorganised +so that: + +- `oidc` and `oauth2` continue to follow the existing XOR rule + (`oidcConfig` xor `oauth2Config`). +- `oidc-trust` requires `oidcConfig` (with a valid `issuerURL`), forbids + `oauth2Config`, and additionally accepts the optional + `caBundleConfigMapRef` and `allowPrivateIP` fields. + +`OIDCUpstreamConfig` gains: + +```go +type OIDCUpstreamConfig struct { + // ... existing fields ... + + // CABundleConfigMapRef references a ConfigMap containing the CA + // certificate for verifying the OIDC issuer's TLS certificate. Used + // for oidc-trust providers where the issuer uses a non-public CA + // (e.g., internal PKI). When nil, the system trust store is used. + // +optional + CABundleConfigMapRef *CABundleSource `json:"caBundleConfigMapRef,omitempty"` + + // AllowPrivateIP allows OIDC discovery and JWKS endpoints to resolve + // to private IP addresses. Required for in-cluster OIDC issuers + // where the service DNS name resolves to a cluster-internal IP. + // Default: false. + // +optional + AllowPrivateIP bool `json:"allowPrivateIP,omitempty"` +} +``` + +The `EmbeddedAuthServerConfig` gains +`delegationTokenLifespan *metav1.Duration`. + +The operator's `controllerutil.GenerateAuthServerVolumes` is extended to +generate a CA-bundle volume mount per `oidc-trust` upstream that has a +`caBundleConfigMapRef`. The mount path follows the convention +`/etc/toolhive/authserver/upstream-ca//` and is +threaded through `RunConfig` as `CABundlePath`. + +### Configuration Changes + +A complete example: an embedded AS with one `oidc` redirect-flow upstream +(corporate SSO for user login) and one `oidc-trust` upstream (Keycloak +for federated delegation), with the proxy runner mounting both the +Keycloak CA bundle and the agent's client secret. + +```yaml +apiVersion: toolhive.stacklok.com/v1beta1 +kind: MCPExternalAuthConfig +metadata: + name: agentic-auth + namespace: my-mcp +spec: + type: embeddedAuthServer + embeddedAuthServer: + issuer: https://auth.toolhive.example.com + delegationTokenLifespan: 15m # cap delegated tokens + signingKeys: + - secretRef: { name: as-signing-keys, key: ec-key.pem } + hmacSecretRefs: + - { name: as-hmac, key: hmac-0 } + upstreamProviders: + # Existing-style upstream: corporate SSO for user login + - name: corporate-sso + type: oidc + oidcConfig: + issuerURL: https://sso.corp.example.com + clientID: toolhive-as + clientSecretRef: { name: corp-sso, key: client-secret } + redirectURI: https://auth.toolhive.example.com/oauth/callback + + # NEW-style upstream: Keycloak whose tokens we trust as subject_tokens + - name: keycloak + type: oidc-trust + oidcConfig: + issuerURL: https://keycloak.svc.cluster.local:8443/realms/agents + clientID: toolhive-resource # used as ExpectedAudience + caBundleConfigMapRef: + configMapRef: { name: keycloak-ca, key: ca.crt } + allowPrivateIP: true # in-cluster service IP +``` + +A delegation request from the agent to the AS: + +```http +POST /oauth/token HTTP/1.1 +Host: auth.toolhive.example.com +Authorization: Basic Y29kaW5nLWFnZW50OnMzY3JldA== +Content-Type: application/x-www-form-urlencoded + +grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange +&subject_token=eyJhbG... +&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt +&audience=https%3A%2F%2Fmcp.example.com +``` + +Successful response (`200 OK`): + +```json +{ + "access_token": "eyJhbGc...", + "token_type": "Bearer", + "expires_in": 899, + "issued_token_type": "urn:ietf:params:oauth:token-type:access_token" +} +``` + +Decoding `access_token`: + +```json +{ + "iss": "https://auth.toolhive.example.com", + "sub": "alice@corp.example.com", + "act": { "sub": "coding-agent" }, + "aud": "https://mcp.example.com", + "exp": 1714000899, + "iat": 1714000000 +} +``` + +## Security Considerations + +### Threat Model + +The token exchange grant is, by design, a privilege-relevant operation: +the AS is being asked to mint a token in *somebody else's* name. Threats: + +- **Subject-token forgery / replay.** A caller crafts or steals a JWT and + asks for a delegated token in that user's name. +- **Actor-token hijack.** A caller steals an actor token and presents it + with their own client authentication to claim a different agent's + identity. +- **External-IDP impersonation.** If the AS naively trusts whatever JWT it + receives, a token from an *unconfigured* issuer could be accepted. +- **SSRF via discovery.** An attacker who controls an `oidc-trust` + upstream's issuer URL could try to point the AS at internal infrastructure + via the discovery / JWKS fetch. +- **Public-client misuse.** A public OAuth client (no secret) could attempt + to exchange tokens. +- **Lifetime escalation.** A short-lived user token is exchanged for a + long-lived delegated token, extending the effective grant. + +### Authentication and Authorization + +- **Confidential clients only.** The handler rejects any request from a + client whose `Public()` is true. Public clients have no proof of identity + and therefore cannot serve as actors. +- **Subject token signature.** Verified against the AS's own JWKS + (same-domain) or against the trusted issuer's JWKS (federated). Standard + claims (`iss`, `aud`, `exp`, `iat`, `nbf`, `sub`) are validated per RFC + 7519. Tokens signed with `none` are rejected by go-jose. +- **Issuer allowlist.** The multi-issuer validator rejects any token whose + `iss` is neither the AS itself nor a configured `oidc-trust` upstream. + There is no wildcard or regex matching; issuer URLs must match exactly. +- **Actor-token binding.** When `actor_token` is supplied, its `sub` must + equal the authenticated client's `client_id`. This binds the actor + identity to the authenticated party and prevents replay of leaked actor + tokens by a different client. The check is unconditional and runs + regardless of authentication method. +- **Standard fosite scope and audience checks** apply to the request: the + client must be allowed each requested scope and each requested audience. + +### Data Security + +- **No new secrets at rest.** The handler does not persist actor tokens or + subject tokens. Delegated tokens are signed in memory and returned over + TLS; their contents (the user's `sub`, `name`, `email`) are no more + sensitive than what the AS already issues today. +- **CA bundles** are referenced through Kubernetes `ConfigMap`s + (non-secret) and mounted read-only into the proxy runner pod with + `0400` mode. +- **JWKS caching** uses an in-memory map with a 5-minute TTL. No JWKS or + discovery response is persisted. + +### Input Validation + +- **Form parameters.** Each RFC 8693 parameter is checked for presence and + format. `subject_token_type` must be one of three known URIs; + `actor_token_type` must be one of two (`id_token` is intentionally + excluded — see below). Mismatched pairs (e.g., `actor_token_type` + without `actor_token`) are rejected. +- **Why `id_token` is rejected as `actor_token_type`.** An ID token is an + *identity assertion* about a user-driven login session, including a + `nonce` and the user's authentication context. It is conceptually wrong + to use it as a bearer credential proving "I am the agent". Allowing it + would invite confused-deputy patterns where a user's ID token is replayed + as an actor token. +- **JWT parsing.** Both validators use `github.com/go-jose/go-jose/v4`, + which enforces strict JSON Web Algorithm allowlists (no `none`, no `HS*` + on RSA/EC keys). +- **Issuer URL.** Discovery URLs are constructed from the configured + issuer plus `/.well-known/openid-configuration`. The constructed URL is + validated to be HTTPS; the `jwks_uri` returned by discovery is similarly + validated and additionally checked against an SSRF allowlist (no + loopback, no link-local, no private IP unless `allowPrivateIP=true` for + that upstream). + +### Secrets Management + +- The agent's OAuth client secret is provided through the existing + `clientSecretRef` mechanism on its registered client; this RFC does not + introduce a new secret type. +- The AS's signing keys and HMAC secrets are unchanged from RFC-0031. +- No client secret is required for `oidc-trust` upstreams — they are + trust-only and never participate in OAuth flows that would require one. + +### Audit and Logging + +The handler emits debug logs with the following fields on a successful +exchange: + +| Field | Meaning | +|-------|---------| +| `subject` | The principal (user) `sub` from the subject token | +| `actor` | The actor `sub` (either authenticated `client_id` or `actor_token.sub`) | +| `lifetime` | The capped lifetime of the issued token | + +On validation failure, the failure reason and the actor are logged at +debug. The token contents themselves are never logged. The fields chosen +allow an operator to reconstruct, from log output alone, "who acted as +whom and for how long" without exposing the raw JWTs. + +A future RFC may add a structured audit-log emitter for delegation events +specifically; the current design relies on the existing `slog` plumbing +shared with the rest of the AS. + +### Mitigations + +- **Lifetime cap.** Delegated tokens are bounded by both the configured + `DelegationTokenLifespan` (default 15m, max 24h) and the remaining + lifetime of the subject token, whichever is shorter. This blunts the + blast radius of a stolen delegated token. +- **No upstream-token access.** Delegated tokens have no `tsid` claim, so + the outbound-swap middleware (RFC-0052) cannot use them to retrieve the + user's stored upstream tokens. The agent's delegated token is good only + for AS-issued downstream backends. +- **No refresh token.** The exchange returns only an access token. The + agent must re-exchange when the token expires, giving the AS a chance to + re-validate the subject token (which may by then have been revoked or + expired). +- **Public-client reject** at the very first step. +- **Issuer allowlist** — see above. +- **TLS-by-default for discovery / JWKS** — `https://` required; + exceptions explicit and per-upstream. + +## Alternatives Considered + +### Alternative 1: extend the existing outbound `tokenexchange` middleware + +Add an "act-claim" mode to `pkg/oauth/tokenexchange` (the outbound +middleware introduced in RFC-0007) so it adds an actor when forwarding to +the upstream. + +- **Pros**: no AS change; works without an embedded AS at all. +- **Cons**: the outbound middleware exchanges at an *external* token + endpoint, which has no notion of the agent and would not produce a + matching `act` claim. We'd be mis-using an outbound feature for an + inbound concern. Also it would require *every* downstream IDP to + implement RFC 8693 with `act` semantics, which most do not. +- **Why not chosen**: the trust boundary is wrong. The party that decides + "this agent may act on this user's behalf" must be in our trust domain. + The embedded AS is that party. + +### Alternative 2: actor identity from mTLS / SPIFFE only, no `actor_token` + +Always derive the actor from the agent's mTLS client certificate (via +SPIFFE ID extraction) and reject `actor_token` outright. + +- **Pros**: cryptographically strong actor identity; no token replay + concerns; aligns with WIMSE / agent-identity directions. +- **Cons**: requires every agent to have a mTLS-capable transport and a + SPIFFE workload-attestation infrastructure. Excludes simpler deployments + using `client_secret` auth. A prior iteration of this work bundled the + mTLS path; we extracted it into a follow-up so the OAuth-only path can + ship first. +- **Why not chosen now**: scope. The `actor_token` + binding-check pattern + is RFC 8693's own answer and works without infrastructure changes. mTLS + remains a future option; the `resolveActorIdentity` function is + structured so an mTLS-context source can be added later as a third + branch. + +### Alternative 3: nest as a "type" inside the existing `oidc` upstream + +Add a `mode: trust-only` field to `oidc` instead of introducing a third +type. + +- **Pros**: smaller enum. +- **Cons**: the `oidc` type's existing fields (`clientSecretRef`, + `redirectURI`, scopes) become contextually invalid; CRD validation + becomes a nest of conditional XORs. Two distinct types are clearer. +- **Why not chosen**: explicit type wins over contextual modes. + +### Alternative 4: validate trusted-issuer tokens only via OIDC userinfo + +Instead of fetching JWKS, hit the issuer's `userinfo` endpoint with the +subject token to verify it. + +- **Pros**: works for opaque (non-JWT) tokens. +- **Cons**: 1 extra round-trip per exchange; not all issuers expose + userinfo; cannot extract `act` semantics. RFC 8693 is JWT-shaped. +- **Why not chosen**: scope is JWT subject tokens. + +## Compatibility + +### Backward Compatibility + +This change is fully additive: + +- The token-exchange grant is opt-in via `delegationTokenLifespan` / + having clients permitted to use it. A deployment that does not configure + any `oidc-trust` upstreams and has no agent clients keeps issuing + exactly the same tokens as before. +- The `oidc-trust` upstream type is a new enum value. Existing + `MCPExternalAuthConfig` resources never set it. +- `OIDCUpstreamConfig.caBundleConfigMapRef` and `allowPrivateIP` are new + optional fields; existing resources omit them. +- The Cedar `act`-claim conversion is purely additive: existing policies + do not reference `claim_act`, so their evaluation is unchanged. +- `EmbeddedAuthServerConfig.delegationTokenLifespan` is optional with a + safe default. + +### Forward Compatibility + +- The `actor_token` validation lives behind the + `SubjectTokenValidator` interface, so future actor sources (mTLS-derived + SPIFFE IDs, asymmetric-key-bound tokens per RFC 7800, …) can be added + by composing additional branches inside `resolveActorIdentity`. +- `TrustedIssuer` is a struct so future per-issuer policy fields (issuer- + specific `aud` lists, allowed `sub` patterns, audit tags) can be added + without breaking the constructor. +- Multi-actor chaining (`act.act…`) is structurally supported by the + underlying claim shape; this RFC chooses not to populate it but does + not preclude a follow-up. + +## Implementation Plan + +The work has already been prototyped and validated against a Keycloak + +in-cluster proxy runner test environment. The plan below splits the +change into reviewable PRs that can land independently. Each PR is +self-contained and the AS continues to build and pass existing tests at +every stage. + +### Phase 1: core handler (foundation) + +Goal: the AS understands the token-exchange grant for *same-domain* +subject tokens, with the actor being the authenticated client. No +federated trust, no `actor_token`, no Cedar changes. + +- New package `pkg/authserver/server/tokenexchange` with + `Handler`, `SelfIssuedTokenValidator`, `Factory`. +- Wire the factory into `createProvider` via `oauthserver.NewAuthorizationServer`'s + `Factory` slice. +- Advertise the grant in `/.well-known/oauth-authorization-server`. +- Add `DelegationTokenLifespan` to `EmbeddedAuthServerConfig` and + `RunConfig`, with default 15m and max 24h validation. + +PR size: small to medium, almost entirely new code. + +### Phase 2: `actor_token` and `id_token` subject-token type + +- Add `actor_token` / `actor_token_type` parameter handling. +- Implement `resolveActorIdentity` with the binding check. +- Accept `id_token` as a valid `subject_token_type`. + +### Phase 3: Cedar `act`-claim support + +- Add `map[string]interface{}` case to `convertToCedarValue`. +- Tests verifying `claim_act.sub` is reachable from a policy. + +This phase can land independently of phases 1-2 and parallel to phase 4. + +### Phase 4: federated trust (`oidc-trust` upstream) + +- Extract `SubjectTokenValidator` interface; implement + `MultiIssuerTokenValidator` with OIDC discovery and JWKS caching. +- Add the `oidc-trust` upstream type to `pkg/authserver` config and to + the `MCPExternalAuthConfig` CRD. +- Operator: extend `defaultUpstreamFactory`, validation, and the + embedded-AS volume generation to handle the new type. Regenerate CRDs. +- Wire trusted issuers from the configured upstreams into the factory. + +### Phase 5: operational plumbing + +- Add `caBundleConfigMapRef` and `allowPrivateIP` to + `OIDCUpstreamConfig` (CRD) and `OIDCUpstreamRunConfig`. +- Operator: generate per-upstream CA-bundle volume mounts. +- AS: aggregate CA-bundle paths and `allowPrivateIP` flags into a + single `*http.Client` injected into the multi-issuer validator. + +### Dependencies + +- RFC-0019 (auth-server overview) — accepted, implemented. +- RFC-0031 (embedded AS in proxy runner) — accepted, implemented. +- RFC-0052 (multi-upstream IDP support) — referenced but **not** a hard + dependency. The new `oidc-trust` type fits inside the multi-upstream + model when it is in place; if RFC-0052 lands first, the integration is + a no-op. If this RFC lands first, `oidc-trust` upstreams compose with + the legacy single-upstream model by being separable from the redirect- + flow login chain. + +## Testing Strategy + +- **Unit tests** for the token-exchange handler covering: valid same-domain + exchange, valid federated exchange, missing parameters, invalid + `subject_token_type`, expired subject token, public client rejection, + scope mismatch, audience mismatch, lifetime capping by subject expiry, + `actor_token` happy path, `actor_token` `sub` mismatch with client, + `actor_token` without `actor_token_type` (and vice versa), + `id_token` rejected as actor token type. +- **Unit tests** for `MultiIssuerTokenValidator`: routing on `iss`, + rejection of unknown issuers, JWKS cache TTL behaviour, discovery + failure surfacing as a token-exchange error. +- **Unit tests** for Cedar `claim_act.sub` reachability with table-driven + policies. +- **Integration tests** wiring an in-process AS with one `oidc-trust` + upstream pointing at a fake OIDC server (test fixture). Round-trip a + forged-by-test token through the exchange and verify the `act` claim. +- **End-to-end test** in the operator e2e suite: Keycloak in a kind + cluster, a `MCPExternalAuthConfig` with one `oidc` upstream and one + `oidc-trust` upstream, an `MCPServer` with Cedar policies that + distinguish direct vs delegated calls, and a script that exercises both + paths. +- **Security tests** specifically covering: `actor_token` issued by an + unrelated party rejected by binding check; `subject_token` with a forged + `iss` rejected by issuer allowlist; private-IP issuer rejected when + `allowPrivateIP=false`. + +## Documentation + +- **Architecture docs** (`docs/arch/`): a new section in the auth-server + arch doc covering delegation, with the two sequence diagrams from this + RFC. +- **Operator docs**: an example of an `MCPExternalAuthConfig` with the + new upstream type, the new fields, and the resulting agent flow. +- **`stacklok/docs-website`**: a "delegating to agents" how-to that walks + through the same Keycloak-based example end to end. +- **CRD reference** (auto-generated from kubebuilder markers): regenerated + after the type changes. + +## Open Questions + +1. **Should `delegationTokenLifespan` default differently per + environment?** 15 minutes is a reasonable default for production but + may be inconvenient for development. Consider a separate + `dev-mode` toggle that loosens the cap. +2. **Should the audit log get a dedicated structured event** (e.g., + `delegation_issued`) rather than relying on debug-level slog? A + follow-up RFC on audit-event taxonomy would be a better home. +3. **Is `actor_token` issued by a *different* AS within the same trust + federation in scope for a future iteration**, or is the binding to + the authenticated client always the right call? Today we say "always"; + a federation use case may want to relax it. +4. **Does the multi-issuer validator need per-issuer client allowlists** + — i.e., "tokens from issuer A may only be exchanged by clients from + set X"? The RFC currently treats every authenticated client as eligible + to exchange any trusted token; finer-grained policy could move into + Cedar. + +## References + +- [RFC 8693 — OAuth 2.0 Token Exchange](https://datatracker.ietf.org/doc/html/rfc8693) + (in particular §1.5 *Delegation vs. Impersonation*, §2 *Token Exchange + Request and Response*, §4.1 *act (Actor) Claim*) +- [RFC 7519 — JSON Web Token](https://datatracker.ietf.org/doc/html/rfc7519) +- [RFC 8414 — OAuth 2.0 Authorization Server Metadata](https://datatracker.ietf.org/doc/html/rfc8414) +- [RFC 7591 — OAuth 2.0 Dynamic Client Registration](https://datatracker.ietf.org/doc/html/rfc7591) +- THV-0007 — Token exchange middleware (outbound counterpart) +- THV-0019 — OAuth Authorization Server (overview and design) +- THV-0031 — Embedded Authorization Server in Proxy Runner +- THV-0035 — Auth Server Redis Storage +- THV-0050 — Dedicated Auth Server Reference (`authServerRef`) +- THV-0052 — Multi-Upstream IDP Support in the Embedded Auth Server +- THV-0053 — vMCP Embedded Auth Server +- ToolHive auth server: `pkg/authserver/` +- Cedar authorizer: `pkg/authz/authorizers/cedar/` + +--- + +## RFC Lifecycle + + + +### Review History + +| Date | Reviewer | Decision | Notes | +|------|----------|----------|-------| +| 2026-04-24 | TBD | Draft | Initial submission | + +### Implementation Tracking + +| Repository | PR | Status | +|------------|-----|--------| +| toolhive | TBD | Pending — feature branch `token-delegation` | From a37c9f9f4fd769f95390043f0e0cf5e955aac5d2 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 15 Jul 2026 11:38:44 +0100 Subject: [PATCH 4/6] Drop unrelated RFC drafts accidentally included These belong to separate proposals; keep this PR to THV-0081 only. Co-Authored-By: Claude Opus 4.8 --- ...58-rekey-upstream-token-storage-by-user.md | 442 ------- rfcs/THV-XXXX-upstream-token-refresh.md | 639 ---------- rfcs/THV-XXXX-user-to-agent-delegation.md | 1090 ----------------- 3 files changed, 2171 deletions(-) delete mode 100644 rfcs/THV-0058-rekey-upstream-token-storage-by-user.md delete mode 100644 rfcs/THV-XXXX-upstream-token-refresh.md delete mode 100644 rfcs/THV-XXXX-user-to-agent-delegation.md diff --git a/rfcs/THV-0058-rekey-upstream-token-storage-by-user.md b/rfcs/THV-0058-rekey-upstream-token-storage-by-user.md deleted file mode 100644 index 39c1f4e..0000000 --- a/rfcs/THV-0058-rekey-upstream-token-storage-by-user.md +++ /dev/null @@ -1,442 +0,0 @@ -# RFC-0058: Re-key Upstream Token Storage by Stable User Identity - -- **Status**: Draft -- **Author(s)**: Jakub Hrozek (@jhrozek) -- **Created**: 2026-04-28 -- **Last Updated**: 2026-04-28 -- **Target Repository**: toolhive -- **Related Issues**: [toolhive#5046](https://github.com/stacklok/toolhive/issues/5046), [toolhive#5047](https://github.com/stacklok/toolhive/issues/5047) -- **Related RFCs**: THV-0019 (auth server design), THV-0031 (auth server integration), THV-0035 (auth server Redis storage), THV-0052 (multi-upstream IDP) - -## Summary - -Re-key `UpstreamTokenStorage` from `(SessionID, providerName)` to `(UserID, providerName)`. Switch the JWT lookup from the custom `tsid` claim to the standard `sub` claim, then drop `tsid` from newly-issued JWTs after a bounded transition window. Fix the callback's overwrite-with-empty-refresh-token bug atomically inside the storage layer. Address newly-discovered `User.ID` stability gaps amplified by the rekey. - -## Problem Statement - -Two bugs share one structural cause: - -- **toolhive#5046** — Tokens from providers that omit `expires_in` (GitHub PATs, Vault, service tokens) were given a fabricated 1h expiry, then evicted, triggering refresh attempts on tokens that have no refresh capability. The provider was omitted from downstream responses every hour. *Fixed in branch `upstream-expiring-issues`, ships separately as a focused PR.* -- **toolhive#5047** — A fresh `/authorize` mints a new SessionID via `rand.Text()` (`pkg/authserver/server/handlers/authorize.go:91`). The previous session's upstream tokens — including a working refresh token — become unreachable from the new JWT's `tsid` claim. Acutely visible with Google IdP, which only re-issues `refresh_token` on first consent without `prompt=consent`. - -The structural cause is that `UpstreamTokens` carry **user-lifetime semantics** (a Google refresh token is valid for ~6 months) but are stored under a **flow-scoped key** that rotates on every authorize call. The stable identifier (`User.ID`, resolved from `(providerID, providerSubject)` in `pkg/authserver/server/handlers/user.go`) exists on the row but is never used as a lookup key. - -Affected users: anyone using ToolHive with an IdP that omits `refresh_token` on re-auth (notably Google), and anyone whose access pattern triggers a fresh `/authorize` rather than a `grant_type=refresh_token` flow. - -## Goals - -- Eliminate orphaning of valid upstream refresh tokens on re-authentication. -- Make storage lookups deterministic for a returning user across `/authorize` calls. -- Retire the `tsid` claim cleanly (it is purely a lookup index, not load-bearing for security — see investigation in §References). -- Fix the callback's verbatim overwrite of an empty `refresh_token` (toolhive#5047 root cause B) atomically. -- Address `User.ID` stability gaps that the rekey amplifies (chain reordering, missing subsequent-leg identity links). -- Implement the dormant `ErrInvalidBinding` defence-in-depth check that has been designed but never wired up. - -## Non-Goals - -- **Account linking across providers.** Alice via Google ≠ Alice via GitHub remains true; auto-linking is deferred per existing `pkg/authserver/storage/types.go:284-288` TODO. -- **RFC 7009 upstream refresh-token revocation on AS logout.** Separate workstream. -- **JWE encryption of stored tokens at rest.** Orthogonal hardening. -- **MCP transport-layer session IDs.** ToolHive has at least four "session" concepts (MCP transport session via `Mcp-Session-Id`, Fosite OAuth session, AS upstream-storage session, transport proxy session). Only the third is in scope. -- **Renaming the upstream provider in config.** Reflects an orphaned-data trade-off documented as a known limitation. - -## Proposed Solution - -### High-Level Design - -```mermaid -flowchart TB - subgraph Before - JWT1[JWT: sub=userUUID, tsid=randomPerFlow] - JWT1 -- tsid lookup --> S1[(UpstreamTokens
keyed by sessionID,providerName)] - end - subgraph After - JWT2[JWT: sub=userUUID] - JWT2 -- sub lookup --> S2[(UpstreamTokens
keyed by userID,providerName)] - end -``` - -The JWT already carries `User.ID` in `sub`. The read path switch is a one-line change in `pkg/auth/token.go:1190`. The interface and backend changes are internal to `pkg/authserver`. Most external consumers only touch `Identity.UpstreamTokens` (a map) and are unaffected. - -### Detailed Design - -#### Storage Interface Change - -```go -// Before -StoreUpstreamTokens(ctx, sessionID, providerName, tokens) error -GetUpstreamTokens(ctx, sessionID, providerName) (*UpstreamTokens, error) -GetAllUpstreamTokens(ctx, sessionID) (map[string]*UpstreamTokens, error) -DeleteUpstreamTokens(ctx, sessionID) error - -// After -StoreUpstreamTokens(ctx, userID, providerName, tokens) error // merging semantics built in -GetUpstreamTokens(ctx, userID, providerName) (*UpstreamTokens, error) -GetAllUpstreamTokens(ctx, userID) (map[string]*UpstreamTokens, error) -DeleteUpstreamTokens(ctx, userID, providerName) error // gain providerName -DeleteAllForUser(ctx, userID) error // user deletion / "sign out everywhere" -``` - -`Store` gains atomic merge semantics: if the inbound `RefreshToken` is empty and an existing record has a non-empty one, the storage layer (Lua script in Redis, mutex-guarded read-then-write in memory) preserves the existing refresh token. This fixes toolhive#5047 root cause B without introducing application-layer races. - -#### JWT Changes - -- Read path (`pkg/auth/token.go:1190`) reads `claims["sub"]` instead of `claims["tsid"]`. -- Write path (`pkg/authserver/server/session/session.go:120-123`) stops emitting `tsid` after the transition window. -- **No fallback to `tsid`-keyed lookup.** A read fallback is a downgrade-attack vector; a JWT carrying only `tsid` (issued before cutover) must fail validation and trigger re-auth. - -To bound the in-flight pool: rotate the AS signing key on cutover, or reduce access-token lifespan to <1h ahead of the migration so the in-flight pool drains within hours. - -#### Storage Field Rename: `SessionExpiresAt` → `LastAccessedAt` - -Field rename with semantic change: sliding-window timestamp updated by both `Get` (read-bump) and `Store`. Backends derive eviction as `LastAccessedAt + idleTTL` (default 24h, configurable). - -Optimization: only update on `Get` if the remaining TTL is below a threshold (e.g., 50%) to avoid every read becoming a write. - -#### Multi-Upstream Chain Handling - -`SessionID` (from `authorize.go:91`) stays as the **chain correlation key** in `PendingAuthorization`. It no longer flows into upstream token storage. `UserID` (resolved on leg 1's callback) is threaded through subsequent legs via `pending.ResolvedUserID` (current behavior) and is now the storage key. - -#### User.ID Stability Fixes - -Investigation found `User.ID` is stable for the common case but unstable in three scenarios that the rekey newly amplifies: - -1. **Chain reordering**. The first leg is hardcoded as `h.upstreams[0]`. Reordering config produces a different `User.ID` for the same human. -2. **Subsequent legs don't auto-link**. `UpdateLastAuthenticated` returns `ErrNotFound` if the `(providerID, providerSubject)` row doesn't exist; the caller only logs a warning. The provider identity for legs 2+ is never persisted. -3. **Different providers, no linking**. Alice via Google → User.ID = X; Alice via GitHub → User.ID = Y. Auto-linking is explicitly out of scope per `types.go:284-288` TODO. - -The rekey adds: - -- **Chain-reorder warning**. On startup, persist the first-leg `providerName`. If it changes between runs, log a loud warning that returning users will resolve to fresh User.IDs. -- **Subsequent-leg auto-linking**. `UpdateLastAuthenticated` upserts: if `(providerID, providerSubject)` doesn't exist, create the `ProviderIdentity` row and link it to `pending.ResolvedUserID`. This is not "account linking" in the security-sensitive sense — it happens inside an authenticated chain where consent is implicit by chain completion. -- **Identity-mismatch check runs on every callback**, not only when `len(h.upstreams) > 1`. After rekey, single-leg flows write to a globally-readable slot; the cross-check is now load-bearing in single-leg too. - -Cross-provider auto-linking remains out of scope. - -#### Multi-IDP Authorization Scoping - -Under user-keying, an attacker who logs in via a "weak" linked provider could request `(UserID, "atlassian")` and pull tokens issued by a "strong" provider. The fix: the read path filters by the provider authorized in the current AS session. Add an `auth_provider` JWT claim or scope-based gating (e.g., `upstream:atlassian` granted at auth time). Refuse cross-provider reads. - -For ToolHive's current single-provider deployments this is dormant. For vMCP multi-upstream scenarios (THV-0052) it is load-bearing. - -#### `ErrInvalidBinding` Implementation - -The binding infrastructure (`UpstreamTokens.ClientID`, `UpstreamTokens.UpstreamSubject`, the `ErrInvalidBinding` sentinel) was designed in PR #3358 as defence-in-depth and never wired up. Investigation confirms `ErrInvalidBinding` is defined but never returned by any code path. - -On `Get`, the storage layer should compare: - -- `claims["client_id"]` vs `tokens.ClientID` — catches tokens being read from the wrong client's session. - -`tokens.UserID` vs `claims["sub"]` becomes tautological after the rekey (the lookup key is the same value), so that check disappears naturally. - -This is technically separate hardening, but the rekey is the moment we reread this code anyway. ~20 LOC across two backends. - -#### Logout Semantics - -Logout terminates the AS session only. Upstream tokens persist (other devices keep working). "Sign out everywhere" becomes an explicit user action that calls `DeleteAllForUser` and best-effort revokes upstream RTs via RFC 7009. **This is a behaviour change** — today's `DeleteUpstreamTokens(sessionID)` evicts the user's tokens on logout because per-session keying made one row equal one session. - -#### Redis Schema Changes - -Existing layout: - -``` -{prefix}upstream:{sessionID}:{providerName} # primary -{prefix}upstream:idx:{sessionID} # session index set -{prefix}user:upstream:{userID} # user reverse index -``` - -New layout: - -``` -{prefix}upstream:{userID}:{providerName} # primary (was idx member) -{prefix}user:upstream:{userID} # promoted to primary index -``` - -The `user:upstream:{userID}` index already exists and is currently used only for `DeleteUser` cascade — promoting it is a small change, not a new build. - -The hash tag `{:}` is unchanged → cluster slot routing unchanged. Do **not** introduce `{userID}` as a new hash tag (would scatter different users across slots and break per-tenant batch operations). - -The new Lua script is ~60 lines (smaller than current ~80) because the old-vs-new userID compare/repair logic disappears (userID is structural, not payload). - -```lua --- Pseudocode for new storeUpstreamTokensScript (atomicity-critical) -KEYS[1] = "{prefix}upstream::" -KEYS[2] = "{prefix}user:upstream:" -ARGV[1] = newTokenJSON -ARGV[2] = ttlMs - -existing = GET KEYS[1] -new = cjson.decode(ARGV[1]) -if existing and existing != "null": - old = cjson.decode(existing) - if isEmpty(new.refresh_token) and notEmpty(old.refresh_token): - new.refresh_token = old.refresh_token -- toolhive#5047 fix B - -if ttlMs > 0: - SET KEYS[1] cjson.encode(new) PX ttlMs -else: - SET KEYS[1] cjson.encode(new) - -SADD KEYS[2] KEYS[1] -if ttlMs > 0: - if PTTL(KEYS[2]) < ttlMs: PEXPIRE KEYS[2] ttlMs -else: - PERSIST KEYS[2] -``` - -#### Migration Strategy: Big-Bang - -No dual-write, no online migration. Old keys self-evict via TTL within `max(refresh-token-TTL)` (typically minutes to hours). - -Justification: upstream tokens are bounded-lifetime caches, not durable user state. The worst-case user experience at cutover is one OAuth re-consent on the next downstream request — no worse than what users hit when an IdP revokes a refresh token (already a normal event). Dual-write trades complexity (extra Lua, dual indices, longer rollback rehearsal) for a UX outcome the system already handles gracefully. - -PR #4198 previously re-keyed Redis storage from `upstream:{sessionID}` to `upstream:{sessionID}:{providerName}` and shipped one-shot startup migration code. The precedent exists if a migration approach proves needed, but for this RFC the trade-off favours big-bang. - -## Security Considerations - -### Threat Model - -Attackers in scope: -- A user holding an old (pre-cutover) JWT. -- A user with multiple linked providers attempting to read across providers. -- A malicious or compromised IdP returning empty `refresh_token` on re-auth. -- Concurrent re-auths racing to overwrite the same user's tokens. -- An adversary attempting to enumerate or guess storage keys. - -### Authentication and Authorization - -- JWT signature validation remains the trust root. -- The lookup key changes from `tsid` (per-flow random, ~130 bits) to `User.ID` (UUIDv4, ~122 bits). Both far exceed brute-force reach. Forging requires breaking JWT signing, at which point all claims are forgeable regardless. -- The dormant `ErrInvalidBinding` check is wired up: `claims["client_id"]` vs `tokens.ClientID` becomes a hard read-side check. -- Multi-IDP scoping (see §Detailed Design): an `auth_provider` claim or scope gates which provider's tokens a session can read. - -### Data Security - -- Eliminates orphaned-token leakage class. Today, post-re-auth refresh tokens sit unreachable in storage with TTL until they age out. Post-rekey: single row per `(UserID, providerName)`, overwritten in place. -- GDPR right-to-erasure becomes one DELETE per user, deterministic. With SessionID keying erasure required scanning by `UserID` field — slower and error-prone. -- Bug B fix preserves an upstream `refresh_token` when the IdP omits it on re-auth. Failure mode: if the IdP just revoked the RT (e.g., user clicked "Revoke app" on Google), the AS keeps using a now-invalid RT until the next refresh attempt. Mitigation: on `invalid_grant` from upstream, the storage row MUST be deleted or dead-marked, forcing re-auth. - -### Input Validation - -- `providerID` (used as part of the storage key and identity lookup) is the **logical** ToolHive upstream name from config, not an upstream-asserted value. Renaming the upstream in config orphans data — documented as a known limitation. -- `providerSubject` is asserted by the upstream IdP. Trust in `(providerID, providerSubject)` uniqueness is the foundational assumption. -- `User.ID` is generated by `uuid.New()` (UUIDv4 from `crypto/rand`-backed `google/uuid`). Birthday-bound collision probability is negligible. - -### Secrets Management - -- Refresh tokens are stored in Redis (or memory). The rekey does not change the at-rest threat model — JWE encryption is out of scope. -- The merge-on-empty pattern for refresh tokens MUST happen inside the Lua script, not in Go application code, to be safe under concurrent `Store` for the same user. - -### Audit and Logging - -- `tsid` was useful as per-flow correlation in logs. Replace with a dedicated `jti` (JWT ID) claim or `flow_id` claim, logged but not used as a storage key. -- Audit log field rename: `session_id` → `user_id` in upstream-token-service log lines. Release-note item. - -### Mitigations - -| Threat | Mitigation | -|--------|------------| -| `tsid` fallback as downgrade vector | Hard switch: no fallback. JWT-version-claim-gated. Bounded JWT drainage. | -| Multi-IDP cross-provider read | `auth_provider` JWT claim or scope; storage read filtered by authorized provider. | -| Stale RT after revocation | `invalid_grant` from upstream MUST delete the row and force re-auth. | -| Concurrent `Store` race | Atomic Lua merge; mutex in memory backend. | -| Concurrent `ResolveUser` race | Atomic upsert (`INSERT ... ON CONFLICT DO NOTHING + re-read`). | - -## Alternatives Considered - -### Alternative 1: Reuse SessionID across authorize calls for the same user - -- Description: Don't mint fresh SessionID on every `/authorize`. Look up the user's existing SessionID from a side index and reuse it. -- Pros: Minimal interface change. -- Cons: User.ID isn't known at `/authorize` time. Breaks chain-leg routing (legs lookup by SessionID). Regresses multi-device (devices compete for same row). -- Why not chosen: Not viable. Contorts the auth flow to dodge a storage problem. - -### Alternative 2: Side index `(userID, providerID) → currentSessionID` - -- Description: Keep storage keyed by SessionID. Add a side index that maps stable `(userID, providerID)` to the current SessionID. Callback merges old→new at the index pointer. -- Pros: No primary-key change. JWT `tsid` continues to work. -- Cons: Lua CAS complexity for an index that should just be the primary key. Eviction-window hole (if old SessionID's row was already TTL'd, merge is a no-op and refresh token is lost). Stale rows linger until TTL. -- Why not chosen: Adds Lua atomicity surface area for the index-that-should-be-primary. Worst of both worlds. - -### Alternative 3: Half-rekey: `GetByUser` on top of session-keyed storage - -- Description: Add `GetByUser(userID, providerID)` method that reads the side index, then fetches by SessionID. Read path switches to `sub`, write path unchanged. -- Pros: Backwards compat for in-flight JWTs (still readable via `tsid` if needed). -- Cons: Once `sub` is the read key, the primary `SessionID` key is dead weight. Pays most of the rekey cost without the structural cleanup. -- Why not chosen: A half-step that would need a follow-up cleanup that "won't happen." - -### Alternative 4: Callback-only merge (Bug B fix without rekey) - -- Description: Apply the refresher's preserve-RT-on-empty pattern to the callback. Don't rekey. -- Pros: Tiny change. -- Cons: Doesn't fix root cause A. Old tokens still orphan on every re-auth. -- Why not chosen: Solves the visible Google symptom without solving the structural problem. - -### Alternative 5: OIDC `id_token_hint` carrying old tsid - -- Description: Client includes `id_token_hint` on `/authorize`. Server validates and reuses old SessionID. -- Pros: Standards-aligned mechanism. -- Cons: Most callers won't send `id_token_hint`. Hostile clients could collide sessions. Only optimisation on top of a real solution. -- Why not chosen: Not standalone. - -### Alternative 6: Explicit session merge - -- Description: Like Alternative 4 but with audit logging and explicit merge semantics. -- Pros: Cleaner code review story. -- Cons: Same shortcomings as Alt 4 — write-side only, doesn't fix Bug A. -- Why not chosen: Same. - -## Compatibility - -### Backward Compatibility - -- **JWT format**: `sub` claim was always populated. Old JWTs containing only `tsid` (issued pre-cutover) will not resolve under new code → re-auth required. Bound the in-flight window via short access-token lifespan or signing-key rotation. -- **Storage data**: old `(SessionID, providerName)` keys are not migrated. They self-evict via TTL within `max(refresh-token-TTL)`. Worst case at cutover: every active user re-auths once on their next downstream request. -- **External consumers of `tsid`**: Investigation found only three production usages — `pkg/auth/token.go:1190` (read path, switching), `pkg/authserver/server/session/session.go:120-123` (write site, stopping), `pkg/auth/context.go:99` (filter, harmless to leave). No external operators or sidecars should depend on the claim. Verify before PR 7. -- **Audit log field name** changes from `session_id` to `user_id` in upstream-token-service log lines. Release-note item; dashboards greppping for `session_id=` need updating. -- **Logout semantics change** (AS session only, upstream tokens persist) — release-note item; behaviour change. - -### Forward Compatibility - -- Account linking across providers is enabled by the data model but not the workflow. A future RFC can build on `User.ID` as the unified identity without further storage changes. -- Multi-client per user (`(userID, clientID, providerName)` keying) is a clean follow-up if scenarios warrant — additive. -- ID-JAG / token exchange flows (memory: XAA PoC) get `User.ID` as the obvious identity anchor instead of session-scoped state. - -## Implementation Plan - -### Phase 0: Ship `upstream-expiring-issues` standalone (current branch) - -Per PR-creation rules, the existing branch ships as its own focused PR for toolhive#5046. Independent of this RFC. - -### Phase 1: Storage interface refactor - -- Rename `sessionID` → `userID` parameter throughout `UpstreamTokenStorage`. -- Rename `SessionExpiresAt` → `LastAccessedAt`. -- Add `DeleteAllForUser`. -- Add atomic merge semantics to `Store` (Lua script for Redis, mutex-guarded RMW for memory). -- Backends keep current Redis keys internally — no behaviour change yet. -- ~250 LOC. Reversible. - -### Phase 2: Big-bang cutover - -- Switch read path to `claims["sub"]`. -- Switch write path to `userID`. -- New Lua script. -- Drop `upstream:idx:{sessionID}` set; promote `user:upstream:{userID}` to primary index. -- Multi-IDP scoping enforced. -- Identity-mismatch check runs single-leg too. -- ~200 LOC. Medium risk. - -### Phase 3: User.ID stability fixes - -- Subsequent-leg auto-linking (`UpdateLastAuthenticated` upserts). -- Chain-reorder startup warning. -- Document config rename / `sub` rotation as known limitations. -- ~80 LOC. - -### Phase 4: Logout semantics change - -- Stop deleting upstream tokens on AS session logout. -- Add admin "Delete all tokens for user" surface. -- Release note. -- ~50 LOC. - -### Phase 5: Concurrency hardening (independent) - -- Atomic `ResolveUser`: `INSERT ... ON CONFLICT DO NOTHING + re-read`. -- Regression test for concurrent identical callbacks. -- Can land in parallel with Phase 1. - -### Phase 6: `ErrInvalidBinding` implementation - -- `claims["client_id"]` vs `tokens.ClientID` check on `Get`. -- ~20 LOC across two backends. - -### Phase 7: Drop `tsid` (one release after Phase 2) - -- Stop populating in JWT. -- Remove constant and `internalClaims` filter entry. -- ~30 LOC. - -### Dependencies - -- Phase 1 must land first. Phases 2–4 serialize. Phase 5 is independent. Phase 6 is independent of the rekey but logically grouped. Phase 7 lands one release after Phase 2. - -## Testing Strategy - -### Unit Tests - -- `Store` merge matrix: fresh has RT × existing has RT, fresh has RT × existing empty, fresh empty × existing RT, fresh empty × existing empty, no existing record. -- Idle-TTL eviction: store, advance clock, verify eviction; sliding-window keep-alive on `Get`. -- `DeleteAllForUser` removes all of a user's records, none of another user's. -- Concurrent `Store` for same `(UserID, providerName)`: refresh-token preservation invariant under contention. -- Concurrent `ResolveUser` for same `(providerID, providerSubject)`: exactly one User row created. -- Non-expiring token (`ExpiresAt.IsZero()`) retained until idle timeout (regression for existing #5046 fix). - -### Integration Tests - -- The toolhive#5047 regression: same client re-runs full authorize flow; tokens remain retrievable. -- Two distinct users, same provider — no cross-contamination. -- Same user, parallel chains — concurrent Store + per-chain Delete. -- Refresh-token-rotation cross-flow. -- Logout on device A leaves device B's access intact. -- Multi-IDP scoping: JWT auth'd via provider X cannot read `(UserID, providerY)`. -- Chain reorder produces different User.IDs (regression for known limitation). -- JWT with only `tsid` (issued pre-cutover, post-cutover validation) fails closed. - -### Real-Redis Integration - -- Sentinel failover during write: no torn writes, no lost user-index entries. -- Cluster slot routing: hash tag `{:}` keeps everything in one slot; Lua script does not raise `CROSSSLOT`. -- Post-migration coexistence: pre-seed old-layout keys, verify reads return `ErrNotFound` and writes go to new layout. - -## Documentation - -- `docs/middleware.md` lines 169-209: replace "extracts `tsid`" with "extracts `sub`". -- `docs/arch/11-auth-server-storage.md`: describe the rekey, the chain-correlation vs storage-key separation, idle-TTL eviction. -- `pkg/authserver/server/doc.go:76` and `pkg/authserver/storage/doc.go:113`: update claim references. -- Operator advisory comments at `cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go:358` and `cmd/thv-operator/controllers/virtualmcpserver_controller.go:507`: drop `tsid` from claim-namespace warning. -- Release notes: logout semantics change, `tsid` removal, audit log field rename. - -## Open Questions - -1. **Chain-reorder handling**: warning-only (cheap) or explicit `primaryIdentityProvider` config field (cleaner)? Current proposal: warning-only; promote to explicit field if reorder scenarios prove common. -2. **Composite key dimensions**: `(userID, providerName)` is the simple proposal. Other dimensions that could legitimately differentiate stored tokens for the same user+provider: - - **`clientID`**: same user logging in from multiple OAuth clients. Each gets its own session today; under the rekey they share a row. - - **Scope set**: re-auth with broader scopes typically issues a token covering the union, so overwriting is usually correct — but providers that don't combine scopes would silently lose access. - - **Audience / RFC 8707 `resource`**: if ToolHive ever passes `resource` upstream, tokens with different audiences should not share a row. - - Oathkeeper's `MutatorIDToken` cache key (`pipeline/mutate/mutator_id_token.go:82`) hashes five dimensions (`IssuerURL | TTL | JWKSURL | claims | subject`) precisely to avoid this class of leak. Current proposal: `(userID, providerName)` only, with all three additional dimensions deferred until concrete evidence requires them. -3. **Idle TTL default value and config surface**: 24h matches the current `SessionExpiresAt` cap, but it's now a separate knob. Default and config surface (env var? CRD field?) need product input. -4. **Does anything outside this repo read `tsid`?** External operators, sidecars, observability — verify before Phase 7. -5. **`UpstreamTokenRefresher.RefreshAndStore(sessionID, ...)` interface**: signature must change to `userID`. Confirm no external implementations exist outside `pkg/authserver`. -6. **Cluster mode usage**: codebase uses `redis.UniversalClient` but configured `redis.NewFailoverClient` (Sentinel). Verify whether any tenant runs against Cluster mode; if not, hash-tag analysis is precautionary. - -## References - -- toolhive#5046 — fabricated 1h expiry on non-expiring upstream tokens -- toolhive#5047 — orphaned upstream refresh tokens on re-authorization -- THV-0019 — original auth server design (defines session-scoped storage model) -- THV-0031 — auth server integration -- THV-0035 — auth server Redis storage -- THV-0052 — multi-upstream IDP authserver -- Hydra reference: `flow/consent_types.go`, `persistence/sql/persister_consent.go` — subject-keyed indexes pattern; `LoginSession` UPSERT-by-cookie-id at `persister_consent.go:217-228`; `FindGrantedAndRememberedConsentRequest` keyed by `(subject, client_id, nid)` at `persister_consent.go:286-331`. -- Oathkeeper reference: `pipeline/mutate/mutator_id_token.go:82` — cache key hashes `IssuerURL | TTL | JWKSURL | claims | session.Subject`, the closest structural analog to our problem; `pipeline/authn/authenticator_oauth2_client_credentials.go:180-194` — `TTL = min(configuredTTL, time.Until(token.Expiry))` pattern. Both Ory products independently picked subject-stable keying for caches that must survive across requests for the same user. -- Prior commits: `86ebd0f49` (`tsid` introduction), `2613d6e7f` (JWT binding security rationale), `bbaee66bf`/PR #3358 (`UserID` field added as metadata), `f52b6d075`/PR #4198 (Redis migration precedent), `e73512271`/PR #4283 (multi-leg chain SessionID threading) - ---- - -## RFC Lifecycle - -### Review History - -| Date | Reviewer | Decision | Notes | -|------|----------|----------|-------| -| 2026-04-28 | @jhrozek | Draft | Initial submission, not yet pushed | - -### Implementation Tracking - -| Repository | PR | Status | -|------------|-----|--------| -| toolhive | (TBD Phase 0 — `upstream-expiring-issues`) | In review | -| toolhive | (TBD Phase 1) | Not started | diff --git a/rfcs/THV-XXXX-upstream-token-refresh.md b/rfcs/THV-XXXX-upstream-token-refresh.md deleted file mode 100644 index 68570bd..0000000 --- a/rfcs/THV-XXXX-upstream-token-refresh.md +++ /dev/null @@ -1,639 +0,0 @@ -# RFC-XXXX: Upstream Token Refresh for Auth Server - -- **Status**: Draft -- **Author(s)**: @tgrunnagle -- **Created**: 2026-02-20 -- **Last Updated**: 2026-02-20 -- **Target Repository**: toolhive -- **Related Issues**: TBD - -## Summary - -Add transparent upstream token refresh to the embedded authorization server. When a stored upstream access token expires, the auth server refreshes it via the upstream IdP before returning it to the middleware, extending session lifetime beyond the upstream AT's ~1-hour window. When upstream refresh permanently fails, the service revokes the internal session to force a clean re-authentication. - -## Problem Statement - -Upstream refresh tokens are stored and `RefreshTokens()` is fully implemented on both OAuth2 and OIDC providers, but no code calls it yet. The upstream swap middleware (`pkg/auth/upstreamswap/middleware.go`) detects expiry but logs a warning and passes the expired token through. This means sessions are effectively limited to the upstream AT lifetime (~1 hour for most IdPs). - -To support long-lived sessions, the auth server needs to use stored upstream refresh tokens to obtain fresh access tokens transparently. Additionally, when an upstream refresh token is permanently invalidated by the IdP, the auth server should revoke the internal session so the client is directed into a clean re-authentication rather than retrying with stale credentials. - -### Current Token Flow - -``` -Client -> GET /oauth/authorize -> Internal AS -> 302 to upstream IdP - | -Client <- redirect with auth code <- Internal AS <- GET /oauth/callback (code) - | - ExchangeCode -> upstream AT, RT, IDT stored (keyed by sessionID) - Issue internal JWT (with tsid=sessionID claim) - Issue internal opaque refresh token - | -Client -> proxied request -> upstream swap middleware - | - Read upstream tokens by tsid from storage - Replace Authorization header with upstream AT - | - Forward to backend -``` - -### Current Token Lifetimes - -| Token | Default | Configurable Range | -|-------|---------|-------------------| -| Internal access token (JWT) | 1h | 1min-24h | -| Internal refresh token (opaque) | 7d | 1h-30d | -| Auth code | 10min | 30s-10min | -| Upstream AT storage TTL | upstream `ExpiresAt` (or 1h fallback) | - | -| Storage cleanup interval | 5min | - | -| Expiry detection buffer | 30s | - | - -### Opportunities - -1. **Upstream refresh tokens stored but never used** - `RefreshTokens()` is fully implemented (`pkg/authserver/upstream/oauth2.go`, `oidc.go`) but no code calls it yet -2. **Storage TTL tied to AT expiry** - the upstream token entry expires when the AT expires, which garbage-collects the refresh token even though it may be valid for 30+ days -3. **No session revocation on upstream failure** - when upstream tokens expire, the internal session (7-day RT) continues without a mechanism to direct the client into re-authentication -4. **Direct storage access from middleware** - the middleware calls raw `storage.UpstreamTokenStorage` directly, which is an implementation detail of the AS, not a proper service boundary - -## Goals - -- Transparently refresh expired upstream access tokens on read, so users never see a 401 due to upstream AT expiry -- Revoke the internal session when upstream refresh permanently fails, forcing the client into a clean re-authentication -- Introduce a service interface (`upstreamtoken.Service`) that encapsulates all upstream token logic behind a clean API boundary -- Decouple storage TTL from upstream AT expiry so refresh tokens are preserved across AT renewal cycles -- Deduplicate concurrent refresh attempts for the same session using singleflight -- Prepare the architecture for out-of-process AS extraction (the service interface becomes a single RPC) - -## Non-Goals - -- **Background refresh loop** - deferred to a future phase; can be added as an internal optimization without changing the `Service` interface -- **Persisting refresh failure state** - the service handles errors transparently and revokes the session on permanent failure; no persistent failure flag is needed -- **Multi-provider refresh** - the current system supports a single upstream provider per auth server instance -- **Redis storage implementation** - this RFC only modifies the in-memory storage; Redis implementation (THV-0035) will implement the same interface methods -- **Upstream token encryption at rest** - tokens remain in-memory with short TTLs - -## Proposed Solution - -### High-Level Design - -Introduce an `upstreamtoken.Service` interface that the middleware calls instead of raw storage. The service transparently refreshes expired tokens on read, deduplicates concurrent refreshes via singleflight, and revokes the internal session when upstream refresh permanently fails. - -```mermaid -flowchart TB - subgraph "Proxy Runner" - MW["Upstream Swap
Middleware"] - SVC["upstreamtoken.Service
(GetValidTokens)"] - SF["singleflight.Group"] - STOR["UpstreamTokenStorage"] - PROV["OAuth2Provider
(RefreshTokens)"] - REV["SessionRevoker
(RevokeSessionByTSID)"] - - MW -->|"GetValidTokens(ctx, tsid)"| SVC - SVC -->|"1. Read tokens"| STOR - SVC -->|"2. Refresh if expired"| SF - SF -->|"One IdP call"| PROV - SVC -->|"3. Revoke on permanent failure"| REV - SVC -->|"4. Update tokens"| STOR - end - - IDP["Upstream IdP"] - PROV -->|"RefreshTokens()"| IDP -``` - -### Detailed Design - -#### Design Decision: Service Interface with On-Read Refresh - -The middleware currently calls `storage.UpstreamTokenStorage.GetUpstreamTokens()` directly. This is the wrong abstraction: storage is an implementation detail of the AS. The middleware should call a service-level interface that represents the business operation: "give me valid upstream tokens for this session." - -Three approaches were evaluated: proactive background refresh, lazy on-demand refresh in the middleware, and on-read refresh behind a service interface. - -**On-read refresh behind the service interface wins** because: - -- **Architecture boundary**: The middleware calls a service, not raw storage. The service owns all IdP interaction. When the AS goes out-of-process, `GetValidTokens()` becomes a single RPC. The middleware code does not change. -- **Simplicity**: No background goroutines, no `Start`/`Stop` lifecycle, no `ListUpstreamTokenSessions()` method. The service is just storage + provider + singleflight. -- **Correctness without optimization**: Every request gets valid tokens or a clear error. No edge cases around idle sessions that the background loop missed. -- **Minimal interface**: One method (`GetValidTokens`) returning an opaque `UpstreamCredential`. Clean for in-process and network use. - -**The latency cost is acceptable**: On-read refresh adds ~200ms (one IdP round-trip) to the first request after AT expiry, once per hour per session. MCP tool calls typically take 500ms-2s. The 200ms is imperceptible. - -#### Service Interface - -**Package**: `pkg/authserver/upstreamtoken/` - -```go -package upstreamtoken - -type Service interface { - // GetValidTokens returns a valid upstream credential for the given session. - // If stored tokens are expired and a refresh token is available, the - // implementation transparently refreshes them before returning. - // - // If refresh permanently fails (e.g. refresh token revoked), the - // implementation revokes the internal session to force re-authentication. - GetValidTokens(ctx context.Context, sessionID string) (UpstreamCredential, error) -} - -// UpstreamCredential is an opaque type representing a valid upstream token. -// The middleware does not need to know internal structure - it only needs -// a bearer token string to inject into upstream requests. -type UpstreamCredential struct { - accessToken string -} - -// NewUpstreamCredential creates an UpstreamCredential. Only the service -// package should call this. -func NewUpstreamCredential(accessToken string) UpstreamCredential { - return UpstreamCredential{accessToken: accessToken} -} - -// BearerToken returns the access token string for injection into HTTP headers. -func (c UpstreamCredential) BearerToken() string { - return c.accessToken -} - -var ( - ErrSessionNotFound = errors.New("upstreamtoken: session not found") - ErrRefreshFailed = errors.New("upstreamtoken: refresh failed") - ErrNoRefreshToken = errors.New("upstreamtoken: no refresh token") -) -``` - -#### In-Process Implementation - -```go -type inProcessService struct { - storage storage.UpstreamTokenStorage - provider upstream.OAuth2Provider - revoker SessionRevoker - group singleflight.Group -} -``` - -Three dependencies: -- `storage.UpstreamTokenStorage` - read/write upstream token entries -- `upstream.OAuth2Provider` - call IdP to refresh tokens -- `SessionRevoker` - revoke internal session when upstream refresh permanently fails - -#### GetValidTokens Flow - -```mermaid -flowchart TD - A["GetUpstreamTokens(sessionID)"] -->|"not found"| B["Return ErrSessionNotFound"] - A -->|"found"| C{"AT expired?"} - C -->|"no"| D["Return UpstreamCredential(AT)"] - C -->|"yes"| E{"Has RT?"} - E -->|"no"| F["Return ErrNoRefreshToken"] - E -->|"yes"| G["singleflight.Do"] - G --> H["Double-check: re-read from storage"] - H -->|"already refreshed"| D - H -->|"still expired"| I["provider.RefreshTokens()"] - I -->|"success"| J["UpdateUpstreamTokens
(reset TTL)"] - J --> D - I -->|"invalid_grant"| K["RevokeSessionByTSID
DeleteUpstreamTokens"] - K --> L["Return ErrRefreshFailed (401)"] - I -->|"transient error"| M["Return ErrRefreshFailed (502)
Session NOT revoked"] -``` - -The singleflight group ensures that concurrent requests for the same expired session result in a single IdP refresh call. The double-check after acquiring the singleflight lock avoids redundant refreshes when another goroutine has already refreshed the tokens. - -When storing refreshed tokens, refresh token rotation is handled via coalesce: if the IdP returns a new refresh token, it is stored; otherwise the existing one is preserved. - -#### Session Revocation on Permanent Failure - -When upstream refresh fails with `invalid_grant` (RT revoked, expired, or invalidated by the IdP), the service must break the internal session to prevent the user from being stuck in a loop. Without this, the client would: - -1. Get 401 from the middleware -2. Use its internal refresh token to get a new internal JWT -3. New JWT has the same `tsid` (fosite preserves session claims on refresh) -4. Next request hits the same dead upstream tokens -5. Infinite loop - user is stuck - -The fix: `RevokeSessionByTSID(tsid)` deletes all internal access tokens and refresh tokens where `session.UpstreamSessionID == tsid`. The next time the client tries to use its internal refresh token, fosite returns `invalid_grant`, which is the standard signal that forces the client into a full re-authentication (new OAuth flow, new upstream tokens, new `tsid`). - -#### Session Revocation Interface - -```go -// SessionRevoker revokes all internal tokens associated with a session. -// The UpstreamTokenService calls this when upstream refresh permanently fails, -// to force the client into re-authentication. -type SessionRevoker interface { - RevokeSessionByTSID(ctx context.Context, tsid string) error -} -``` - -Implementation on `MemoryStorage`: - -```go -func (s *MemoryStorage) RevokeSessionByTSID(ctx context.Context, tsid string) error { - s.mu.Lock() - defer s.mu.Unlock() - - // Delete all refresh tokens for this session - for sig, entry := range s.refreshTokens { - session, ok := entry.value.GetSession().(*session.Session) - if ok && session.UpstreamSessionID == tsid { - delete(s.refreshTokens, sig) - } - } - - // Delete all access tokens for this session - for sig, entry := range s.accessTokens { - session, ok := entry.value.GetSession().(*session.Session) - if ok && session.UpstreamSessionID == tsid { - delete(s.accessTokens, sig) - } - } - - return nil -} -``` - -This follows the existing pattern. `RevokeRefreshToken(requestID)` and `RevokeAccessToken(requestID)` already do O(n) scans over the same maps. The service never touches fosite internals (signatures, request IDs). It only knows about `tsid`. - -#### Storage Layer Changes - -**New method on `UpstreamTokenStorage`**: - -```go -// UpdateUpstreamTokens atomically updates token values for an existing session -// and resets the sliding-window TTL (2h inactivity timeout). Used by refresh logic. -// Since there is no background refresh loop, every call to UpdateUpstreamTokens -// is triggered by a user request, so resetting the TTL here is correct. -UpdateUpstreamTokens(ctx context.Context, sessionID string, tokens *UpstreamTokens) error -``` - -**Behavioral change to `StoreUpstreamTokens`**: - -This is a change to an existing method's behavior, not just a new method: - -``` -BEFORE: expiresAt = tokens.ExpiresAt (AT expiry, ~1h) - -AFTER: if tokens.RefreshToken != "" -> expiresAt = now + DefaultUpstreamInactivityTimeout (2h) - if tokens.RefreshToken == "" -> expiresAt = tokens.ExpiresAt (unchanged) -``` - -New constant: `DefaultUpstreamInactivityTimeout = 2 * time.Hour` - -**Why 2 hours?** -- Long enough that a briefly idle user (lunch, meeting) does not lose their session -- Short enough that abandoned sessions are reaped quickly (not 7 days) -- TTL is reset by `StoreUpstreamTokens` (initial auth) and `UpdateUpstreamTokens` (refresh). Since there is no background loop, every write is user-triggered, so resetting TTL on write correctly tracks user activity -- Without a background refresh loop, there are zero unnecessary IdP refreshes for abandoned sessions: the entry simply expires and gets reaped - -No `LastAccessedAt` field is needed. The `timedEntry.expiresAt` sliding window (reset by `UpdateUpstreamTokens`) already tracks user activity. No `RefreshFailed` flag is needed. The service handles errors transparently and returns them to the caller. - -#### Error Mapping in Middleware - -| Service Error | HTTP | When | -|---|---|---| -| `ErrSessionNotFound` | 401 | No upstream tokens for session | -| `ErrNoRefreshToken` | 401 | AT expired, no RT to refresh with | -| `ErrRefreshFailed` + `invalid_grant` | 401 | RT permanently dead (internal session also revoked) | -| `ErrRefreshFailed` + transient | 502 | IdP temporarily down | - -#### Wiring Chain - -The `EmbeddedAuthServer` constructs the `upstream.OAuth2Provider` directly (it already builds the upstream config), keeps a reference, and passes it to both the Server and the Service. The `authserver.Server` interface does not change. - -```go -// In NewEmbeddedAuthServer: -provider := upstream.NewOAuth2Provider(upstreamConfig) -server := authserver.New(provider, ...) - -tokenService := upstreamtoken.NewInProcessService( - server.IDPTokenStorage(), // UpstreamTokenStorage - provider, // OAuth2Provider for refresh calls - server.IDPTokenStorage(), // SessionRevoker (MemoryStorage implements both) -) -``` - -**`MiddlewareRunner` interface change**: - -```go -// BEFORE: -GetUpstreamTokenStorage() func() storage.UpstreamTokenStorage - -// AFTER: -GetUpstreamTokenService() func() upstreamtoken.Service -``` - -**Middleware change**: - -```go -// BEFORE: -stor := storageGetter() -tokens, err := stor.GetUpstreamTokens(ctx, tsid) -if tokens.IsExpired(time.Now()) { - logger.Warn("upstreamswap: upstream tokens expired") - // Continue with expired token -} -injectToken(r, tokens.AccessToken) - -// AFTER: -svc := serviceGetter() -cred, err := svc.GetValidTokens(r.Context(), tsid) -if err != nil { - // map error to 401 or 502 - return -} -injectToken(r, cred.BearerToken()) -``` - -### Token Lifecycle Scenarios - -#### A. Active user (continuous requests) - -``` -T+0:00 Auth. AT stored (exp T+1:00). Entry TTL = T+2:00. -T+0:30 Request. GetValidTokens -> AT valid. TTL unchanged (T+2:00). -T+1:00 Request. AT expired. GetValidTokens refreshes via IdP (~200ms). - UpdateUpstreamTokens stores new AT (exp T+2:00), resets TTL -> T+3:00. -T+1:30 Request. AT valid. TTL unchanged (T+3:00). -T+2:00 Request. AT expired. Refresh again. TTL resets -> T+4:00. -... continues indefinitely. -``` - -#### B. User idle 30 min, returns - -``` -T+0:00 Last request. TTL = T+2:00. -T+0:30 User returns. AT still valid (expires T+1:00). TTL unchanged (T+2:00). -``` -Result: Seamless. No refresh needed. - -#### C. User idle 90 min, returns - -``` -T+0:00 Last request. TTL = T+2:00. -T+1:30 User returns. AT expired (expired at T+1:00). TTL still alive (T+2:00). - GetValidTokens refreshes via IdP (~200ms). UpdateUpstreamTokens resets TTL -> T+3:30. -``` -Result: 200ms latency on first request. Transparent to user. - -#### D. User idle 2+ hours, returns - -``` -T+0:00 Last request. TTL = T+2:00. -T+2:00 Cleanup loop reaps entry. -T+2:15 User returns. GetValidTokens -> ErrSessionNotFound -> 401. Re-authenticate. -``` -Result: Must re-authenticate. Expected for 2h+ idle. - -#### E. Abandoned session - -``` -T+0:00 Last request. TTL = T+2:00. -T+2:00 Entry reaped. Zero unnecessary IdP calls. -``` - -#### F. Refresh token revoked by IdP - -``` -T+1:00 User request. AT expired. GetValidTokens attempts refresh. - IdP returns invalid_grant. - Service calls revoker.RevokeSessionByTSID(tsid) -> deletes internal AT + RT. - Service calls storage.DeleteUpstreamTokens(tsid) -> deletes upstream entry. - Returns ErrRefreshFailed -> middleware returns 401. -T+1:01 Client tries internal refresh token -> fosite returns invalid_grant. - Client forced into full re-authentication. - New OAuth flow -> new upstream tokens -> new tsid -> user is recovered. -``` -Result: Clean recovery. User re-authenticates and gets a fresh session. - -#### G. IdP temporarily down - -``` -T+1:00 User request. AT expired. GetValidTokens attempts refresh. - IdP returns connection error. - Return ErrRefreshFailed wrapping transient error -> middleware returns 502. - Internal session NOT revoked (transient, not permanent). - Client retries. -T+1:01 Client retries. GetValidTokens attempts refresh again. - IdP is back -> refresh succeeds. New AT stored. Request proceeds. -``` -Result: Brief 502, then recovery on retry. - -#### H. Concurrent requests for same expired session - -``` -T+1:00 5 requests arrive. All call GetValidTokens(sessionID). - singleflight deduplicates: 1 actual refresh call to IdP. - All 5 callers block, all get same result. -``` - -#### I. IdP rotates refresh token - -``` -T+1:00 Refresh returns new AT + new RT. UpdateUpstreamTokens stores both. - Old RT replaced atomically. coalesce() keeps old RT if IdP doesn't rotate. -``` - -### Future: Out-of-Process AS - -When the AS moves to a separate service: - -**HTTP endpoint** (on the AS): -``` -POST /internal/upstream-tokens/{sessionID} - -200: { "access_token": "..." } -404: { "error": "session_not_found" } -401: { "error": "refresh_failed" } -422: { "error": "no_refresh_token" } -``` - -**Network client**: Implements `upstreamtoken.Service` by making HTTP calls to the AS. `GetValidTokens()` becomes a single HTTP request. Error types are serializable. - -**Middleware code does not change** - same interface, different implementation behind it. - -**Background refresh loop** (Phase 2 optimization): When added, it lives inside the AS process as an internal optimization of the concrete `Service` implementation. It can be added without changing the `Service` interface. - -## Security Considerations - -### Threat Model - -| Threat | Description | Likelihood | Impact | -|--------|-------------|------------|--------| -| Stolen upstream refresh token | Attacker obtains RT from memory and uses it to get new ATs | Low (requires process memory access) | Access to upstream resources for one user | -| IdP refresh token replay | Attacker replays an old RT after rotation | Low | Blocked by IdP RT rotation policies | -| Stale session exploitation | Attacker uses internal session after upstream RT revocation | Medium (before this fix) | Currently possible; this RFC eliminates it | - -### Authentication and Authorization - -- No changes to authentication flows. The service operates within an already-authenticated session. -- The service validates that a session exists before attempting refresh. It does not create new sessions. -- Session revocation on permanent failure is an authorization enforcement: if the upstream IdP revokes access, the internal session is terminated. - -### Data Security - -- Upstream access tokens and refresh tokens remain in-memory only, matching the current storage model -- Refreshed tokens overwrite the previous values atomically; no history is retained -- The `UpstreamCredential` type uses an unexported field (`accessToken`) to prevent accidental logging or serialization - -### Input Validation - -- `sessionID` is an internal identifier (UUID) generated during the OAuth flow; it is not user-supplied -- The service does not accept external input beyond the session ID -- Upstream IdP responses are validated by the existing `upstream.OAuth2Provider` implementation - -### Secrets Management - -- No new secrets are introduced -- Upstream refresh tokens are already stored in the existing `UpstreamTokenStorage` -- The 2-hour inactivity TTL ensures tokens are reaped more aggressively than before (previously tied to AT expiry only) - -### Audit and Logging - -- Token refresh attempts logged at INFO level (session ID, provider, success/failure) -- Permanent refresh failures (invalid_grant) logged at WARN level with session revocation noted -- Transient refresh failures logged at WARN level -- Token values are never logged - -### Mitigations - -| Threat | Mitigation | -|--------|------------| -| Stolen upstream RT | In-memory storage with 2h inactivity TTL; tokens reaped when session idle | -| Stale session loop | Session revocation on `invalid_grant`; internal RT deleted, forcing re-auth | -| IdP outage causing cascading failures | Transient errors return 502 without revoking session; client retries naturally | -| Concurrent refresh thundering herd | singleflight deduplication; one IdP call per session per expiry window | -| RT replay after rotation | `coalesce()` stores new RT when IdP rotates; old RT is replaced atomically | - -## Alternatives Considered - -### Alternative 1: Background Refresh Loop - -- **Description**: A background goroutine periodically scans all sessions and refreshes tokens before they expire -- **Pros**: Zero latency on requests (tokens always pre-refreshed), IdP outage resilience (grace period with pre-refreshed tokens) -- **Cons**: Requires `Start`/`Stop` lifecycle, `ListUpstreamTokenSessions()` method, background goroutine management, and refreshes tokens for abandoned sessions (unnecessary IdP calls) -- **Why not chosen**: Adds significant complexity for minimal benefit. The ~200ms on-read refresh cost (once per hour per session) is imperceptible given MCP tool call latencies of 500ms-2s. Can be added as a Phase 2 optimization without changing the `Service` interface. - -### Alternative 2: Lazy Refresh in Middleware - -- **Description**: Add refresh logic directly in the upstream swap middleware -- **Pros**: Simpler, fewer files changed -- **Cons**: The middleware would need to know about IdP providers, storage internals, and session revocation. This violates the architecture boundary: storage and IdP interaction are AS concerns, not middleware concerns. When the AS goes out-of-process, the middleware would need a major rewrite. -- **Why not chosen**: Wrong abstraction layer. The service interface provides a clean boundary that works for both in-process and future out-of-process AS deployments. - -### Alternative 3: No Service Interface (Direct Storage Enhancement) - -- **Description**: Add refresh logic to the storage layer itself (e.g., auto-refresh on `GetUpstreamTokens`) -- **Pros**: Minimal API change -- **Cons**: Storage should not call IdP providers. It would couple the storage layer to network calls, making testing difficult and violating the single-responsibility principle. -- **Why not chosen**: Storage is a persistence concern; token refresh is a business logic concern. Mixing them creates an untestable, tightly-coupled design. - -## Compatibility - -### Backward Compatibility - -- **`MiddlewareRunner` interface change**: `GetUpstreamTokenStorage()` is renamed to `GetUpstreamTokenService()`. This is an internal interface; no external consumers are affected. -- **`StoreUpstreamTokens` TTL change**: Sessions with refresh tokens now have a 2-hour sliding-window TTL instead of AT-expiry TTL. This is strictly better behavior (tokens live longer, not shorter). Sessions without refresh tokens are unchanged. -- **No client-facing changes**: The middleware still injects a bearer token into upstream requests. Clients see no API changes. - -### Forward Compatibility - -- The `upstreamtoken.Service` interface is designed for out-of-process AS extraction. `GetValidTokens()` maps directly to a single HTTP endpoint. -- Background refresh can be added to the concrete implementation without changing the interface. -- Redis storage (THV-0035) will implement `UpdateUpstreamTokens` and `RevokeSessionByTSID` using the same interface contracts. - -## Implementation Plan - -### Phase 1: Storage Layer Extensions - -**Files**: -- `pkg/authserver/storage/types.go` - Add `UpdateUpstreamTokens` to `UpstreamTokenStorage` interface. Add `SessionRevoker` interface. -- `pkg/authserver/storage/memory.go` - Implement new methods including `RevokeSessionByTSID`. Change `StoreUpstreamTokens` TTL logic (RT present -> 2h sliding window). -- `pkg/authserver/storage/config.go` - Add `DefaultUpstreamInactivityTimeout = 2h` -- `pkg/authserver/storage/mocks/` - Regenerate - -**Tests**: New tests for `UpdateUpstreamTokens` (including TTL reset behavior), `RevokeSessionByTSID`, and the `StoreUpstreamTokens` TTL behavior change. - -### Phase 2: Service Package - -**Files**: -- `pkg/authserver/upstreamtoken/service.go` - `Service` interface, `UpstreamCredential`, sentinel errors -- `pkg/authserver/upstreamtoken/inprocess.go` - `inProcessService` with `GetValidTokens` (on-read refresh, singleflight, session revocation on permanent failure) -- `pkg/authserver/upstreamtoken/inprocess_test.go` - Unit tests -- `pkg/authserver/upstreamtoken/mocks/` - Generated mock for `Service` - -**Tests**: -- Happy path: valid tokens returned as UpstreamCredential -- Expired AT with RT: successful refresh, tokens updated -- Expired AT without RT: `ErrNoRefreshToken` -- Refresh failure (invalid_grant): `ErrRefreshFailed`, internal session revoked, upstream tokens deleted -- Refresh failure (transient): `ErrRefreshFailed`, internal session NOT revoked -- Singleflight dedup: concurrent calls, only one refresh -- RT rotation: new RT stored when IdP rotates - -### Phase 3: Wiring Chain - -**Files**: -- `pkg/authserver/runner/embeddedauthserver.go` - Construct provider + service, add `UpstreamTokenService()` accessor -- `pkg/transport/types/transport.go` - `GetUpstreamTokenStorage()` -> `GetUpstreamTokenService()` -- `pkg/runner/runner.go` - Updated implementation -- `pkg/transport/types/mocks/` - Regenerate - -### Phase 4: Middleware Rewrite - -**Files**: -- `pkg/auth/upstreamswap/middleware.go` - Replace `StorageGetter` with `ServiceGetter`, simplify to `GetValidTokens()` + error mapping -- `pkg/auth/upstreamswap/middleware_test.go` - Rewrite tests with mock `upstreamtoken.Service` - -### Phase 5: Cleanup and Verification - -- Remove dead imports and unused code -- Full test suite (unit + e2e) -- Lint, generate, build - -Each phase leaves the codebase compilable. Phases 3+4 should ship together if in separate PRs. - -### Dependencies - -- No new external dependencies. Uses the standard library's `golang.org/x/sync/singleflight`. -- Depends on existing `pkg/authserver/upstream/` (already implements `RefreshTokens()`) -- Depends on existing `pkg/authserver/storage/` (extended, not replaced) - -## Testing Strategy - -- **Unit tests**: Service implementation with mocked storage, provider, and revoker. Cover all error paths, singleflight deduplication, and RT rotation. -- **Unit tests**: Storage layer extensions (`UpdateUpstreamTokens`, `RevokeSessionByTSID`, TTL behavior change) -- **Unit tests**: Middleware error mapping (service errors to HTTP status codes) -- **Integration tests**: Full refresh flow with real storage and mocked IdP -- **End-to-end tests**: OAuth flow through token expiry, refresh, and re-authentication -- **Concurrency tests**: Verify singleflight deduplication under concurrent load - -## Documentation - -- Update `docs/arch/` in toolhive with upstream token refresh design -- GoDoc for new `pkg/authserver/upstreamtoken/` package -- Update operational guides with token lifecycle behavior changes - -## Open Questions - -None. All design questions resolved. - -## References - -- [THV-0019: OAuth Authorization Server Design](rfcs/THV-0019-auth-server-design.md) - Auth server architecture -- [THV-0031: Embedded Authorization Server in Proxy Runner](rfcs/THV-0031-auth-server-integration.md) - Auth server integration -- [THV-0035: Redis-Backed Storage for Auth Server](rfcs/THV-0035-auth-server-redis-storage.md) - Redis storage (implements same interfaces) -- [OAuth 2.0 RFC 6749](https://datatracker.ietf.org/doc/html/rfc6749) - Token refresh grant type (Section 6) -- [Fosite OAuth2 Library](https://github.com/ory/fosite) - OAuth2 implementation -- [golang.org/x/sync/singleflight](https://pkg.go.dev/golang.org/x/sync/singleflight) - Request deduplication - ---- - -## RFC Lifecycle - -### Review History - -| Date | Reviewer | Decision | Notes | -|------|----------|----------|-------| -| 2026-02-20 | TBD | Draft | Initial draft | - -### Implementation Tracking - -| Repository | PR | Status | -|------------|-----|--------| -| toolhive | TBD | Not Started | diff --git a/rfcs/THV-XXXX-user-to-agent-delegation.md b/rfcs/THV-XXXX-user-to-agent-delegation.md deleted file mode 100644 index 1390dc8..0000000 --- a/rfcs/THV-XXXX-user-to-agent-delegation.md +++ /dev/null @@ -1,1090 +0,0 @@ -# RFC-XXXX: User-to-Agent Delegation via OAuth Token Exchange - -- **Status**: Draft -- **Author(s)**: Jakub Hrozek (@jhrozek) -- **Created**: 2026-04-24 -- **Last Updated**: 2026-04-24 -- **Target Repository**: toolhive -- **Related Issues**: TBD - -> **Note on numbering**: this file uses the placeholder `THV-XXXX` until the -> PR is opened, at which point the number will be set to the PR number per -> the [naming convention](../CONTRIBUTING.md#rfc-file-naming-format). - -## Summary - -Extend ToolHive's embedded authorization server with the [RFC 8693 OAuth 2.0 -Token Exchange](https://datatracker.ietf.org/doc/html/rfc8693) grant so an -authenticated *agent* can present a user's JWT and receive a delegated JWT -that names the user as the principal (`sub`) and the agent as the acting -party (`act`). The exchange supports both same-trust-domain tokens (subject -tokens issued by ToolHive's own AS) and federated tokens issued by an -external OIDC provider, configured via a new trust-only upstream type -(`oidc-trust`). Cedar authorization is extended to read the `act` claim so -policies can scope what an agent may do *on behalf of* a user. - -## Problem Statement - -### What is "user-to-agent delegation"? - -In an *agentic* deployment of MCP, a non-human caller — typically an LLM agent -— calls an MCP server and ultimately a backend resource. Two distinct -identities are present on every such call: - -- the **user** who originally requested the work (e.g. "Alice") -- the **agent** that is performing the work on the user's behalf - (e.g. "coding-agent") - -Today, the embedded auth server (RFC-0019, RFC-0031) issues a JWT that -identifies *the user* and nothing else. The agent is invisible. This forces -a binary choice at the MCP server / backend: - -1. **Pretend the agent is the user.** The agent sends the user's JWT - verbatim. Authorization, audit, and rate-limiting see only the user. An - audit log entry that says "Alice deleted the repo" cannot tell whether - Alice typed the command herself or whether her agent did it on her behalf - from a tool call that may itself have come from a malicious prompt - injection. - -2. **Treat the agent as the only identity.** The agent uses its own - `client_credentials` token. The user is invisible. Per-user policy and - audit are lost; the agent looks like a single super-user account. - -Neither option is acceptable for production. We need a token format that -carries **both** identities at once, with a clear semantic distinction -between *principal* (whose data) and *actor* (who is acting). - -### Why the existing token exchange middleware (RFC-0007) does not solve it - -RFC-0007 (`pkg/oauth/tokenexchange`) is **outbound** middleware: it takes the -incoming token to the proxy and exchanges it at an *external* token endpoint -for an upstream-bound token before forwarding the request to a backend. It -does not change the identity of the principal — the result still names the -same user — and it never adds an actor. It cannot create a token that says -"this is Alice, acted upon by coding-agent" because the AS at the other end -of the exchange has no notion of the agent. - -This RFC works on the **inbound** side: the *AS itself* learns to issue -delegated tokens. The outbound middleware then continues to work unchanged on -top. - -### Who is affected - -- **Platform operators** running ToolHive with an embedded AS in front of - agentic workloads, who need per-agent audit and per-agent policy on top of - per-user policy. -- **MCP server authors** who want to express authorization rules of the form - "Alice may write to the repository, but coding-agent acting on Alice's - behalf may only read". -- **Compliance / security teams** who require an auditable record of which - agent acted on which user's behalf, distinct from the user's own actions. - -### Why now - -ToolHive is increasingly used to front agentic systems, and the existing -single-identity JWT model is the largest gap in our authorization story. -RFC 8693 has been an IETF standard since 2020 and is supported by every -major OAuth library, so the solution is well-trodden ground; the only novel -part is the integration with ToolHive's existing AS, upstream-provider -model, and Cedar authorization. - -## Background - -This section is for readers unfamiliar with RFC 8693 or with ToolHive's -auth-server architecture. Readers comfortable with both can skip ahead to -[Goals](#goals). - -### RFC 8693 in 30 seconds - -A token exchange request is an HTTP POST to the token endpoint with -`grant_type=urn:ietf:params:oauth:grant-type:token-exchange`. Its core -parameters are: - -| Parameter | Meaning | -|-----------|---------| -| `subject_token` | The token representing **the principal** the new token is *for* | -| `subject_token_type` | The format of `subject_token` (typically `access_token` or `id_token`) | -| `actor_token` *(optional)* | The token representing **the acting party** | -| `actor_token_type` *(optional)* | The format of `actor_token` | -| `audience`, `scope`, `resource` | Standard scoping parameters | - -The token endpoint validates both tokens, applies its own policy, and issues -a new token. RFC 8693 §4.1 specifies that delegated tokens carry an **`act` -claim**: a nested object whose `sub` field names the actor. - -```json -{ - "iss": "https://auth.example.com", - "sub": "alice@example.com", - "act": { "sub": "coding-agent" }, - "exp": 1714000000 -} -``` - -The principal is `alice`. The actor is `coding-agent`. Backends authorise -against `sub`; audit records both. - -### What the AS already does (after RFC-0019, RFC-0031, RFC-0052) - -- Speaks OAuth 2.0 / OIDC against MCP clients on `/oauth/{authorize,token, - callback,register}` and `/.well-known/{openid-configuration, - oauth-authorization-server,jwks.json}`. -- Authenticates the user against one or more configured **upstream IDPs** - (`oidc` for full OIDC discovery, `oauth2` for explicit-endpoint OAuth 2.0) - and stores the resulting upstream tokens. -- Issues short-lived signed JWTs whose `tsid` claim links to the stored - upstream tokens for later retrieval by vMCP outbound middleware. -- Runs **embedded** inside the proxy runner, configured by an - `MCPExternalAuthConfig` of `type: embeddedAuthServer`. - -Today the AS supports the `authorization_code`, `refresh_token`, and -`client_credentials` grants. It does **not** support token exchange. - -## Goals - -- Add the RFC 8693 token-exchange grant to the embedded AS so an - authenticated client can exchange a user JWT for a delegated JWT that - carries both identities. -- Support **two trust models** for the subject token: - 1. *Same-domain*: the subject token is an AS-issued JWT, validated - against the AS's own JWKS. - 2. *Federated*: the subject token is issued by an external OIDC provider, - trusted via a new `oidc-trust` upstream type and validated against - that provider's JWKS. -- Bind the actor identity to the authenticated OAuth client to prevent - replay of leaked actor tokens by a different client. -- Extend Cedar authorization so policies can read `act.sub` and write rules - that depend on which agent is acting. -- Plumb operational details for in-cluster IDPs: a CA-bundle reference for - private TLS and a flag to permit private-IP issuer endpoints. -- Stay strictly additive: deployments that do not configure delegation or - trust upstreams are unaffected, and the existing `authorization_code`, - `refresh_token`, and `client_credentials` flows continue to work - unchanged. - -## Non-Goals - -- **Multiple actor chaining (`act.act.…`).** RFC 8693 §4.1 allows nested - `act` claims to record an agent acting on behalf of another agent acting - on behalf of a user. This RFC issues a single-level `act` claim only. -- **Issuing OIDC ID tokens or refresh tokens from the exchange.** Only an - access token is returned. The exchange is one-shot; the agent re-exchanges - on its own schedule. -- **Token Exchange between two different ToolHive AS instances** (trust - federation across deployments). All federated trust is one-way: the AS - trusts an external OIDC provider, not vice versa. -- **Redirect-flow login against `oidc-trust` upstreams.** The new upstream - type contributes trust material only; it does not participate in browser - redirect flows. -- **mTLS / SPIFFE-based client authentication.** A separate effort explored - using the agent's mTLS identity as the actor; this RFC uses the - authenticated OAuth client identity (HTTP Basic, `client_secret_post`, or - similar). Adding mTLS-derived actors is left to a follow-up. -- **Step-up authentication** when the subject token's authentication context - is insufficient for the requested scope. -- **CLI-mode (`thv proxy`) integration.** Like RFC-0031, this is an - embedded-AS feature; the CLI does not expose an embedded AS to extend. - -## Proposed Solution - -### High-Level Design - -The AS gains a new fosite handler for the token-exchange grant. The handler -sits alongside the existing handlers in the same fosite provider; nothing -about the existing endpoints or grant types changes. - -```mermaid -flowchart TB - subgraph AuthServer["Embedded Auth Server"] - Token["/oauth/token"] - subgraph Handlers["Fosite token-endpoint handlers"] - AC["Authorization Code"] - RT["Refresh Token"] - CC["Client Credentials"] - TE["Token Exchange (NEW)"] - end - subgraph Validators["Subject-token validators"] - SV["Self-issued (own JWKS)"] - MV["Multi-issuer
(self + trusted external)"] - end - subgraph Upstreams["Upstream provider types"] - OIDC["oidc"] - OAUTH2["oauth2"] - TRUST["oidc-trust (NEW)"] - end - end - - Token --> AC - Token --> RT - Token --> CC - Token --> TE - TE --> MV - MV --> SV - MV --> TRUST - TRUST -->|JWKS via OIDC discovery| External[External OIDC IdP] -``` - -#### Two flows: same-domain and federated - -The exchange has two flows that differ only in which JWKS validates the -subject token. Everything downstream of validation is identical. - -**Same-domain flow** — the user has an AS-issued JWT and the agent -exchanges it for a delegated AS-issued JWT. - -```mermaid -sequenceDiagram - participant User - participant Agent - participant AS as ToolHive AS - participant Backend as MCP Server - - User->>AS: 1. Authorization code flow - AS-->>User: 2. JWT-A (sub=alice) - User->>Agent: 3. Pass JWT-A to agent - Note over Agent: Agent authenticates as
OAuth client coding-agent - - Agent->>AS: 4. POST /oauth/token
grant_type=token-exchange
subject_token=JWT-A
(client_secret_basic auth) - Note over AS: Validate JWT-A against own JWKS
Verify client = coding-agent
Build delegated session - AS-->>Agent: 5. JWT-B
sub=alice, act={sub:coding-agent} - - Agent->>Backend: 6. Bearer JWT-B - Note over Backend: Auth middleware validates JWT-B
Cedar: principal=alice,
actor=coding-agent - Backend-->>Agent: 7. Response -``` - -**Federated flow** — the user authenticated against an external OIDC IDP -(say Keycloak) and holds a Keycloak-issued JWT. The agent presents that -token directly, without first exchanging it through the AS authorization -code flow. - -```mermaid -sequenceDiagram - participant User - participant Agent - participant Ext as External OIDC IdP
(Keycloak) - participant AS as ToolHive AS - participant Backend as MCP Server - - User->>Ext: Authenticate - Ext-->>User: JWT-X (iss=keycloak, sub=alice) - User->>Agent: Pass JWT-X - - Agent->>AS: POST /oauth/token
grant_type=token-exchange
subject_token=JWT-X - Note over AS: peek iss claim → keycloak
route to MultiIssuerValidator
fetch keycloak JWKS via discovery
verify JWT-X
verify client = coding-agent - AS-->>Agent: JWT-B
iss=toolhive-as
sub=alice
act={sub:coding-agent} - - Agent->>Backend: Bearer JWT-B - Backend-->>Agent: Response -``` - -The federated flow lets organisations keep their existing IDP as the user's -authentication source while still using ToolHive's per-agent delegation and -audit semantics. - -### Vocabulary - -This RFC and the implementation use the following terms consistently. They -match RFC 8693 wherever the spec defines them. - -| Term | Meaning | -|------|---------| -| **Principal** | The identity carried in `sub` of the issued token. The "on behalf of" party. | -| **Actor** | The identity carried in `act.sub` of the issued token. The party performing the action. | -| **Subject token** | The token presented as `subject_token`. Names the principal. | -| **Actor token** | The token presented as `actor_token`. Names the actor. Optional in this RFC. | -| **Authenticated client** | The OAuth client that authenticated to the token endpoint (HTTP Basic etc.). Used as the actor when no `actor_token` is supplied. | -| **Self-issued token** | A JWT whose `iss` matches the AS's own issuer URL. Validated against the AS's own JWKS. | -| **Trusted external token** | A JWT whose `iss` matches one of the configured `oidc-trust` upstreams. Validated against that upstream's JWKS. | -| **Delegated token** | The access token returned from a token exchange. Carries `sub` and `act`. | - -### Detailed Design - -#### 1. Token-exchange handler - -A new package `pkg/authserver/server/tokenexchange` implements -`fosite.TokenEndpointHandler` for the `urn:ietf:params:oauth:grant-type: -token-exchange` grant. - -The handler runs after fosite's client authentication step, so by the time -it executes, `requester.GetClient()` is the authenticated OAuth client. - -Algorithm: - -1. Reject if the client is public. Token exchange requires a confidential - client. -2. Parse and validate the RFC 8693 form parameters (`subject_token`, - `subject_token_type`, optional `actor_token`/`actor_token_type`). - - Accepted subject token types: `access_token`, `jwt`, `id_token`. - - Accepted actor token types: `access_token`, `jwt`. (`id_token` - intentionally excluded — see *Security Considerations*.) -3. Validate the subject token via the configured `SubjectTokenValidator` - (see §2). Returns canonical claims (`sub`, `name`, `email`, `exp`,…) on - success. -4. Resolve the actor identity (see §3). Result is a single string: either - the actor token's `sub` (after a binding check) or the authenticated - client's `client_id`. -5. Validate that every requested scope is allowed for the client and that - the requested audience is permitted. -6. Build a delegated `session.Session` with: - - `sub` = subject token's `sub` - - `act` = `{ "sub": }` - - `name`, `email` propagated from the subject token (UI surfaces) - - **No `tsid` claim.** Delegated tokens are not bound to a stored - upstream session; the agent is acting on the *user's* identity but - does not gain access to upstream IDP tokens stored under the user's - `tsid`. -7. Cap the lifetime to `min(subject_remaining, configured_delegation_max)`. - The configured maximum defaults to 15 minutes and is bounded above by 24 - hours. A delegated token may not outlive the user's own grant. -8. Hand off to fosite's standard `IssueAccessToken` to sign and return. - -Pseudocode for the central decision in step 6: - -```go -delegatedSession := session.New( - validatedClaims.Subject, // sub = principal (user) - "", // no upstream session link - actorSub, // session.client_id field for audit - session.UserClaims{Name, Email}, -) -delegatedSession.JWTClaims.Extra["act"] = map[string]any{"sub": actorSub} -``` - -#### 2. Subject-token validation: same-domain vs federated - -The handler depends on a `SubjectTokenValidator` interface: - -```go -type SubjectTokenValidator interface { - Validate(ctx context.Context, rawToken string) (*ValidatedClaims, error) -} -``` - -Two implementations: - -- **`SelfIssuedTokenValidator`** — validates against the AS's own JWKS. - Used when the deployment has only same-domain delegation. -- **`MultiIssuerTokenValidator`** — wraps a `SelfIssuedTokenValidator` plus - zero or more `TrustedIssuer` entries. On `Validate(ctx, raw)`: - 1. Parse the JWT *without* signature verification to peek at `iss`. - 2. If `iss` equals the AS's own issuer, delegate to the self-issued - validator. - 3. If `iss` matches one of the configured trusted issuers, fetch (and - cache, with a 5-minute TTL) the issuer's JWKS via OIDC discovery and - verify there. - 4. Otherwise, reject — unknown issuers are never trusted. - -The factory selects which validator to install based on whether the AS -config contains any `oidc-trust` upstreams. Same-domain-only deployments -get the simpler self-issued validator with no external HTTP traffic. - -```go -type TrustedIssuer struct { - IssuerURL string - ExpectedAudience string // required `aud` claim, may be empty to skip -} -``` - -The `MultiIssuerTokenValidator` uses an `*http.Client` injected at -construction time. The same client is used for both OIDC discovery and -JWKS retrieval, so a single CA bundle / private-IP-permission policy -applies to everything (see §6). - -The discovery document is parsed into the existing -`pkg/oauthproto.OIDCDiscoveryDocument` type. The JWKS URL is validated -against an SSRF guard that requires HTTPS and rejects private/loopback -addresses unless `AllowPrivateIP` is enabled. - -#### 3. Actor identity resolution - -The actor is resolved by the handler in this order: - -1. **If `actor_token` is supplied:** - - Validate it against the AS's *own* JWKS — actor tokens are *always* - self-issued in this RFC. This prevents an external IDP from - unilaterally asserting "this is the agent". - - Enforce the binding `actor_token.sub == authenticated_client.client_id`. - Without this, a leaked actor token could be presented by a different - client to claim its identity. The check is unconditional, regardless - of how the client authenticated (basic, post, future mTLS). - - On success the actor is `actor_token.sub`. - -2. **If `actor_token` is absent:** - - The actor is the authenticated client's `client_id`. - -The `actor_token` path exists because some deployments want a richer actor -representation than just a `client_id` string — e.g., a JWT carrying -additional claims (deployment, region, agent version) that downstream -authorization or audit might consume. Today only `actor.sub` is consumed -by Cedar, but the audit record can still include the full claim set. - -The `actor_token` validator is exposed on the handler as a separate -`SubjectTokenValidator` (`selfValidator`) so it can be unit-tested in -isolation. The factory wires `selfValidator = validator` for same-domain -deployments and reuses the same self-issued validator inside the -multi-issuer wrapper for federated deployments. - -#### 4. New upstream provider type: `oidc-trust` - -The existing upstream provider model (RFC-0019, RFC-0052) supports two -types: `oidc` (OIDC with discovery + redirect-flow login) and `oauth2` -(explicit-endpoint OAuth 2.0 with redirect-flow login). Both are designed -around participating in the user's *login* — they hold a `client_id` / -`client_secret` and a `redirect_uri`, and the AS uses them to exchange an -authorization code for tokens. - -A federated trust upstream is fundamentally different: the AS never logs -the user in through it, never holds a client secret for it, and never has -a redirect URI. The AS only needs to verify a signature and an audience. -Reusing `oidc` would force unnecessary fields and make the validation logic -ambiguous about whether redirect-flow is supported. - -Therefore: a third type `oidc-trust`. It holds: - -- `issuer` (required) — the OIDC issuer URL, used to construct the - discovery URL (`/.well-known/openid-configuration`) and to - match the `iss` claim of incoming tokens. -- `clientId` (optional) — used as the `ExpectedAudience` for incoming - token validation. Empty disables the audience check (some IDPs do not - populate `aud` on access tokens). -- `caBundleConfigMapRef` (optional) — see §6. -- `allowPrivateIP` (optional, default `false`) — see §6. - -`oidc-trust` upstreams implement the same `OAuth2Provider` interface as -the others for storage compatibility, but the redirect-flow methods -(`AuthorizationURL`, `ExchangeCodeForIdentity`, `RefreshTokens`) return an -explicit "not supported" error. - -The AS's `defaultUpstreamFactory` extracts every `oidc-trust` upstream -from the configured list and feeds them into the -`MultiIssuerTokenValidator` as `TrustedIssuer` entries. The remaining -upstreams (`oidc`, `oauth2`) continue to drive the redirect-flow login -chain established by RFC-0052. - -#### 5. Cedar authorization: the `act` claim - -ToolHive's Cedar authorizer (`pkg/authz/authorizers/cedar`) builds a -context map from the token's claims so policies can write rules like -`context.claim_email == "alice@example.com"`. Today the converter -silently drops nested-map claims, so the `act` claim — which is itself a -JSON object — is invisible to policies. - -This RFC adds a `map[string]interface{}` case to `convertToCedarValue` so -nested claims are converted to Cedar Records. After the change, policies -can be written like: - -```cedar -// Allow Alice to call any tool, but if she's acting through coding-agent, -// only the read-only tools are allowed. - -permit ( - principal, - action == Action::"call_tool", - resource -) -when { - context.claim_email == "alice@example.com" && - !(context has claim_act) -}; - -permit ( - principal, - action == Action::"call_tool", - resource -) -when { - context.claim_email == "alice@example.com" && - context has claim_act && - context.claim_act.sub == "coding-agent" && - resource.readOnlyHint == true -}; -``` - -The change is purely additive in Cedar: existing policies that do not -reference `claim_act` are unaffected. - -#### 6. Operational plumbing: CA bundle and `AllowPrivateIP` - -Two practical concerns appeared during implementation that do not change -the conceptual model but require new fields to surface to the operator. - -**CA bundle (`caBundleConfigMapRef`)**: an `oidc-trust` upstream is most -commonly an in-cluster Keycloak (or similar) using TLS certificates issued -by a private cluster CA. The system trust store does not contain that CA, -so OIDC discovery and JWKS fetching fail with a TLS error. The new field -points at a `ConfigMap` containing the CA in PEM form. The operator -mounts the ConfigMap into the proxy runner pod, the runner passes the -filesystem path through `RunConfig`, and the AS uses -`pkg/networking.NewHttpClientBuilder().WithCABundle(path)` to construct -the HTTP client used by the multi-issuer validator. When unset, the system -trust store is used. - -**`allowPrivateIP`**: an in-cluster issuer URL typically resolves to a -private (RFC 1918) cluster IP. ToolHive's networking package defaults to -rejecting private IPs as a defence-in-depth measure against SSRF. The new -boolean field opts a single `oidc-trust` upstream into permitting private -IP resolution for its discovery and JWKS fetches. When `false` (the -default), the existing rejection policy applies. The flag is per-upstream -because two different `oidc-trust` upstreams in the same AS may want -different policies (one in-cluster, one external). - -Both fields aggregate at AS startup: if any configured `oidc-trust` -upstream has a CA bundle or requests private IPs, the HTTP client is -built once with the union of those settings and shared by the validator -across all upstreams. A future refinement could give each upstream its -own client. - -#### 7. Wiring: `RunConfig` → AS - -The `EmbeddedAuthServerConfig` `RunConfig` gains: - -```go -type RunConfig struct { - // ... existing fields ... - - // DelegationTokenLifespan caps the lifetime of delegated tokens - // issued via RFC 8693 token exchange. Empty defaults to 15 minutes; - // bounded above by 24h. - DelegationTokenLifespan string - - Upstreams []UpstreamRunConfig // existing, but Type may now be "oidc-trust" -} - -type OIDCUpstreamRunConfig struct { - // ... existing fields ... - - // CABundlePath is the absolute filesystem path to a CA-bundle PEM. - // Used only by oidc-trust upstreams for TLS verification of the - // OIDC discovery and JWKS endpoints. Empty means "use the system - // trust store". - CABundlePath string - - // AllowPrivateIP permits OIDC discovery and JWKS endpoints to - // resolve to private IP addresses. Off by default. - AllowPrivateIP bool -} -``` - -The `authserver.Config.UpstreamProviderType` enum gains -`UpstreamProviderTypeOIDCTrust` alongside the existing `oidc` and -`oauth2`. Validation requires `oidc_config` (with at least an issuer URL) -and forbids `oauth2_config` for the new type. - -The runner derives the `extraFactories` slice for the fosite provider: - -```go -extraFactories := []oauthserver.Factory{ - tokenexchange.Factory(tokenexchange.FactoryConfig{ - DelegationLifespan: cfg.DelegationTokenLifespan, - TrustedIssuers: trustedIssuersFromOIDCTrustUpstreams(upstreams), - HTTPClient: buildHTTPClientFromCAandPrivateIPFlags(upstreams), - }), -} -``` - -When no `oidc-trust` upstreams are configured, `TrustedIssuers` is empty -and the validator collapses back to the self-issued path. - -The token-exchange grant type is advertised in -`/.well-known/oauth-authorization-server` unconditionally. (RFC 8414 -discovery has no notion of "this client may use this grant"; the -authorization decision is made per-client by fosite at the token endpoint.) - -#### 8. Operator / CRD changes - -`MCPExternalAuthConfig.spec.embeddedAuthServer.upstreamProviders[].type` -gains the enum value `oidc-trust`. The validation function is reorganised -so that: - -- `oidc` and `oauth2` continue to follow the existing XOR rule - (`oidcConfig` xor `oauth2Config`). -- `oidc-trust` requires `oidcConfig` (with a valid `issuerURL`), forbids - `oauth2Config`, and additionally accepts the optional - `caBundleConfigMapRef` and `allowPrivateIP` fields. - -`OIDCUpstreamConfig` gains: - -```go -type OIDCUpstreamConfig struct { - // ... existing fields ... - - // CABundleConfigMapRef references a ConfigMap containing the CA - // certificate for verifying the OIDC issuer's TLS certificate. Used - // for oidc-trust providers where the issuer uses a non-public CA - // (e.g., internal PKI). When nil, the system trust store is used. - // +optional - CABundleConfigMapRef *CABundleSource `json:"caBundleConfigMapRef,omitempty"` - - // AllowPrivateIP allows OIDC discovery and JWKS endpoints to resolve - // to private IP addresses. Required for in-cluster OIDC issuers - // where the service DNS name resolves to a cluster-internal IP. - // Default: false. - // +optional - AllowPrivateIP bool `json:"allowPrivateIP,omitempty"` -} -``` - -The `EmbeddedAuthServerConfig` gains -`delegationTokenLifespan *metav1.Duration`. - -The operator's `controllerutil.GenerateAuthServerVolumes` is extended to -generate a CA-bundle volume mount per `oidc-trust` upstream that has a -`caBundleConfigMapRef`. The mount path follows the convention -`/etc/toolhive/authserver/upstream-ca//` and is -threaded through `RunConfig` as `CABundlePath`. - -### Configuration Changes - -A complete example: an embedded AS with one `oidc` redirect-flow upstream -(corporate SSO for user login) and one `oidc-trust` upstream (Keycloak -for federated delegation), with the proxy runner mounting both the -Keycloak CA bundle and the agent's client secret. - -```yaml -apiVersion: toolhive.stacklok.com/v1beta1 -kind: MCPExternalAuthConfig -metadata: - name: agentic-auth - namespace: my-mcp -spec: - type: embeddedAuthServer - embeddedAuthServer: - issuer: https://auth.toolhive.example.com - delegationTokenLifespan: 15m # cap delegated tokens - signingKeys: - - secretRef: { name: as-signing-keys, key: ec-key.pem } - hmacSecretRefs: - - { name: as-hmac, key: hmac-0 } - upstreamProviders: - # Existing-style upstream: corporate SSO for user login - - name: corporate-sso - type: oidc - oidcConfig: - issuerURL: https://sso.corp.example.com - clientID: toolhive-as - clientSecretRef: { name: corp-sso, key: client-secret } - redirectURI: https://auth.toolhive.example.com/oauth/callback - - # NEW-style upstream: Keycloak whose tokens we trust as subject_tokens - - name: keycloak - type: oidc-trust - oidcConfig: - issuerURL: https://keycloak.svc.cluster.local:8443/realms/agents - clientID: toolhive-resource # used as ExpectedAudience - caBundleConfigMapRef: - configMapRef: { name: keycloak-ca, key: ca.crt } - allowPrivateIP: true # in-cluster service IP -``` - -A delegation request from the agent to the AS: - -```http -POST /oauth/token HTTP/1.1 -Host: auth.toolhive.example.com -Authorization: Basic Y29kaW5nLWFnZW50OnMzY3JldA== -Content-Type: application/x-www-form-urlencoded - -grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Atoken-exchange -&subject_token=eyJhbG... -&subject_token_type=urn%3Aietf%3Aparams%3Aoauth%3Atoken-type%3Ajwt -&audience=https%3A%2F%2Fmcp.example.com -``` - -Successful response (`200 OK`): - -```json -{ - "access_token": "eyJhbGc...", - "token_type": "Bearer", - "expires_in": 899, - "issued_token_type": "urn:ietf:params:oauth:token-type:access_token" -} -``` - -Decoding `access_token`: - -```json -{ - "iss": "https://auth.toolhive.example.com", - "sub": "alice@corp.example.com", - "act": { "sub": "coding-agent" }, - "aud": "https://mcp.example.com", - "exp": 1714000899, - "iat": 1714000000 -} -``` - -## Security Considerations - -### Threat Model - -The token exchange grant is, by design, a privilege-relevant operation: -the AS is being asked to mint a token in *somebody else's* name. Threats: - -- **Subject-token forgery / replay.** A caller crafts or steals a JWT and - asks for a delegated token in that user's name. -- **Actor-token hijack.** A caller steals an actor token and presents it - with their own client authentication to claim a different agent's - identity. -- **External-IDP impersonation.** If the AS naively trusts whatever JWT it - receives, a token from an *unconfigured* issuer could be accepted. -- **SSRF via discovery.** An attacker who controls an `oidc-trust` - upstream's issuer URL could try to point the AS at internal infrastructure - via the discovery / JWKS fetch. -- **Public-client misuse.** A public OAuth client (no secret) could attempt - to exchange tokens. -- **Lifetime escalation.** A short-lived user token is exchanged for a - long-lived delegated token, extending the effective grant. - -### Authentication and Authorization - -- **Confidential clients only.** The handler rejects any request from a - client whose `Public()` is true. Public clients have no proof of identity - and therefore cannot serve as actors. -- **Subject token signature.** Verified against the AS's own JWKS - (same-domain) or against the trusted issuer's JWKS (federated). Standard - claims (`iss`, `aud`, `exp`, `iat`, `nbf`, `sub`) are validated per RFC - 7519. Tokens signed with `none` are rejected by go-jose. -- **Issuer allowlist.** The multi-issuer validator rejects any token whose - `iss` is neither the AS itself nor a configured `oidc-trust` upstream. - There is no wildcard or regex matching; issuer URLs must match exactly. -- **Actor-token binding.** When `actor_token` is supplied, its `sub` must - equal the authenticated client's `client_id`. This binds the actor - identity to the authenticated party and prevents replay of leaked actor - tokens by a different client. The check is unconditional and runs - regardless of authentication method. -- **Standard fosite scope and audience checks** apply to the request: the - client must be allowed each requested scope and each requested audience. - -### Data Security - -- **No new secrets at rest.** The handler does not persist actor tokens or - subject tokens. Delegated tokens are signed in memory and returned over - TLS; their contents (the user's `sub`, `name`, `email`) are no more - sensitive than what the AS already issues today. -- **CA bundles** are referenced through Kubernetes `ConfigMap`s - (non-secret) and mounted read-only into the proxy runner pod with - `0400` mode. -- **JWKS caching** uses an in-memory map with a 5-minute TTL. No JWKS or - discovery response is persisted. - -### Input Validation - -- **Form parameters.** Each RFC 8693 parameter is checked for presence and - format. `subject_token_type` must be one of three known URIs; - `actor_token_type` must be one of two (`id_token` is intentionally - excluded — see below). Mismatched pairs (e.g., `actor_token_type` - without `actor_token`) are rejected. -- **Why `id_token` is rejected as `actor_token_type`.** An ID token is an - *identity assertion* about a user-driven login session, including a - `nonce` and the user's authentication context. It is conceptually wrong - to use it as a bearer credential proving "I am the agent". Allowing it - would invite confused-deputy patterns where a user's ID token is replayed - as an actor token. -- **JWT parsing.** Both validators use `github.com/go-jose/go-jose/v4`, - which enforces strict JSON Web Algorithm allowlists (no `none`, no `HS*` - on RSA/EC keys). -- **Issuer URL.** Discovery URLs are constructed from the configured - issuer plus `/.well-known/openid-configuration`. The constructed URL is - validated to be HTTPS; the `jwks_uri` returned by discovery is similarly - validated and additionally checked against an SSRF allowlist (no - loopback, no link-local, no private IP unless `allowPrivateIP=true` for - that upstream). - -### Secrets Management - -- The agent's OAuth client secret is provided through the existing - `clientSecretRef` mechanism on its registered client; this RFC does not - introduce a new secret type. -- The AS's signing keys and HMAC secrets are unchanged from RFC-0031. -- No client secret is required for `oidc-trust` upstreams — they are - trust-only and never participate in OAuth flows that would require one. - -### Audit and Logging - -The handler emits debug logs with the following fields on a successful -exchange: - -| Field | Meaning | -|-------|---------| -| `subject` | The principal (user) `sub` from the subject token | -| `actor` | The actor `sub` (either authenticated `client_id` or `actor_token.sub`) | -| `lifetime` | The capped lifetime of the issued token | - -On validation failure, the failure reason and the actor are logged at -debug. The token contents themselves are never logged. The fields chosen -allow an operator to reconstruct, from log output alone, "who acted as -whom and for how long" without exposing the raw JWTs. - -A future RFC may add a structured audit-log emitter for delegation events -specifically; the current design relies on the existing `slog` plumbing -shared with the rest of the AS. - -### Mitigations - -- **Lifetime cap.** Delegated tokens are bounded by both the configured - `DelegationTokenLifespan` (default 15m, max 24h) and the remaining - lifetime of the subject token, whichever is shorter. This blunts the - blast radius of a stolen delegated token. -- **No upstream-token access.** Delegated tokens have no `tsid` claim, so - the outbound-swap middleware (RFC-0052) cannot use them to retrieve the - user's stored upstream tokens. The agent's delegated token is good only - for AS-issued downstream backends. -- **No refresh token.** The exchange returns only an access token. The - agent must re-exchange when the token expires, giving the AS a chance to - re-validate the subject token (which may by then have been revoked or - expired). -- **Public-client reject** at the very first step. -- **Issuer allowlist** — see above. -- **TLS-by-default for discovery / JWKS** — `https://` required; - exceptions explicit and per-upstream. - -## Alternatives Considered - -### Alternative 1: extend the existing outbound `tokenexchange` middleware - -Add an "act-claim" mode to `pkg/oauth/tokenexchange` (the outbound -middleware introduced in RFC-0007) so it adds an actor when forwarding to -the upstream. - -- **Pros**: no AS change; works without an embedded AS at all. -- **Cons**: the outbound middleware exchanges at an *external* token - endpoint, which has no notion of the agent and would not produce a - matching `act` claim. We'd be mis-using an outbound feature for an - inbound concern. Also it would require *every* downstream IDP to - implement RFC 8693 with `act` semantics, which most do not. -- **Why not chosen**: the trust boundary is wrong. The party that decides - "this agent may act on this user's behalf" must be in our trust domain. - The embedded AS is that party. - -### Alternative 2: actor identity from mTLS / SPIFFE only, no `actor_token` - -Always derive the actor from the agent's mTLS client certificate (via -SPIFFE ID extraction) and reject `actor_token` outright. - -- **Pros**: cryptographically strong actor identity; no token replay - concerns; aligns with WIMSE / agent-identity directions. -- **Cons**: requires every agent to have a mTLS-capable transport and a - SPIFFE workload-attestation infrastructure. Excludes simpler deployments - using `client_secret` auth. A prior iteration of this work bundled the - mTLS path; we extracted it into a follow-up so the OAuth-only path can - ship first. -- **Why not chosen now**: scope. The `actor_token` + binding-check pattern - is RFC 8693's own answer and works without infrastructure changes. mTLS - remains a future option; the `resolveActorIdentity` function is - structured so an mTLS-context source can be added later as a third - branch. - -### Alternative 3: nest as a "type" inside the existing `oidc` upstream - -Add a `mode: trust-only` field to `oidc` instead of introducing a third -type. - -- **Pros**: smaller enum. -- **Cons**: the `oidc` type's existing fields (`clientSecretRef`, - `redirectURI`, scopes) become contextually invalid; CRD validation - becomes a nest of conditional XORs. Two distinct types are clearer. -- **Why not chosen**: explicit type wins over contextual modes. - -### Alternative 4: validate trusted-issuer tokens only via OIDC userinfo - -Instead of fetching JWKS, hit the issuer's `userinfo` endpoint with the -subject token to verify it. - -- **Pros**: works for opaque (non-JWT) tokens. -- **Cons**: 1 extra round-trip per exchange; not all issuers expose - userinfo; cannot extract `act` semantics. RFC 8693 is JWT-shaped. -- **Why not chosen**: scope is JWT subject tokens. - -## Compatibility - -### Backward Compatibility - -This change is fully additive: - -- The token-exchange grant is opt-in via `delegationTokenLifespan` / - having clients permitted to use it. A deployment that does not configure - any `oidc-trust` upstreams and has no agent clients keeps issuing - exactly the same tokens as before. -- The `oidc-trust` upstream type is a new enum value. Existing - `MCPExternalAuthConfig` resources never set it. -- `OIDCUpstreamConfig.caBundleConfigMapRef` and `allowPrivateIP` are new - optional fields; existing resources omit them. -- The Cedar `act`-claim conversion is purely additive: existing policies - do not reference `claim_act`, so their evaluation is unchanged. -- `EmbeddedAuthServerConfig.delegationTokenLifespan` is optional with a - safe default. - -### Forward Compatibility - -- The `actor_token` validation lives behind the - `SubjectTokenValidator` interface, so future actor sources (mTLS-derived - SPIFFE IDs, asymmetric-key-bound tokens per RFC 7800, …) can be added - by composing additional branches inside `resolveActorIdentity`. -- `TrustedIssuer` is a struct so future per-issuer policy fields (issuer- - specific `aud` lists, allowed `sub` patterns, audit tags) can be added - without breaking the constructor. -- Multi-actor chaining (`act.act…`) is structurally supported by the - underlying claim shape; this RFC chooses not to populate it but does - not preclude a follow-up. - -## Implementation Plan - -The work has already been prototyped and validated against a Keycloak + -in-cluster proxy runner test environment. The plan below splits the -change into reviewable PRs that can land independently. Each PR is -self-contained and the AS continues to build and pass existing tests at -every stage. - -### Phase 1: core handler (foundation) - -Goal: the AS understands the token-exchange grant for *same-domain* -subject tokens, with the actor being the authenticated client. No -federated trust, no `actor_token`, no Cedar changes. - -- New package `pkg/authserver/server/tokenexchange` with - `Handler`, `SelfIssuedTokenValidator`, `Factory`. -- Wire the factory into `createProvider` via `oauthserver.NewAuthorizationServer`'s - `Factory` slice. -- Advertise the grant in `/.well-known/oauth-authorization-server`. -- Add `DelegationTokenLifespan` to `EmbeddedAuthServerConfig` and - `RunConfig`, with default 15m and max 24h validation. - -PR size: small to medium, almost entirely new code. - -### Phase 2: `actor_token` and `id_token` subject-token type - -- Add `actor_token` / `actor_token_type` parameter handling. -- Implement `resolveActorIdentity` with the binding check. -- Accept `id_token` as a valid `subject_token_type`. - -### Phase 3: Cedar `act`-claim support - -- Add `map[string]interface{}` case to `convertToCedarValue`. -- Tests verifying `claim_act.sub` is reachable from a policy. - -This phase can land independently of phases 1-2 and parallel to phase 4. - -### Phase 4: federated trust (`oidc-trust` upstream) - -- Extract `SubjectTokenValidator` interface; implement - `MultiIssuerTokenValidator` with OIDC discovery and JWKS caching. -- Add the `oidc-trust` upstream type to `pkg/authserver` config and to - the `MCPExternalAuthConfig` CRD. -- Operator: extend `defaultUpstreamFactory`, validation, and the - embedded-AS volume generation to handle the new type. Regenerate CRDs. -- Wire trusted issuers from the configured upstreams into the factory. - -### Phase 5: operational plumbing - -- Add `caBundleConfigMapRef` and `allowPrivateIP` to - `OIDCUpstreamConfig` (CRD) and `OIDCUpstreamRunConfig`. -- Operator: generate per-upstream CA-bundle volume mounts. -- AS: aggregate CA-bundle paths and `allowPrivateIP` flags into a - single `*http.Client` injected into the multi-issuer validator. - -### Dependencies - -- RFC-0019 (auth-server overview) — accepted, implemented. -- RFC-0031 (embedded AS in proxy runner) — accepted, implemented. -- RFC-0052 (multi-upstream IDP support) — referenced but **not** a hard - dependency. The new `oidc-trust` type fits inside the multi-upstream - model when it is in place; if RFC-0052 lands first, the integration is - a no-op. If this RFC lands first, `oidc-trust` upstreams compose with - the legacy single-upstream model by being separable from the redirect- - flow login chain. - -## Testing Strategy - -- **Unit tests** for the token-exchange handler covering: valid same-domain - exchange, valid federated exchange, missing parameters, invalid - `subject_token_type`, expired subject token, public client rejection, - scope mismatch, audience mismatch, lifetime capping by subject expiry, - `actor_token` happy path, `actor_token` `sub` mismatch with client, - `actor_token` without `actor_token_type` (and vice versa), - `id_token` rejected as actor token type. -- **Unit tests** for `MultiIssuerTokenValidator`: routing on `iss`, - rejection of unknown issuers, JWKS cache TTL behaviour, discovery - failure surfacing as a token-exchange error. -- **Unit tests** for Cedar `claim_act.sub` reachability with table-driven - policies. -- **Integration tests** wiring an in-process AS with one `oidc-trust` - upstream pointing at a fake OIDC server (test fixture). Round-trip a - forged-by-test token through the exchange and verify the `act` claim. -- **End-to-end test** in the operator e2e suite: Keycloak in a kind - cluster, a `MCPExternalAuthConfig` with one `oidc` upstream and one - `oidc-trust` upstream, an `MCPServer` with Cedar policies that - distinguish direct vs delegated calls, and a script that exercises both - paths. -- **Security tests** specifically covering: `actor_token` issued by an - unrelated party rejected by binding check; `subject_token` with a forged - `iss` rejected by issuer allowlist; private-IP issuer rejected when - `allowPrivateIP=false`. - -## Documentation - -- **Architecture docs** (`docs/arch/`): a new section in the auth-server - arch doc covering delegation, with the two sequence diagrams from this - RFC. -- **Operator docs**: an example of an `MCPExternalAuthConfig` with the - new upstream type, the new fields, and the resulting agent flow. -- **`stacklok/docs-website`**: a "delegating to agents" how-to that walks - through the same Keycloak-based example end to end. -- **CRD reference** (auto-generated from kubebuilder markers): regenerated - after the type changes. - -## Open Questions - -1. **Should `delegationTokenLifespan` default differently per - environment?** 15 minutes is a reasonable default for production but - may be inconvenient for development. Consider a separate - `dev-mode` toggle that loosens the cap. -2. **Should the audit log get a dedicated structured event** (e.g., - `delegation_issued`) rather than relying on debug-level slog? A - follow-up RFC on audit-event taxonomy would be a better home. -3. **Is `actor_token` issued by a *different* AS within the same trust - federation in scope for a future iteration**, or is the binding to - the authenticated client always the right call? Today we say "always"; - a federation use case may want to relax it. -4. **Does the multi-issuer validator need per-issuer client allowlists** - — i.e., "tokens from issuer A may only be exchanged by clients from - set X"? The RFC currently treats every authenticated client as eligible - to exchange any trusted token; finer-grained policy could move into - Cedar. - -## References - -- [RFC 8693 — OAuth 2.0 Token Exchange](https://datatracker.ietf.org/doc/html/rfc8693) - (in particular §1.5 *Delegation vs. Impersonation*, §2 *Token Exchange - Request and Response*, §4.1 *act (Actor) Claim*) -- [RFC 7519 — JSON Web Token](https://datatracker.ietf.org/doc/html/rfc7519) -- [RFC 8414 — OAuth 2.0 Authorization Server Metadata](https://datatracker.ietf.org/doc/html/rfc8414) -- [RFC 7591 — OAuth 2.0 Dynamic Client Registration](https://datatracker.ietf.org/doc/html/rfc7591) -- THV-0007 — Token exchange middleware (outbound counterpart) -- THV-0019 — OAuth Authorization Server (overview and design) -- THV-0031 — Embedded Authorization Server in Proxy Runner -- THV-0035 — Auth Server Redis Storage -- THV-0050 — Dedicated Auth Server Reference (`authServerRef`) -- THV-0052 — Multi-Upstream IDP Support in the Embedded Auth Server -- THV-0053 — vMCP Embedded Auth Server -- ToolHive auth server: `pkg/authserver/` -- Cedar authorizer: `pkg/authz/authorizers/cedar/` - ---- - -## RFC Lifecycle - - - -### Review History - -| Date | Reviewer | Decision | Notes | -|------|----------|----------|-------| -| 2026-04-24 | TBD | Draft | Initial submission | - -### Implementation Tracking - -| Repository | PR | Status | -|------------|-----|--------| -| toolhive | TBD | Pending — feature branch `token-delegation` | From 4bfadf414405e2036cdeb2a3f65da2820ca49071 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Wed, 15 Jul 2026 12:47:57 +0100 Subject: [PATCH 5/6] Add Middleware and Observability analysis to stateless transport RFC Documents why the middleware chain and telemetry need no structural change under stateless Modern traffic (nothing keys on the MCP protocol session id), verified against source: - Cedar authz, rate limiting, and audit all reconstruct identity from the token per request; annotation cache is keyed by tool name, not session. - Telemetry gains richer per-request attribution (protocol version from the mandatory header, client name from _meta on every span, W3C trace context for correlation in place of mcp.session.id). - Distinguish the JWT tsid (upstream-token session) from the MCP protocol session; note the audit initialize target name still reflects clientInfo. - Flag vMCP per-session backend connections as the one structural dependency, deferred to vMCP. Co-Authored-By: Claude Opus 4.8 --- rfcs/THV-0081-stateless-transport-proxies.md | 42 +++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/rfcs/THV-0081-stateless-transport-proxies.md b/rfcs/THV-0081-stateless-transport-proxies.md index 5fb0688..4bcf73b 100644 --- a/rfcs/THV-0081-stateless-transport-proxies.md +++ b/rfcs/THV-0081-stateless-transport-proxies.md @@ -51,7 +51,9 @@ front of MCP servers is affected once either side of the connection upgrades. ## Non-Goals - vMCP stateless support and the cross-generation bridge (a client on either revision - reaching a backend on either revision) — tracked separately ([toolhive#5756]). + reaching a backend on either revision) — tracked separately ([toolhive#5756]). This + includes vMCP's per-session backend MCP client connections (`pkg/vmcp/session/`), which + would churn per request under stateless unless pooled by a stable key. - The go-sdk v1.7 adoption / `mcpcompat` stateless plumbing ([toolhive#5754]). This RFC argues the transport-proxy work is independent of it (see Compatibility / Alternatives). - OAuth RFC 9207 `iss` validation ([toolhive#5760]) — genuine but unrelated work in @@ -132,7 +134,7 @@ which would start the monitor before the backend is warm and risk self-stopping empty resource ID) and the Modern request headers `Mcp-Method` (required on all requests) / `Mcp-Name` (required for `tools/call`, `resources/read`, `prompts/get`) so they are labeled for authz/audit; source `clientInfo`/protocolVersion from `_meta` -for Modern requests. +for Modern requests (consumed by telemetry — see Middleware and Observability). #### API Changes @@ -165,6 +167,42 @@ Compatibility). None. The session manager (`pkg/transport/session`) is trigger-agnostic and unchanged; only the proxy call sites that create sessions move behind the Legacy branch. +#### Middleware and Observability + +The middleware chain is session-agnostic and needs no structural change. Authentication, +token exchange, tool filtering, authorization, audit, and rate limiting all operate on the +raw request plus the authenticated identity reconstructed from the token per request; none +requires a session object or keys state on the MCP protocol session id (`Mcp-Session-Id`): + +- **Authorization (Cedar)** reconstructs its principal from the token per request. The + tool-annotation cache is keyed by tool name (not session) and full-replaced on each + `tools/list`, so the per-request routing token never touches it. +- **Rate limiting** is keyed on principal and tool, so it does not collapse into per-request + buckets when sessions disappear. +- **Audit** derives its subject client name/version from token claims, not the handshake (the + `mcp_initialize` event's target name still reflects the handshake `clientInfo`, but no + subject/identity field depends on it). + +Note the codebase has two distinct "session" concepts: token exchange keys on the JWT `tsid` +claim — the embedded auth server's upstream-token session — which is independent of the MCP +protocol session and unaffected by this change. + +The one observability touchpoint is telemetry span attributes, and the new revision supplies +them per request rather than losing them: + +- `mcp.protocol.version` is read from the `MCP-Protocol-Version` header, which is mandatory on + every Modern POST — present, and more reliably than under Legacy where the header was optional. +- `mcp.client.name` is today set only on the `initialize` span. Under Modern, telemetry instead + reads `clientInfo` from per-request `_meta` (surfaced by the parser) and sets the attribute on + every span — richer per-request attribution than Legacy, where the name appeared only on the + `initialize` span. +- `mcp.session.id` is intentionally absent under Modern; W3C trace context carried in `_meta` + (already consumed per request by the telemetry middleware) is the cross-request correlation + mechanism. + +The only code change is telemetry-side: consume `clientInfo` from the parser's `_meta` fields and +set `mcp.client.name` on all spans instead of only on `initialize`. + ## Security Considerations ### Threat Model From fe82eddd347f85f10de0a4cb20200d5dcb44c900 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 16 Jul 2026 12:31:05 +0100 Subject: [PATCH 6/6] Expand THV-0081 to full A+B design Expand the RFC to the full in-scope change set (deepened classification, Modern message-shape enforcement, authz-map registration, ping-removed health readiness) plus the deferred request-scoped streaming and subscriptions designs. Restructure the implementation plan into In scope / Deferred, dropping the tier/phase labeling, and extend Testing with the conformance and test-inversion notes. Co-Authored-By: Claude Opus 4.8 --- rfcs/THV-0081-stateless-transport-proxies.md | 201 +++++++++++++------ 1 file changed, 141 insertions(+), 60 deletions(-) diff --git a/rfcs/THV-0081-stateless-transport-proxies.md b/rfcs/THV-0081-stateless-transport-proxies.md index 4bcf73b..4661f4e 100644 --- a/rfcs/THV-0081-stateless-transport-proxies.md +++ b/rfcs/THV-0081-stateless-transport-proxies.md @@ -82,25 +82,29 @@ flowchart TD ``` The proxy fronts one backend and serves whatever client connects; the two sides have -independent revisions, giving a client × backend matrix: - -| Client | Backend | Proxy behavior | -|--------|---------|----------------| -| Legacy | Legacy | Today's path, unchanged. | -| Modern | Modern | Stateless pass-through. | -| Legacy | Modern | Unbridged — spec marks this "Fails" (no client fall-forward). Deferred to vMCP. | -| Modern | Legacy | Client's probe-then-fallback downgrades to `initialize`; the pass-through proxy forwards and classifies it Legacy. No new proxy work. | - -The two diagonal cells are the scope of this RFC. The **Modern-client / Legacy-server** -cell needs no proxy work either: the MCP-normative probe-then-fallback runs client-side, -so the client downgrades to an `initialize` handshake that the transparent proxy forwards -to the Legacy backend and classifies as Legacy. That leaves only the **Legacy-client / -Modern-server** cell genuinely unbridged — the spec's compatibility matrix marks it -"Fails" ("Legacy clients have no fall-forward mechanism"), so only a gateway could bridge -it, and that work belongs to vMCP. The agentgateway project, as an aggregator that cannot -rely on transparent pass-through, instead actively synthesizes the handshake for the -Modern-client / Legacy-server cell (`stateless_send_and_initialize`); it likewise leaves -Legacy-client / Modern-server unbridged. +independent revisions. The draft spec defines three client kinds — Legacy, Modern +(Modern-only, no fallback), and Dual-era (probe-then-fallback) — giving: + +| Client | Backend | Proxy behavior | +|-----------------|---------|----------------| +| Legacy | Legacy | Today's path, unchanged. | +| Modern/Dual-era | Modern | Stateless pass-through. | +| Legacy | Modern | Unbridged — spec marks "Fails" (no client fall-forward). Deferred to vMCP. | +| Dual-era | Legacy | Client probes, falls back to `initialize`; proxy forwards and classifies Legacy. No new proxy work. | +| Modern-only | Legacy | Spec marks "Fails" — client has no fallback. Client-side gap, outside proxy scope. | + +The era-matched cells are the scope of this RFC. The **Dual-era-client / Legacy-server** +cell needs no proxy work either: the MCP-normative probe-then-fallback runs client-side, so a +dual-era client downgrades to an `initialize` handshake that the transparent proxy forwards to +the Legacy backend and classifies as Legacy. In practice this is the common transition case — +the go-sdk and TS SDK both act as dual-era clients. A *Modern-only* client against a Legacy +server has no fallback and the spec marks it "Fails"; that is a client-side gap, outside proxy +scope. That leaves the **Legacy-client / Modern-server** cell genuinely unbridged on the proxy +side — the spec marks it "Fails" ("Legacy clients have no fall-forward mechanism"), so only a +gateway could bridge it, and that work belongs to vMCP. The agentgateway project, as an +aggregator that cannot rely on transparent pass-through, instead actively synthesizes the +handshake when fronting a Legacy server for a Modern/dual-era client +(`stateless_send_and_initialize`); it likewise leaves Legacy-client / Modern-server unbridged. ### Detailed Design @@ -129,12 +133,30 @@ to a plain reverse proxy. The internal health-monitor gate today flips ready on gate would never open. The Modern path must therefore acquire an alternate readiness trigger: gate off the first successful backend contact (and never default the gate ready, which would start the monitor before the backend is warm and risk self-stopping the proxy). - -**Parser vocabulary.** Register `server/discover` (currently unlabeled, yielding an -empty resource ID) and the Modern request headers `Mcp-Method` (required on all requests) -/ `Mcp-Name` (required for `tools/call`, `resources/read`, `prompts/get`) so they are -labeled for authz/audit; source `clientInfo`/protocolVersion from `_meta` -for Modern requests (consumed by telemetry — see Middleware and Observability). +This gate is the internal monitor's start condition and its shutdown-on-failure trigger — it is +distinct from the `/health` endpoint served to Kubernetes probes, which pings the backend +independently and is unaffected. +Note the existing `StatelessMCPPinger` POSTs a JSON-RPC `ping`, which is **removed in 2026-07-28** — +a Modern-only backend rejects it, so the Modern readiness signal must use `server/discover` or plain +successful backend contact, not the current pinger. + +**Modern message-shape enforcement.** For Modern-classified traffic (Legacy unchanged), reject +JSON-RPC batches (accepted today via `resolveSessionForBatch`) and client-sent responses (accepted +today via `handleNotificationOrClientResponse`); `GET`/`DELETE`→405; ignore `Mcp-Session-Id` and +`Last-Event-ID` (not errors). Enforcement is driven off the validated `Revision` that +`ClassifyRevision` returns for the request — **not** a re-read of the client-controlled +`MCP-Protocol-Version` header — so a client cannot flip enforcement by spoofing the header. +Because `ClassifyRevision` already rejects a header/`_meta` mismatch with `-32020`, the value +enforced against is trusted rather than raw. + +**Parser vocabulary and authz registration.** Register `server/discover` (currently unlabeled, +yielding an empty resource ID) and the Modern request headers `Mcp-Method` (required on all +requests) / `Mcp-Name` (required for `tools/call`, `resources/read`, `prompts/get`); source +`clientInfo`/protocolVersion from `_meta` (consumed by telemetry — see Middleware and Observability). +Parser labeling is **not sufficient for authorization**: `server/discover` and `subscriptions/listen` +are absent from `pkg/authz/middleware.go`'s `MCPMethodToFeatureOperation` and unknown methods are +default-denied there, so both must also be added to the authz map (`server/discover` allowed; +`subscriptions/listen` recognized so it is not 403'd, even though its delivery is deferred). #### API Changes @@ -149,12 +171,23 @@ type Revision int // Legacy, Modern func ClassifyRevision(method string, meta map[string]any, protoHeader string) (Revision, error) ``` -Classification rule: **Modern** iff `_meta` carries -`io.modelcontextprotocol/protocolVersion` (required on every Modern request); on -Streamable HTTP that value is mirrored into the `MCP-Protocol-Version` header, and a -header/body mismatch is rejected with `400` and JSON-RPC error `-32020` (the body is -authoritative). **Legacy** otherwise, with `method == "initialize"` marking session -start. A malformed or absent `_meta` classifies as Legacy — the safe direction. +A committed but unwired starting point exists — `pkg/mcp/revision.go`'s `ClassifyRevision` +implements the basic decision and the `-32020` header check (unit-tested, called nowhere). This +RFC wires it in and **deepens it** into validation, not key-sniffing: + +- **Modern** iff `_meta` carries `io.modelcontextprotocol/protocolVersion`; **Legacy** otherwise, + with `method == "initialize"` marking session start. +- **Version match, not presence.** An unknown/future version → `-32022 UnsupportedProtocolVersion` + (not implemented in `revision.go` today). +- **Required metadata.** Modern requests must carry `clientInfo` and `clientCapabilities`; a + missing required capability → `-32021 MissingRequiredClientCapability`. +- **No silent downgrade.** A request whose `MCP-Protocol-Version` header claims Modern but whose + `_meta` is absent/malformed is an **error**, not a Legacy fallback. "Malformed → Legacy" applies + only when there is *no* Modern signal at all; a request that claims Modern then fails validation + must not be downgraded (or a client could dodge Modern enforcement with broken metadata). +- **Header/body: neither wins.** On Streamable HTTP the version is mirrored into the + `MCP-Protocol-Version` header; a mismatch is rejected with `400` + `-32020 HeaderMismatch`. Do not + treat the body as authoritative and proceed. #### Configuration Changes @@ -203,6 +236,43 @@ them per request rather than losing them: The only code change is telemetry-side: consume `clientInfo` from the parser's `_meta` fields and set `mcp.client.name` on all spans instead of only on `initialize`. +#### Request-scoped streaming (designed, deferred) + +In 2026-07-28 the standalone GET SSE stream is gone but streaming is not: one POST request may +return an SSE stream of N server→client notifications (`notifications/progress`, +`notifications/message`) then a single terminal response. The in-scope changes forward only the +terminal response; this deferred work forwards the notifications too. This is **stdio-only** — the transparent proxy +(HTTP/remote, k8s) already streams N-message POST responses natively (`FlushInterval:-1`). The stdio +streamable proxy is one-request-one-response and drops notifications at `dispatcher.go:23`; note +this is a **pre-existing gap**, dropped on 2025-11-25 too, not Modern-specific. + +Shape (extends, not replaces, the existing rewrite/waiter seam): replace the buffer-1 waiter with a +per-request stream registered under two keys — the composite id (terminal) and a proxy-rewritten +`progressToken` (client-chosen, collides across sessions, so rewritten/restored like the request id); +turn the one-shot handler into a loop that forwards each notification as an SSE event and closes on +the terminal response; stop dropping notifications in dispatch; hand off to a per-stream bounded +buffer so a slow client cannot head-of-line-block others (drop oldest *progress* under pressure, +never the terminal); propagate a downstream disconnect as upstream cancellation. + +Two constraints make this a distinct, deferred piece: +- **Test inversion (required).** `TestHTTPRequestIgnoresNotifications` and the batch spec tests + currently *assert* notifications are dropped; the fix must invert them — the notification-delivery + test is the acceptance criterion. +- **Logging attribution is a protocol-level limit.** `notifications/progress` carries a + `progressToken` and routes cleanly; `notifications/message` (logging) carries no correlation + token, so over multiplexed stdio it cannot be attributed per-request — broadcast-or-drop only. + Progress works; logging is best-effort. This bounds what "streaming works" can mean for stdio. + +#### Deferred: subscriptions + +`subscriptions/listen` (replaces `resources/subscribe` + the GET SSE stream) is a long-lived +POST-response stream: an `acknowledged` first frame carrying a `subscriptionId`, then unbounded +change notifications until disconnect. Additive on the request-scoped streaming work (a third demux key; a generalized close +predicate; indefinite lifetime), but with two genuinely new and costly pieces — disconnect-driven +teardown of many concurrent indefinite streams (leak risk), and a **streaming authorization filter** +(the buffer-then-filter `ResponseFilteringWriter` cannot filter a live stream; per-message policy is +a new enforcement mode). Deferred until subscription demand is demonstrated. + ## Security Considerations ### Threat Model @@ -335,58 +405,69 @@ carries the negotiated version string — a signature-level change, not a drop-i ## Implementation Plan -### Phase 1: Classifier and parser vocabulary (no behavior change) - -- Add `ClassifyRevision`; wire it at the decode points; store `Revision` in transparent-proxy - session metadata. -- Register `server/discover`, `Mcp-Method`, `Mcp-Name` in the parser; source `clientInfo` - from `_meta` for Modern requests. - -### Phase 2: Streamable proxy — Modern is stateless - -- Unconditionally mint a per-request routing token and ignore client `Mcp-Session-Id` - for Modern; do not mint a session. - -### Phase 3: Transparent proxy — gate the initialize-triggered machinery - -- Gate the session guard, pod-pinning, re-init replay, and backend-SID rewrite on Legacy; - gate the health monitor off first backend contact rather than defaulting ready. - -### Phase 4 (deferred): Off-diagonal translation - -- Only if the cross-era cells are taken on for the transport proxies; otherwise this stays - with vMCP. +The in-scope changes are grouped by component; each is independently shippable, and everything +classifies as Legacy by default so existing traffic is unchanged until a Modern request arrives. + +### In scope — changes to make + +- **Revision classifier, parser, authz.** Wire the committed `ClassifyRevision` + (`pkg/mcp/revision.go`) at the decode points and deepen it (`-32022` unsupported version, required + `clientInfo`/`clientCapabilities`, no silent downgrade, `-32020` on mismatch); store `Revision` in + transparent-proxy session metadata. Register `server/discover`, `Mcp-Method`, `Mcp-Name` in the + parser; source `clientInfo` from `_meta`. Add authz-map (`MCPMethodToFeatureOperation`) entries for + `server/discover` + `subscriptions/listen` — parser labeling alone leaves them default-denied. +- **Streamable proxy (Modern stateless + shape enforcement).** For Modern, unconditionally mint a + per-request routing token and ignore client `Mcp-Session-Id`; do not mint a session. Behind the + Legacy/Modern mode flag, reject Modern batches and client-sent responses; GET/DELETE→405. +- **Transparent proxy.** Gate the session guard, pod-pinning, re-init replay, and backend-SID rewrite + on Legacy; give the health monitor a Modern readiness trigger (first backend contact / + `server/discover`), not the `ping`-based `StatelessMCPPinger`. +- **Telemetry.** Set `mcp.client.name` on all spans from `_meta.clientInfo`. + +### Deferred (designed above, built separately) + +- **Request-scoped streaming** (stdio streamable proxy): the one-shot→streaming dispatcher rework, + `progressToken` demux, backpressure, and the test inversion. Lower-impact, stdio-only. +- **Subscriptions** (`subscriptions/listen` + streaming authz filter): gated on demand. +- **Legacy-client/Modern-server bridge**: belongs to vMCP ([toolhive#5756]); needs backend-revision + detection (probe `server/discover` on stdio; inspect the `400` body on HTTP). ### Dependencies -None blocking. Independent of [toolhive#5754] (see Compatibility). Off-diagonal work, if -pursued, needs backend-revision detection (probe `server/discover` on stdio backends; -inspect the `400` body on HTTP backends). +None blocking. Independent of [toolhive#5754] (see Compatibility). ## Testing Strategy -- **Unit**: `ClassifyRevision` truth table (Modern `_meta`, absent/malformed `_meta`, - `initialize`, header/body mismatch → `-32020`). +- **Unit**: `ClassifyRevision` truth table — Modern `_meta`, absent/malformed `_meta`, + `initialize`, header/body mismatch → `-32020`, unsupported version → `-32022`, and + Modern-header-with-broken-`_meta` → error (not Legacy downgrade). - **Integration**: dual-era matrix — Legacy and Modern requests against the same endpoint; verify Legacy session behavior is unchanged and Modern requests mint no session and emit - no `Mcp-Session-Id`. + no `Mcp-Session-Id`; Modern batches / client-sent responses rejected. - **Security**: concurrency test asserting no response cross-delivery for concurrent Modern requests sharing a JSON-RPC id, one carrying a foreign session header. - **Kubernetes**: Modern request to a multi-replica backend routes without pod-pinning and does not re-initialize on backend `404`; the internal monitor does not self-stop on a cold-starting Modern backend. +- **Health**: pinger against a Modern backend that rejects `ping` (the removed-method case). +- **Conformance**: the existing job (`test/conformance/`, #5806) runs the upstream suite against + the **HTTP/transparent** path on spec `2025-06-18`, so it does **not** cover the stdio streamable + proxy or 2026-07-28 — it is not a safety net for this work. A stdio-backend run and a + 2026-07-28-capable upstream version are deferred as a separate testing item. +- **Streaming (when built)**: the notification-delivery test that **inverts** + `TestHTTPRequestIgnoresNotifications` and the batch spec tests, which today assert the drop. ## Documentation - Architecture doc: `docs/arch/stateless-transport-proxies-design.md` (already drafted, [toolhive#5808]) — this RFC is its decision-level counterpart. -- Update `docs/arch/03-transport-architecture.md` when Phase 2/3 land. +- Update `docs/arch/03-transport-architecture.md` when the proxy changes land. ## Open Questions 1. Should the revision-aware GET/DELETE gate on the transparent proxy be a refinement of the existing `statelessMethodGate`, or a distinct mechanism keyed on `MCP-Protocol-Version`? -2. Timing: file the Phase 1–3 implementation issues on acceptance, or after go-sdk v1.7 +2. Timing: file the implementation issues on acceptance, or after go-sdk v1.7 final (given the spec is still finalizing)? 3. Do we want the optional future tie-in where `--stateless` implies an "assume Modern" fast-path, or keep the two strictly orthogonal?