Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/auth-sep-2352-credential-isolation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@modelcontextprotocol/client': minor
---

Per-authorization-server credential isolation (SEP-2352). `auth()` now stamps an `issuer` field onto every value it passes to `saveTokens()` / `saveClientInformation()` and threads `{ issuer }` to `tokens()` / `clientInformation()`; on read, a stored credential whose stamp names a different authorization server is treated as `undefined`, so a `client_id` / `refresh_token` issued by one AS is never sent to another. Providers that round-trip stored values verbatim are protected with no code change; multi-AS providers may key storage on `ctx.issuer`. New `AuthorizationServerMismatchError` (callback-leg gate). `OAuthClientProvider.saveAuthorizationServerUrl()` / `authorizationServerUrl()` are deprecated (still written, never read). `ClientCredentialsProvider`, `PrivateKeyJwtProvider`, `StaticPrivateKeyJwtProvider`, and `CrossAppAccessProvider` gain `expectedIssuer` and no longer define `saveClientInformation()`.
3 changes: 3 additions & 0 deletions docs/migration-SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ The OAuth client flow additionally throws dedicated classes from `@modelcontextp
| `exchangeAuthorization()` / `refreshAuthorization()` / `fetchToken()` / `requestJwtAuthorizationGrant()` / `exchangeJwtAuthGrant()` non-https token endpoint | `InsecureTokenEndpointError` (`tokenEndpoint`) |
| RFC 9207 `iss` mismatch / RFC 8414 §3.3 issuer-echo mismatch | `IssuerMismatchError` (`kind`, `expected`, `received`) |
| Transport 403 `insufficient_scope` with `onInsufficientScope: 'throw'`, or default mode without an `OAuthClientProvider` | `InsufficientScopeError` (`requiredScope`, `resourceMetadataUrl`, `errorDescription`) |
| `auth()` callback leg: discovery resolves a different AS than the recorded redirect target | `AuthorizationServerMismatchError` (`recordedIssuer`, `currentIssuer`) |

Update OAuth error handling:

Expand Down Expand Up @@ -598,6 +599,8 @@ OAuth callback handling: pass the callback URL's `URLSearchParams` to `transport

Token-exchange / refresh now refuse to send credentials to a non-`https:` token endpoint (loopback `localhost` / `127.0.0.1` / `::1` exempt), throwing `InsecureTokenEndpointError` with no opt-out. `auth()` surfaces this on every path including refresh — switch any plain-`http:` AS on a non-loopback host to TLS.

`auth()` stamps an `issuer` field onto every value it passes to `saveTokens()` / `saveClientInformation()` and threads `{ issuer }` as the `ctx` argument to those methods plus `tokens()` / `clientInformation()` (SEP-2352). On read, a stored value whose `issuer` names a different AS is treated as `undefined` and the flow re-registers / re-authorizes. Round-trip the stored object verbatim and you're protected; multi-AS providers key storage on `ctx.issuer`. `OAuthClientProvider.saveAuthorizationServerUrl()` / `authorizationServerUrl()` are `@deprecated` (still written for back-compat, never read by the SDK). `ClientCredentialsProvider` / `PrivateKeyJwtProvider` / `StaticPrivateKeyJwtProvider` / `CrossAppAccessProvider` gain `expectedIssuer?: string` and no longer define `saveClientInformation()`.

No code changes required; wire-behavior note: on a 2026-07-28 Streamable HTTP connection, aborting an in-flight client request (caller `signal` / timeout) closes that request's SSE response stream as the spec cancellation signal — `notifications/cancelled` is no longer POSTed
there. 2025-era connections and stdio at any era still send `notifications/cancelled`. Custom `Transport` implementations that open one underlying request per outbound message and honor `TransportSendOptions.requestSignal` may declare `readonly hasPerRequestStream = true` to opt
into the same routing.
Expand Down
18 changes: 18 additions & 0 deletions docs/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1633,10 +1633,28 @@ Step-up retries are now hard-capped per send (`maxStepUpRetries`, default 1) reg

The GET listen-stream open path now applies the same step-up handling as the POST send path.

### Credentials are bound to the issuing authorization server (SEP-2352)

