feat(mcp): delegated per-user token resolution for type=user (#317)

[initializ-mk]

Delegated per-user token resolution for auth.type: user — the token-acquisition core of #317's delegated path. Builds directly on #326 (which landed type: user as a pure stub that always returned ErrNoToken).

What

type: user now resolves a per-requesting-user access token from the platform token endpoint (§19 delegated contract), instead of always failing.

• delegatedTokenSource — POSTs {server, subject} and caches the access token per subject, so distinct users never share a token. The lock is not held across the network fetch, so a slow fetch for user A never blocks user B (the multi-user path is the whole point of Epic: runtime per-user, ephemeral per-session MCP OAuth (interactive login via Slack / A2A UI) #317). A platform 401/403/404 → ErrNoToken (auth-required), so a user without a grant stays lazy/non-blocking. The refresh token never reaches the agent (invariant 8).
• buildAuthFn type: user — reads the requesting user from auth.IdentityFromContext(ctx) (email preferred, then user id) and resolves that subject's token. authFn runs per-request with the request ctx (verified: MCPTool.Execute(ctx) → CallTool(ctx) → transport → authFn(ctx)), so a shared connection carries per-user tokens on the call path. No user in ctx → ErrNoToken (lazy; never at startup).
• NewServer type: user now requires the top-level platform block (it resolves via the platform endpoint), same as type: platform.
• Extracted doPlatformTokenRequest shared by the agent-principal (feat(mcp): platform + user auth resolvers — reads via platform, writes via user (§19) #326) and delegated sources; the agent-principal path is behavior-preserved.

How it fits the seam

MCP tool call (ctx carries auth.Identity)
  → authFn(ctx)  [type: user]
  → delegatedSubject(ctx) = email/uid
  → POST {token_endpoint} {server: ref, subject}   (agent credential as Bearer)
  → per-user access token   (403/404 ⇒ ErrNoToken, auth-required)

Scope (honest)

This is the token-acquisition layer. Two follow-ons, called out so nobody expects them here:

• Per-user connection lifecycle — MCP initialize runs at connection setup; a type: user connection must establish lazily under a user's session (there's no user at startup), and ideally per-subject. Today the per-request token works on a connection whose ctx carries a user; the lazy/per-user connection establishment is the next slice.
• Raw OIDC assertion (user_assertion) for a broker-side ID-JAG exchange — auth.Identity carries only parsed claims today. The platform's vaulted-3LO path resolves on subject alone, so this isn't needed yet; carrying the raw assertion is Epic: runtime per-user, ephemeral per-session MCP OAuth (interactive login via Slack / A2A UI) #317 item 1's second half.

Docs

#326 shipped type: platform/type: user without config docs — added a "Managed identity (platform / user)" section (the platform block, both types, the lazy/required rules, egress).

Tests

Per-subject fetch+cache (distinct users → distinct tokens; same user cached — one fetch each), platform-403 → ErrNoToken, buildAuthFn resolves the subject from ctx / lazy without a user. Updated #326's rules test for the new platform-block requirement on type: user. Full forge-core + forge-cli/runtime suites pass; lint clean.

[initializ-mk]

Review: delegated per-user token resolution for type=user

This is the highest-risk area of the MCP-OAuth series — multi-user token resolution, where a cross-user token leak would be the worst outcome — and I traced the security-critical invariants; they hold:

• Per-subject caching is correct. cache map[string]cachedToken is keyed by subject, guarded by d.mu on every read/write (no concurrent-map hazard), and cachedToken is a value type so the fast-path copy doesn't alias. There is no path by which user A's fetch stores under, or serves to, user B.
• The subject comes from a trusted source, not a user-controllable field. delegatedSubject reads auth.IdentityFromContext(ctx) — the identity the auth middleware set from the verified bearer/OIDC token — email preferred, then uid. A requesting user can't spoof another's subject because it's the authenticated identity, not anything in the MCP call.
• Fail-closed throughout: empty subject → ErrNoToken (no empty-subject fetch), no user in ctx → ErrNoToken (lazy, never at startup), platform 401/403/404 → ErrNoToken (awaiting consent). required: true is rejected for type: user.
• Invariant 8 held structurally: doPlatformTokenRequest parses only access_token/expires_in — a refresh_token can't reach the agent even if the platform sends one.
• Concurrency pattern is right: the lock is released across the network fetch, so a slow fetch for user A never blocks user B — the whole point of #317.
• Refactor is behavior-preserving: the extracted doPlatformTokenRequest returns the raw non-200 status so each caller classifies it — agent-principal still maps any non-200 → ErrProtocolError (unchanged), delegated maps 401/403/404 → ErrNoToken.

Good test coverage (distinct users → distinct tokens, same user cached to one fetch, 403 → ErrNoToken, lazy-without-user). CI green. No blockers — one docs finding, then minors.

1. Medium (docs, isolation-boundary clarity): "each user gets their own token" omits the connection-sharing caveat

The operator docs state delegated type: user gives "each user their own token (cached per subject)" — but the PR's own honest scope note says the per-user connection lifecycle is a follow-up: MCP initialize runs at connection setup, and today the per-request token rides a shared connection. So this PR delivers per-user token-level isolation, not yet connection-level. Against a session-stateful MCP server (one that binds identity at initialize rather than re-authorizing per call), swapping the per-call token may not re-scope the session — all users could act as the connection-establisher. The scope note is in the PR body, but an operator reads the docs, where "each user gets their own token" reads as complete isolation. Recommend a docs caveat: this resolves a per-user token per call; the underlying MCP connection is currently shared, and full per-subject connection isolation is a follow-up — verify a session-stateful server re-authorizes per call before relying on it for isolation. Inline.

2. Minor: the per-subject cache is unbounded and never evicts

d.cache[subject] is added per distinct user and never removed — expired entries are checked on read but never deleted, and there's no size bound. A long-running agent serving many distinct users grows the map with distinct-subject cardinality (each entry a token string + timestamp). For a high-user-count deployment that's a slow memory leak. Recommend evicting on expiry (delete the stale entry when the fast-path check finds it expired) or an LRU/TTL bound. Inline.

3. Minor: no per-subject singleflight → first-request stampede

Unlike the #325 client_credentials path (which singleflights), concurrent first requests for the same subject each miss the cache and each fire a platform fetch (last-write-wins on the cache). Correct (all return that user's token), just N redundant platform calls on a cold-cache burst for one user. Per-subject singleflight would collapse them; acceptable to defer, worth noting.

Nit: the subject (email) appears in error strings

ErrNoToken/ErrProtocolError messages embed subject %q (the user's email). If these errors are logged, user emails land in logs — a mild PII surface depending on the redaction posture. Consider logging the uid or a hash where the email isn't needed for the operator action.

Verdict

Merge-ready. The token-acquisition core is correct and secure — per-subject isolation, trusted subject, fail-closed, invariant 8 structural. Finding 1 is the one I'd fold in: it's a two-line docs caveat, and it's the difference between an operator understanding this as token-level isolation (true today) vs full per-user isolation (the follow-up). Findings 2–3 and the nit are cache-lifecycle and diagnostics follow-ups.

[initializ-mk]

Medium (isolation-boundary clarity): "each user gets their own token" needs the connection-sharing caveat.

This is true at the token level — but the PR's own scope note says the per-user connection lifecycle is a follow-up: MCP initialize runs at connection setup, and today the per-request token rides a shared connection. Against a session-stateful MCP server (identity bound at initialize, not re-checked per call), the per-call token swap may not re-scope the session, so users could effectively act as the connection-establisher.

An operator reads the docs, not the PR body, and "each user gets their own token" reads as complete isolation. Suggest adding: "This resolves a per-user token on each call; the underlying MCP connection is currently shared. Full per-subject connection isolation is a follow-up — for a session-stateful MCP server, confirm it re-authorizes per call before relying on this for isolation." Keeps operators from over-trusting isolation this slice doesn't yet fully provide.

[initializ-mk]

Added the caveat. The docs now spell out that this is per-user token-level isolation per call, the connection is shared, and a session-stateful server may not re-scope on the per-call token — confirm per-request re-auth before relying on it for hard isolation. This is exactly what the per-user connection lifecycle follow-up closes (starting on it next).

[initializ-mk]

Minor: this per-subject cache is unbounded and never evicts. Entries are added per distinct user and only read-checked for expiry — expired entries are never deleted, and there's no size cap. A long-running agent serving many distinct users grows the map with distinct-subject cardinality (a slow leak). Cheap fix: delete(d.cache, subject) when the fast-path check finds an entry expired, and/or a TTL/LRU bound. The token strings are also sensitive, so not holding stale ones longer than needed is a mild plus.

[initializ-mk]

Bounded it: evict the stale entry on the fast-path read, and sweep expired entries on each write — so one-shot users can't grow the map unbounded and tokens aren't held past use, with no background goroutine. Per-subject singleflight (finding 3) I've left as the noted follow-up.

[initializ-mk]

Re-review of 6582f78 — both findings resolved, CI green

Finding 1 (docs isolation caveat) — landed exactly as recommended, arguably clearer. The new ⚠️ callout draws the distinction precisely: per-user token on each call vs. the shared connection, and — the operational nuance that matters — it splits stateless servers (re-authorize per request → real per-user isolation) from session-stateful ones (bind identity at initialize → may not re-scope), telling operators to confirm per-request re-auth before relying on type: user for hard isolation. That closes the over-trust gap: an operator now knows this is token-level isolation and exactly what to verify before treating it as full isolation.

Finding 2 (unbounded cache) — bounded correctly, with a sound design choice. The fast-path read now deletes a stale entry, and each write opportunistically sweeps expired entries before inserting — bounding the map to live entries, no background goroutine, and the side benefit of not holding sensitive tokens past expiry.

One characteristic worth a mental note (not a blocker): the on-write sweep is O(n) under the lock, so a cold-start burst of N distinct first-time users does ~O(N²) cumulative map work across brief critical sections. Negligible at realistic per-agent user cardinality (the network fetches parallelize outside the lock); only worth revisiting (heap-by-expiry / periodic sweep) at extreme scale — tens of thousands of concurrent distinct users on one agent. The deliberate no-goroutine simplicity is the right call for the expected scale.

Findings 3 + the email nit — deferred with sound reasoning. Per-subject singleflight is a noted follow-up (I'd flagged it as deferrable). Keeping the subject email in the "no grant yet" error is a deliberate, correct call — that error is actionable precisely because it names the user who must consent. Good judgment on which nit to accept vs. justify.

Verdict

Merge-ready. The token-acquisition core was already correct and secure (per-subject isolation, trusted subject, fail-closed, invariant 8 structural), the cache is now simply bounded, and — the finding that mattered most — the docs draw the token-level-vs-connection-level isolation boundary honestly so operators don't over-trust this slice against a session-stateful server. All 10 checks green.