OAuth: re-key store to (server, issuer) + SEP-837/2350/2352 auth UX

[cliffhall]

Closes #1625

Re-keys the persisted OAuth store to (server, issuer) and adds the SEP-837/2350/2352 authorization-hardening UX. SDK v2 auth() drives the SEP-2352 flow off the provider's ctx.issuer; the Inspector's job is to honor it and round-trip the issuer stamp.

SEP-2352 — issuer-bound credentials

• ServerOAuthState nests issuer-bound credentials under byIssuer[issuer] = { clientInformation, clientRegistrationKind, tokens }, with an activeIssuer pointer answering the transport's ctx-less per-request bearer read.
• Fixes a latent bug: OAuthStorageBase re-parsed tokens/client-info through OAuthTokensSchema/OAuthClientInformationSchema, which strip the non-RFC issuer field and silently defeat the SDK's discardIfIssuerMismatch. The store re-attaches the stamp from the byIssuer key.
• BaseOAuthClientProvider threads ctx.issuer and implements discoveryState()/saveDiscoveryState() (activates the SDK's callback-leg AuthorizationServerMismatchError binding check) and invalidateCredentials().
• Lazy, migration-free upgrade: a legacy pre-1625 blob's top-level clientInformation/tokens are returned as an unkeyed fallback until the first issuer-stamped save promotes them into byIssuer. Existing web/TUI/CLI sessions survive.

SEP-837 — application_type

• DCR declares application_type: "native" (the Inspector is reached over localhost).
• ClientSettingsForm renders RegistrationRejectedError detail and reflects DCR's deprecated-in-favor-of-CIMD (SEP-991) status.

SEP-2350 — step-up scope union

• Per-server onInsufficientScope (reauthorize | throw) setting wired ServerSettingsForm → StoredMCPServer.oauth → StreamableHTTPClientTransport.
• StepUpAuthModal visualizes the prior ∪ challenged scope union, tagging each scope new vs already granted.

mcpAuth simplification

• Deletes the hand-rolled authorizeWithoutRefresh (which mishandled issuer stamping); forwards forceReauthorization/skipIssuerMetadataValidation to the native SDK auth() so both the initial and step-up paths get SEP-2352 stamping.

Vocabulary + docs

• OAuthStep gains cimd_fetch / issuer_comparison / scope_step_up (recorded on a step-up redirect); the Network tab labels auth-category requests by OAuth phase (Discovery/Registration/Authorize/Token).
• specification/v2_auth_hardening.md gains an as-built status section.

Deferred (per issue triage)

The SdkError(EraNegotiationFailed) → UnauthorizedError unwrap and finishAuth(URLSearchParams) whole-params form only fire under auto/pin negotiation; the Inspector is pinned to versionNegotiation: 'legacy', so these land with the era-aware connection model (card 3 of the stack).

Screen capture

step-up.mov

Verification

Full npm run ci equivalent, all green:

• validate (4 clients: lint/build/test)
• coverage (per-file ≥90, exit 0 — every changed file clears the gate; new integration coverage for the issuer round-trip, AS-migration fallback, transport option, backend validation)
• Storybook (412 play tests)
• smoke (launcher/cli/tui/web boot)

🤖 Generated with Claude Code

[cliffhall]

Live smoke test — standard OAuth + step-up against a real server

Verified end-to-end in the running web client (fresh Vite dev build on this branch) against a real standard-OAuth server with DCR, driving the actual InspectorClient → provider → file-backed oauth.json path — not the in-process integration harness. The run also exercised the legacy → byIssuer migration on a real, pre-existing ~/.mcp-inspector/storage/oauth.json (3 prior entries incl. an EMA IdP session).

Server under test: test-servers/configs/oauth-step-up-demo.json — streamable-http, supportDCR: true, scopesSupported: [mcp, tools:read, weather:read], tool get_temp gated on weather:read.

1. SEP-2352 — issuer re-keying (initial connect: discovery → DCR → token)

Persisted servers["http://localhost:8081/mcp"] after connect:

top-level keys: [activeIssuer, byIssuer, codeVerifier, discoveryState, scope]
activeIssuer:   http://localhost:8081
byIssuer["http://localhost:8081"]:
  clientRegistrationKind: dcr
  clientInformation.client_id: test_client_…   issuer stamp: http://localhost:8081
  tokens: access_token ✓  refresh_token ✓      issuer stamp: http://localhost:8081
legacy top-level tokens/clientInformation: absent

• Credentials are keyed under byIssuer[issuer], not per-server-URL. ✅
• The issuer stamp round-trips on both clientInformation and tokens — this is the latent bug the PR fixes (the Zod re-parse was stripping it). ✅
• activeIssuer set (answers the transport's ctx-less per-request bearer read); discoveryState persisted (activates the SDK's callback-leg AuthorizationServerMismatchError binding check). ✅
• New saves write straight to byIssuer — no legacy top-level fields. ✅

2. Migration integrity (real pre-1625 store)

Diffed the live store against a pre-test backup:

http://localhost:63244/mcp      UNCHANGED
http://localhost:8080/mcp       UNCHANGED
ema-idp:https://idp.xaa.dev     UNCHANGED
idpSessions                     UNCHANGED

Lazy migration touched nothing that existed — only the newly-authorized server gained a byIssuer entry. ✅

3. SEP-2350 — step-up scope union

Set the new per-server Scopes field to mcp tools:read (omitting weather:read) via Server Settings → cleared stored OAuth → reconnected → ran get_temp:

• get_temp returned 403 insufficient_scope (initial token lacked weather:read).
• The StepUpAuthModal rendered the union with each scope tagged new (weather:read) vs already granted (mcp, tools:read) — see the screen capture section in the PR description.
• After approving, the re-authorized token carried the union:

byIssuer["http://localhost:8081"].tokens.scope = "mcp tools:read weather:read"
  (issuer stamp preserved; DCR re-registered cleanly after the clear)

The 403 → union → re-authorize loop resolved and the widened grant persisted. ✅

Also exercised

• SEP-837 application_type: "native" DCR (registration accepted by the AS).
• New per-server "Insufficient-scope response" (reauthorize | throw) control in Server Settings.
• Network tab OAuth phase badges (Discovery / Registration / Token) on auth-category requests.

Gate

Full npm run ci equivalent green locally: validate (4 clients) · coverage (per-file ≥90, exit 0) · Storybook (412 play tests) · smoke (launcher/cli/tui/web boot).

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 47s —— View job

---

PR Review — OAuth re-key to (server, issuer) + SEP-837/2350/2352 auth UX

• Read the diff against origin/v2/main
• Review core auth changes (storage, providers, store, mcpAuth)
• Review transport / server-route wiring
• Review web UI components (forms, modals, network phase)
• Post review feedback

Reviewed all 35 files. This is a well-structured change: the store re-keying, the lazy migration, and the mcpAuth simplification are all clean, and the test coverage + live smoke are thorough. A few observations below — one substantive edge case, the rest minor.

---

🟡 Legacy-fallback credentials get stamped with an issuer they weren't minted by

core/auth/oauth-storage.ts — getClientInformation (~line 165) and getTokens (~line 268):

const slot = this.issuerSlot(state, issuer);
const clientInfo = slot?.clientInformation ?? state.clientInformation; // may come from legacy fallback
const parsed = await OAuthClientInformationSchema.parseAsync(clientInfo);
return withIssuer(
  parsed,
  slot ? this.resolveReadIssuer(state, issuer) : undefined, // stamped whenever a slot *exists*
);

The issuer stamp is gated on whether a byIssuer slot exists, not on whether the returned value actually came from that slot. If a slot exists for issuer X but is missing this particular credential (e.g. byIssuer[X] = { tokens } with no clientInformation), the read falls back to the legacy top-level state.clientInformation and still stamps it with X.

Because withIssuer is precisely what re-activates the SDK's discardIfIssuerMismatch, stamping a legacy credential registered against a different AS with the current issuer would make the SDK trust a cross-AS credential — the exact reuse SEP-2352 is meant to block. The scenario is narrow (a partially-populated slot alongside a surviving legacy field; the normal SDK flow keys client info before tokens), so this is low-likelihood rather than a live bug, but the guard is looser than the invariant it's protecting. Consider gating the stamp on the source:

return withIssuer(
  parsed,
  slot?.clientInformation ? this.resolveReadIssuer(state, issuer) : undefined,
);
// and for getTokens: slot?.tokens ? ... : undefined

Fix this →

🟢 Minor — clearing one issuer's credentials promotes it to activeIssuer

updateIssuerSlot unconditionally sets activeIssuer: issuer. That's correct for saves, but clearClientInformation(url, false, issuer) and clearTokens(url, issuer) route through it too, so clearing a specific AS's credentials marks that AS as the active one. Harmless in the usual clear-then-reauthorize-same-AS path, but a "clear" setting the active pointer is a slightly surprising side effect. Worth a comment if intentional.

🟢 Minor — oauthNetworkPhase substring matches can over-match

clients/web/src/utils/oauthNetworkPhase.ts uses path.includes("/register/"), "/token/", "/authorize/" in addition to the endsWith checks. A nested path segment (/token/refresh, or a proxy path like /api/register/foo) would be classified even when it isn't that OAuth endpoint. This is presentation-only (a Network-tab badge) so the blast radius is nil, but the includes arms are broader than the endsWith arms suggest.

✅ Things that look right

• The latent-bug fix is the highlight. Re-attaching the issuer stamp that OAuthTokensSchema/OAuthClientInformationSchema strip on parse (withIssuer) is the correct fix, and recovering it from the byIssuer key rather than the parsed value is the right source of truth.
• Lazy migration (legacy top-level fields as unkeyed fallback, promoted on first stamped save, never written by new saves) is a clean migration-free upgrade, and the live smoke against a real pre-1625 oauth.json is convincing evidence it preserves existing sessions.
• Deleting authorizeWithoutRefresh and forwarding forceReauthorization/skipIssuerMetadataValidation straight to SDK v2 auth() removes a genuinely error-prone reimplementation. Confirmed @modelcontextprotocol/client@2.0.0-beta.4 is pinned, which is what supplies these native options.
• onInsufficientScope wiring is complete for the paths that matter: types → serverList → node transport, plus request-body validation on both the read and write paths of the remote server route. The web client reaches the transport through the node backend, so wiring only core/mcp/node/transport.ts is sufficient (no separate browser StreamableHTTPClientTransport).
• application_type: "native" (SEP-837) with the explanatory comment about OIDC ASes defaulting to "web" is correct and well-documented.
• StepUpAuthModal union split (new vs already granted) is a real UX improvement over the old "the server may also show scopes you already granted" hand-wave.

Nice work — the 🟡 item is the only one I'd want addressed (or explicitly waved off) before merge; the two 🟢s are optional polish.
· v2/1625-oauth-rekey-issuer

[cliffhall]

Thanks — all three addressed in 111d50b.

🟡 Legacy-fallback stamped with an unrelated issuer — fixed. The stamp is now gated on the credential's actual source, not on the slot merely existing. getClientInformation/getTokens capture fromSlot = slot?.clientInformation / slot?.tokens and only withIssuer(..., fromSlot ? issuer : undefined). So a partially-populated slot that falls back to a surviving legacy top-level field returns it unstamped, keeping discardIfIssuerMismatch engaged. Added a test covering both directions (slot has only tokens → legacy client info returned unstamped, and slot has only client info → legacy tokens returned unstamped).

🟢 Clear promotes to activeIssuer — fixed rather than commented, since the side effect wasn't intentional. updateIssuerSlot gained a setActive param (default true); the save paths keep the promotion, and the two issuer-scoped clear paths pass false. Added a test asserting that clearing issuer B leaves issuer A's token answering the ctx-less (per-request bearer) read.

🟢 oauthNetworkPhase over-match — tightened. The register/token/authorize arms now match the final path segment (trailing slash tolerated) instead of includes("/…/"), so /oauth/token/refresh and /api/register/foo no longer classify. Discovery keeps includes since RFC 9728 nests the resource path under /.well-known/…. Added a test for the nested-segment and trailing-slash cases.

Coverage gate green (per-file ≥90, exit 0; 4015 tests). Re-requesting review below.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 35s —— View job

---

Re-review — OAuth re-key + SEP-837/2350/2352 (after commit 111d50b3)

• Read the follow-up diff (111d50b3) against the prior review
• Verify 🟡 issuer-stamp source gating fix + test
• Verify 🟢 clear/activeIssuer fix + test
• Verify 🟢 oauthNetworkPhase over-match fix + test

All three items from the prior review are addressed correctly, with targeted tests. The follow-up is a tight 4-file, +102/−9 change that touches only what was flagged — no scope creep. LGTM.

---

🟡 → ✅ Issuer-stamp source gating (core/auth/oauth-storage.ts)

Fixed exactly as suggested, and arguably better documented than the ask. Both reads now capture the credential's source and gate the stamp on it, not on slot existence:

const fromSlot = slot?.clientInformation;      // getClientInformation (L161)
const clientInfo = fromSlot ?? state.clientInformation;
...
return withIssuer(parsed, fromSlot ? this.resolveReadIssuer(state, issuer) : undefined);

const fromSlot = slot?.tokens;                 // getTokens (L274)
const tokens = fromSlot ?? state.tokens;
...
return withIssuer(parsed, fromSlot ? this.resolveReadIssuer(state, issuer) : undefined);

A partially-populated slot (byIssuer[X] = { tokens } with no clientInformation) now falls back to the legacy unkeyed credential unstamped, so discardIfIssuerMismatch stays engaged and a cross-AS credential can't be silently trusted. The test at storage-browser.test.ts:536 covers both directions (slot-has-only-tokens → legacy client info unstamped; slot-has-only-client-info → legacy tokens unstamped). ✅

🟢 → ✅ Clear no longer promotes to activeIssuer (core/auth/oauth-storage.ts)

updateIssuerSlot gained a setActive = true param spread conditionally (...(setActive && { activeIssuer: issuer }), L127). I confirmed all four call sites route correctly: the two save paths (saveClientInformation L196, saveTokens L295) keep the default true; the two issuer-scoped clears (clearClientInformation L244, clearTokens L318) pass false. The test at storage-browser.test.ts:585 asserts that clearing issuer B leaves issuer A answering the ctx-less read. ✅

_{Non-blocking: clearing tokens for the issuer that is currently active leaves activeIssuer pointing at a now-empty slot — but the ctx-less read then just returns undefined/legacy-fallback, which is the correct outcome, so no action needed.}

🟢 → ✅ oauthNetworkPhase over-match (clients/web/src/utils/oauthNetworkPhase.ts)

The register/token/authorize arms now strip trailing slashes and match the final segment:

const endpoint = path.replace(/\/+$/, "");
if (endpoint.endsWith("/register")) return "registration";
if (endpoint.endsWith("/token")) return "token";
if (endpoint.endsWith("/authorize")) return "authorize";

/oauth/token/refresh and /api/register/foo no longer classify; /oauth/token/ (trailing slash) still resolves to token. Discovery correctly retains includes since RFC 9728 nests the resource path under /.well-known/…. Test at oauthNetworkPhase.test.ts:48 covers the nested-segment and trailing-slash cases. ✅

---

Clean follow-up — nothing further from me. Ship it. 🚢

_{· v2/1625-oauth-rekey-issuer}