`auth()` now stamps an `issuer` field onto every value it passes to `saveTokens()` and `saveClientInformation()`, and passes `{ issuer }` as the second (`ctx`) argument to `tokens()` / `saveTokens()` / `clientInformation()` / `saveClientInformation()`. On read, a stored value whose `issuer` stamp names a different authorization server is treated as `undefined` — the flow re-registers / re-authorizes exactly as if nothing were stored. A `client_id` or `refresh_token` issued by one authorization server is therefore never sent to another.

**Round-trip the stored object unchanged and you're protected** — single-slot storage works. To hold credentials for several authorization servers at once, key your storage on `ctx.issuer` instead (and treat **`ctx === undefined` as "return the most-recently-saved token set"** — the transport's per-request `Authorization: Bearer` read (`adaptOAuthProvider().token()`) calls `tokens()` with no `ctx`).
Comment thread
felixweinberger marked this conversation as resolved.

Implement `discoveryState()` / `saveDiscoveryState()` so the callback leg can verify it is exchanging the authorization code at the same AS the redirect targeted; without it the SDK `console.warn`s once per callback. **`discoveryState` must persist with the same durability as `codeVerifier`** — it has to survive the redirect round-trip (page navigation, app restart). A provider that implements `saveDiscoveryState()` but cannot return it on the callback leg fails closed with `AuthorizationServerMismatchError`. Hosts that drive 401 handling themselves (custom middleware / `withOAuthRetry`) should call `provider.invalidateCredentials?.('discovery')` on a fresh 401 so a changed `authorization_servers` list is picked up on re-discovery. The built-in `StreamableHTTPClientTransport` / `SSEClientTransport` 401 path does not currently invalidate discovery state.

`OAuthClientProvider.saveAuthorizationServerUrl()` / `authorizationServerUrl()` are **deprecated** — `auth()` still calls `saveAuthorizationServerUrl()` for back-compat with providers that read it internally (Cross-App Access), but the SDK never reads `authorizationServerUrl()`. The call timing changed in v2: it was post-discovery, it is now post-`saveTokens` (after a successful token exchange). Read the `issuer` stamp on stored tokens, or `ctx.issuer`, instead.
Comment thread
felixweinberger marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The migration guide's SEP-2352 section refers hosts to withOAuthRetry, but that helper exists only in test/conformance/src/helpers/withOAuthRetry.ts and is not exported from @modelcontextprotocol/client — the published equivalents are withOAuth / handleOAuthUnauthorized. Reword the sentence to reference the actual exported middleware (or describe the pattern generically as "your own 401 handler") so consumers don't search the package for a symbol that doesn't exist.

Extended reasoning...

What the doc says vs. what the package ships. The new "Credentials are bound to the issuing authorization server (SEP-2352)" section in docs/migration.md (line 1644) reads: "Hosts that drive 401 handling themselves (custom middleware / withOAuthRetry) should call provider.invalidateCredentials?.('discovery') on a fresh 401 …". The backticked withOAuthRetry is presented as if it were an SDK symbol a host could reach for. It is not: the only definitions and uses of withOAuthRetry in the repository are test/conformance/src/helpers/withOAuthRetry.ts and its caller test/conformance/src/everythingClient.ts — internal conformance-fixture code that is never published.

What the package actually exports. packages/client/src/index.ts exports the middleware family applyMiddlewares, createMiddleware, withLogging, and withOAuth (from ./client/middleware.js), plus handleOAuthUnauthorized / adaptOAuthProvider in auth.ts. There is no withOAuthRetry anywhere in packages/*/src, in examples/, or elsewhere in docs/ — the new migration.md sentence added by this PR is the only documentation reference to it.

Concrete walk-through of the consumer impact. (1) A host author migrating to v2 reads the SEP-2352 section to learn their new obligations. (2) They drive 401 handling themselves and want to follow the guidance, so they search @modelcontextprotocol/client (or its typings) for withOAuthRetry to see how that middleware integrates the invalidateCredentials('discovery') call. (3) The symbol does not exist in the published package; the closest names are withOAuth and handleOAuthUnauthorized, neither of which performs the discovery invalidation (a separate, already-reported behavioral gap). (4) Best case the reader is confused and loses time; worst case they conclude the SDK ships a retry middleware that already handles the migration trigger and skip implementing the invalidation themselves.

Why nothing catches it. Documentation prose is not type-checked or linted against the export surface, so a test-fixture name leaking into the migration guide is silent. The conformance fixture is even where this PR added the very invalidateCredentials('discovery') call the sentence describes, which is presumably how the name ended up in the prose.

Distinct from the existing inline comment on auth.ts:1133. That comment is about the behavioral gap — the SDK's own withOAuth / handleOAuthUnauthorized 401 path never invalidates discovery, so SDK-driven consumers cannot complete an AS migration. This finding is purely about the wording: even if that behavioral gap is accepted and documented as-is, the sentence still names a non-existent public symbol.

How to fix. A one-word edit: replace withOAuthRetry with the exported withOAuth / handleOAuthUnauthorized, or phrase it generically — e.g. "Hosts that drive 401 handling themselves (custom middleware or their own 401 handler) should call provider.invalidateCredentials?.('discovery') on a fresh 401 …". Documentation-only, hence nit severity.


The bundled `ClientCredentialsProvider`, `PrivateKeyJwtProvider`, `StaticPrivateKeyJwtProvider`, and `CrossAppAccessProvider` gain an `expectedIssuer` option that stamps the constructor-supplied credential; when set, `auth()` against any other authorization server fails before the credential leaves the process. These providers no longer define `saveClientInformation()`.

### Conformance obligations for `OAuthClientProvider` implementers

<!-- Filled in as the SEP-2352/2350/837/2207 behavior PRs land. -->

#### SEP-2352 — per-authorization-server credential isolation

**No code change required for the common case.** If your `saveTokens()` / `saveClientInformation()` persist the value passed to them verbatim and your `tokens()` / `clientInformation()` return it verbatim, the SDK-stamped `issuer` round-trips and the binding holds.

If you serialise to a custom format, persist the `issuer` field alongside the rest of the value. If you key storage by `ctx.issuer`, return `undefined` for an issuer you have no entry for, and treat **`ctx === undefined` as "return the most-recently-saved token set"** — the transport's per-request `Authorization: Bearer` read (`adaptOAuthProvider().token()`) calls `tokens()` with no `ctx`.

## Using an LLM to migrate your code

An LLM-optimized version of this guide is available at [`docs/migration-SKILL.md`](migration-SKILL.md). It contains dense mapping tables designed for tools like Claude Code to mechanically apply all the changes described above. You can paste it into your LLM context or load it as
Expand Down
35 changes: 32 additions & 3 deletions examples/oauth/simpleOAuthClientProvider.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
import type { OAuthClientInformationMixed, OAuthClientMetadata, OAuthClientProvider, OAuthTokens } from '@modelcontextprotocol/client';
import type {
OAuthClientInformationMixed,
OAuthClientMetadata,
OAuthClientProvider,
OAuthDiscoveryState,
OAuthTokens
} from '@modelcontextprotocol/client';
import { validateClientMetadataUrl } from '@modelcontextprotocol/client';

/**
* In-memory OAuth client provider for demonstration purposes
* In production, you should persist tokens securely
* In-memory OAuth client provider for demonstration purposes.
* In production, you should persist tokens and client credentials securely.
*
* Tokens and client credentials are stored as single-slot blobs. The SDK stamps an
* `issuer` field onto every value it saves; round-tripping the blob unchanged means
* a credential issued by one authorization server is never reused at another (the
* SDK reads the stamp back as a key-not-found and re-registers / re-authorizes).
* To hold credentials for several authorization servers at once, key your storage
* on the `ctx.issuer` argument instead.
*/
export class InMemoryOAuthClientProvider implements OAuthClientProvider {
private _clientInformation?: OAuthClientInformationMixed;
private _tokens?: OAuthTokens;
private _codeVerifier?: string;
private _discoveryState?: OAuthDiscoveryState;

constructor(
private readonly _redirectUrl: string | URL,
Expand Down Expand Up @@ -66,4 +80,19 @@ export class InMemoryOAuthClientProvider implements OAuthClientProvider {
}
return this._codeVerifier;
}

saveDiscoveryState(state: OAuthDiscoveryState): void {
this._discoveryState = state;
}

discoveryState(): OAuthDiscoveryState | undefined {
return this._discoveryState;
}

invalidateCredentials(scope: 'all' | 'client' | 'tokens' | 'verifier' | 'discovery'): void {
if (scope === 'all' || scope === 'client') this._clientInformation = undefined;
if (scope === 'all' || scope === 'tokens') this._tokens = undefined;
if (scope === 'all' || scope === 'verifier') this._codeVerifier = undefined;
if (scope === 'all' || scope === 'discovery') this._discoveryState = undefined;
}
}
Loading
Loading