feat(client): SEP-2352 per-authorization-server credential isolation

.changeset/auth-iss-server-and-overload.md

@@ -0,0 +1,6 @@
+---
+"@modelcontextprotocol/client": minor
+"@modelcontextprotocol/server-legacy": minor
+---
+
+SEP-2468 follow-up: `transport.finishAuth()` gains a `URLSearchParams` overload (preferred) that extracts `code`/`iss`, validates `iss` first, and on mismatch throws a sanitized `IssuerMismatchError` (no callback `error_description` text); callers remain responsible for `state`. **Behavior change for `@modelcontextprotocol/server-legacy`:** `mcpAuthRouter` now advertises `authorization_response_iss_parameter_supported` (default `true`; `ProxyOAuthServerProvider` reports `false`) and the bundled authorize handler appends `iss` (RFC 9207) to every `res.redirect(...)` your `OAuthServerProvider.authorize()` issues to the client's `redirect_uri`. If your provider redirects another way (`res.writeHead`, a separate consent-page response, or a standalone `authorizationHandler({provider})` without `issuerUrl`), append `params.issuer` as `iss` yourself or set `authorizationResponseIssParameterSupported: false` — otherwise RFC 9207-compliant clients (including this SDK) will reject the callback.

docs/migration.md

@@ -1528,16 +1528,19 @@
 
 `transport.finishAuth()` and `auth()` now validate the `iss` parameter from the authorization callback against the issuer recorded from the authorization server's validated metadata (RFC 9207). A **mismatched** `iss` is rejected with `IssuerMismatchError` before the code is exchanged regardless of what the AS advertised; a **missing** `iss` is rejected only when the AS advertised `authorization_response_iss_parameter_supported: true`.
 
-**You must** pass the callback URL's query parameters to the SDK so it can read `iss` alongside `code`:
+**You must** pass the callback URL's query parameters to the SDK so it can read `iss` alongside `code`. The SDK does **not** validate `state`; compare it to your stored value before calling `finishAuth`:
 
 ```typescript
-const url = new URL(callbackUrl);
-await transport.finishAuth(url.searchParams); // SDK reads `code` + `iss`
+const params = new URL(callbackUrl).searchParams;
+if (params.get('state') !== expectedState) throw new Error('state mismatch');
+await transport.finishAuth(params); // SDK reads `code` + `iss`
 ```
 
 `transport.finishAuth(code, iss)` remains supported for back-compat. If you bypass `auth()` and call `exchangeAuthorization()` / `fetchToken()` directly, pass `iss` in the options bag — the same validation runs there.
 
-**You must not** display or act on `error`, `error_description`, or `error_uri` from the callback URL when `IssuerMismatchError` is thrown — those values are attacker-controlled in a mix-up attack.
+**You must not** display or act on `error`, `error_description`, or `error_uri` from the callback URL when `IssuerMismatchError` is thrown — those values are attacker-controlled in a mix-up attack. The `URLSearchParams` overload handles this for you; if you parse the callback yourself, suppress them.
+
+_(`@modelcontextprotocol/server-legacy` AS implementers — **behavior change**)_ `mcpAuthRouter()` now advertises `authorization_response_iss_parameter_supported` (default `true`) and the bundled authorize handler appends `iss` to **every** redirect — success or error — that your `OAuthServerProvider.authorize()` issues to the client's `redirect_uri` **via `res.redirect(...)` on the supplied `res`**. No provider change is required when that is how you redirect. If you emit the `Location` header another way (e.g. `res.writeHead(302, { Location })`), issue the final callback redirect from a different response (e.g. after a separate consent-page POST), or wire a standalone `authorizationHandler({provider})` without `issuerUrl`, append `params.issuer` as `iss` yourself — otherwise RFC 9207-compliant clients (including this SDK's) will reject the callback with `IssuerMismatchError`. If the callback is issued by an upstream AS you proxy to, set `authorizationResponseIssParameterSupported = false` on your provider (`ProxyOAuthServerProvider` does this) so the metadata does not over-claim.
 
 `discoverAuthorizationServerMetadata()` now rejects metadata whose `issuer` does not exactly match the URL it was fetched for (RFC 8414 §3.3). If you connect to a known-misconfigured AS, set `skipIssuerMetadataValidation: true` on `StreamableHTTPClientTransportOptions` / `SSEClientTransportOptions` (or on `AuthOptions` if you call `auth()` directly, or `skipIssuerValidation: true` on the low-level helper) — **this weakens the mix-up defense and should be treated as a temporary workaround.** It suppresses only the metadata-echo check; the callback-`iss` validation always runs (and degrades to a no-op only when `iss` is absent and the AS does not advertise support).
 

examples/shared/src/authServer.ts

@@ -132,6 +132,33 @@ export function setupAuthServer(options: SetupAuthServerOptions): void {
     // toNodeHandler bypasses Express methods
     const betterAuthHandler = toNodeHandler(auth);
 
+    // The issuer identifier this AS publishes in its metadata; must exactly match the
+    // `issuer` value better-auth emits at /.well-known/oauth-authorization-server.
+    const issuer = authServerUrl.toString().replace(/\/$/, '');
+    const issuerOrigin = new URL(issuer).origin;
+
+    // RFC 9207 (SEP-2468): append `iss` to every authorization-response redirect (success
+    // and error) that targets the client's redirect_uri. better-auth does not emit `iss`
+    // itself yet, so intercept the 302 Location header. Internal hops (to /sign-in or back
+    // to /api/auth/mcp/authorize) are left untouched.
+    authApp.use('/api/auth/mcp/authorize', (_req: Request, res: ExpressResponse, next: NextFunction) => {
+        const originalWriteHead = res.writeHead.bind(res);
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        res.writeHead = function (statusCode: number, ...args: any[]) {
+            const headers = args.find(a => typeof a === 'object' && a !== null) as Record<string, string> | undefined;
+            const loc = headers?.location ?? headers?.Location ?? (res.getHeader('Location') as string | undefined);
+            if (statusCode >= 300 && statusCode < 400 && loc && !loc.startsWith('/') && new URL(loc).origin !== issuerOrigin) {
+                const u = new URL(loc);
+                u.searchParams.set('iss', issuer);
+                if (headers && 'location' in headers) headers.location = u.href;
+                else if (headers && 'Location' in headers) headers.Location = u.href;
+                else res.setHeader('Location', u.href);
+            }
+            return originalWriteHead(statusCode, ...args);
+        } as typeof res.writeHead;
+        next();
+    });
+
     // DEMO ONLY: simulate the user clicking "Approve" on the consent screen.
     // The SDK auth driver appends `prompt=consent` whenever it requests the
     // `offline_access` scope (per OIDC §11). With a real user, better-auth
@@ -207,7 +234,15 @@ export function setupAuthServer(options: SetupAuthServerOptions): void {
     // OAuth metadata endpoints using better-auth's built-in handlers
     // Add explicit OPTIONS handler for CORS preflight
     authApp.options('/.well-known/oauth-authorization-server', cors());
-    authApp.get('/.well-known/oauth-authorization-server', cors(), toNodeHandler(oAuthDiscoveryMetadata(auth)));
+    // Wrap better-auth's metadata to advertise RFC 9207 support (the `iss` middleware
+    // above makes that claim true).
+    const discoveryHandler = oAuthDiscoveryMetadata(auth);
+    authApp.get('/.well-known/oauth-authorization-server', cors(), async (req: Request, res: ExpressResponse) => {
+        const upstream = await discoveryHandler(new Request(new URL(req.originalUrl, issuer)));
+        const body = (await upstream.json()) as Record<string, unknown>;
+        body.authorization_response_iss_parameter_supported = true;
+        res.status(upstream.status).json(body);
+    });
 
     // Body parsers for non-better-auth routes (like /sign-in)
     authApp.use(express.json());

packages/client/src/client/auth.ts

@@ -540,6 +540,69 @@
     return false;
 }
 
+/**
+ * Shared `finishAuth` resolver for the `(code, iss?)` and `(URLSearchParams)` overloads.
+ *
+ * For the `URLSearchParams` form, only `iss` and `code` are read up front. When a `code` is
+ * present the returned values flow into {@linkcode auth}, which runs
+ * {@linkcode validateAuthorizationResponseIssuer} against freshly-discovered metadata before
+ * the code is redeemed — so on mismatch the thrown {@linkcode IssuerMismatchError} carries no
+ * `error`/`error_description`/`error_uri` text from the callback (those are attacker-controlled
+ * in a mix-up). When no `code` is present (an error-shaped callback), `iss` is validated here
+ * against the provider's recorded discovery state — or, when the provider does not implement
+ * `discoveryState`, against freshly-discovered metadata mirroring what {@linkcode auth} does on
+ * the code-present path — **before** the callback's error parameters are read; only after that
+ * passes are they surfaced as an {@linkcode OAuthError}. When no issuer baseline can be
+ * obtained either way, a generic {@linkcode UnauthorizedError} is thrown without surfacing the
+ * callback's `error`/`error_description`/`error_uri`.
+ *
+ * @internal Exported for the transport `finishAuth` overloads; not part of the public barrel.
+ */
+export async function resolveAuthorizationCallbackParams(
+    codeOrParams: string | URLSearchParams,
+    iss: string | undefined,
+    provider: OAuthClientProvider,
+    serverUrl: string | URL,
+    opts?: { fetchFn?: FetchLike; resourceMetadataUrl?: URL }
+): Promise<{ authorizationCode: string; iss: string | undefined }> {
+    if (typeof codeOrParams === 'string') {
+        return { authorizationCode: codeOrParams, iss };
+    }
+    const issParam = codeOrParams.get('iss') ?? undefined;
+    const code = codeOrParams.get('code');
+    if (code) {
+        return { authorizationCode: code, iss: issParam };
+    }
+    // No code → error response. Gate the (potentially attacker-supplied) error params on the
+    // issuer first. Prefer the provider's recorded discovery state; when absent, mirror auth()'s
+    // code-present path and run a fresh discovery so the iss gate has an authentic baseline.
+    const discoveryState = await provider.discoveryState?.();
+    let metadata = discoveryState?.authorizationServerMetadata;
+    if (!metadata) {
+        try {
+            const serverInfo = await discoverOAuthServerInfo(serverUrl, opts);
+            metadata = serverInfo.authorizationServerMetadata;
+        } catch {
+            metadata = undefined;
+        }
+    }
+    if (!metadata) {
+        // No authentic baseline → cannot prove the error params came from our AS. Do NOT surface
+        // attacker-controllable `error`/`error_description`/`error_uri` here.
+        throw new UnauthorizedError('Authorization callback failed and the issuer could not be verified');
+    }
+    validateAuthorizationResponseIssuer({
+        iss: issParam,
+        expectedIssuer: metadata.issuer,
+        issParameterSupported: isIssParameterSupported(metadata)
+    });
+    const error = codeOrParams.get('error');
+    if (error) {
+        throw new OAuthError(error, codeOrParams.get('error_description') ?? error, codeOrParams.get('error_uri') ?? undefined);
+    }
+    throw new UnauthorizedError('Authorization callback contained neither `code` nor `error`');
+}
+
 export type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none';
 
 function isClientAuthMethod(method: string): method is ClientAuthMethod {

packages/client/src/client/sse.ts

@@ -11,7 +11,16 @@ import type { ErrorEvent, EventSourceInit } from 'eventsource';
 import { EventSource } from 'eventsource';
 
 import type { AuthProvider, OAuthClientProvider } from './auth.js';
-import { adaptOAuthProvider, auth, extractWWWAuthenticateParams, isOAuthClientProvider, UnauthorizedError } from './auth.js';
+import {
+    adaptOAuthProvider,
+    auth,
+    extractWWWAuthenticateParams,
+    isOAuthClientProvider,
+    resolveAuthorizationCallbackParams,
+    UnauthorizedError
+} from './auth.js';
+// eslint-disable-next-line @typescript-eslint/no-unused-vars -- referenced in JSDoc {@linkcode}
+import type { IssuerMismatchError } from './authErrors.js';
 
 export class SseError extends Error {
     constructor(
@@ -242,10 +251,15 @@ export class SSEClientTransport implements Transport {
     /**
      * Call this method after the user has finished authorizing via their user agent and is redirected back to the MCP client application. This will exchange the authorization code for an access token, enabling the next connection attempt to successfully auth.
      *
-     * Prefer passing the callback URL's `searchParams` directly — the SDK extracts
-     * `code` and `iss` (and validates `iss` per RFC 9207) for you. The `(code, iss?)`
+     * **Preferred:** pass the callback URL's `searchParams` directly. The SDK extracts `code`
+     * and `iss`, validates `iss` against the recorded issuer (RFC 9207) **before** reading any
+     * other parameter, and on mismatch throws an {@linkcode IssuerMismatchError} that carries
+     * none of the callback's `error`/`error_description`/`error_uri` text. The `(code, iss?)`
      * positional form remains supported for back-compat.
      *
+     * The SDK does **not** validate `state`; compare it to your stored value before calling
+     * `finishAuth`.
+     *
      * @param callbackParams - The `URLSearchParams` from the authorization callback URL
      *   (e.g. `new URL(callbackUrl).searchParams`). `code` and `iss` are read from it.
      */
@@ -261,22 +275,18 @@ export class SSEClientTransport implements Transport {
             throw new UnauthorizedError('finishAuth requires an OAuthClientProvider');
         }
 
-        let authorizationCode: string;
-        if (codeOrParams instanceof URLSearchParams) {
-            const code = codeOrParams.get('code');
-            if (!code) {
-                throw new UnauthorizedError('Authorization callback is missing the "code" parameter');
-            }
-            authorizationCode = code;
-            iss = codeOrParams.get('iss') ?? undefined;
-        } else {
-            authorizationCode = codeOrParams;
-        }
+        const { authorizationCode, iss: issParam } = await resolveAuthorizationCallbackParams(
+            codeOrParams,
+            iss,
+            this._oauthProvider,
+            this._url,
+            { fetchFn: this._fetchWithInit, resourceMetadataUrl: this._resourceMetadataUrl }
+        );
 
         const result = await auth(this._oauthProvider, {
             serverUrl: this._url,
             authorizationCode,
-            iss,
+            iss: issParam,
             resourceMetadataUrl: this._resourceMetadataUrl,
             scope: this._scope,
             fetchFn: this._fetchWithInit,

packages/client/src/client/streamableHttp.ts

@@ -26,6 +26,7 @@ import {
     extractWWWAuthenticateParams,
     isOAuthClientProvider,
     isStrictScopeSuperset,
+    resolveAuthorizationCallbackParams,
     UnauthorizedError
 } from './auth.js';
 // eslint-disable-next-line @typescript-eslint/no-unused-vars -- referenced via {@linkcode} in finishAuth JSDoc
@@ -829,10 +830,16 @@ export class StreamableHTTPClientTransport implements Transport {
     /**
      * Call this method after the user has finished authorizing via their user agent and is redirected back to the MCP client application. This will exchange the authorization code for an access token, enabling the next connection attempt to successfully auth.
      *
-     * Prefer passing the callback URL's `searchParams` directly — the SDK extracts
-     * `code` and `iss` (and validates `iss` per RFC 9207) for you. The `(code, iss?)`
+     * **Preferred:** pass the callback URL's `searchParams` directly. The SDK extracts `code`
+     * and `iss`, validates `iss` against the recorded issuer (RFC 9207) **before** reading any
+     * other parameter, and on mismatch throws an {@linkcode IssuerMismatchError} that carries
+     * none of the callback's `error`/`error_description`/`error_uri` text — those are
+     * attacker-controlled in a mix-up attack and MUST NOT be displayed. The `(code, iss?)`
      * positional form remains supported for back-compat.
      *
+     * The SDK does **not** validate `state`; compare it to your stored value before calling
+     * `finishAuth`.
+     *
      * @param callbackParams - The `URLSearchParams` from the authorization callback URL
      *   (e.g. `new URL(callbackUrl).searchParams`). `code` and `iss` are read from it.
      */
@@ -850,22 +857,18 @@ export class StreamableHTTPClientTransport implements Transport {
             throw new UnauthorizedError('finishAuth requires an OAuthClientProvider');
         }
 
-        let authorizationCode: string;
-        if (codeOrParams instanceof URLSearchParams) {
-            const code = codeOrParams.get('code');
-            if (!code) {
-                throw new UnauthorizedError('Authorization callback is missing the "code" parameter');
-            }
-            authorizationCode = code;
-            iss = codeOrParams.get('iss') ?? undefined;
-        } else {
-            authorizationCode = codeOrParams;
-        }
+        const { authorizationCode, iss: issParam } = await resolveAuthorizationCallbackParams(
+            codeOrParams,
+            iss,
+            this._oauthProvider,
+            this._url,
+            { fetchFn: this._fetchWithInit, resourceMetadataUrl: this._resourceMetadataUrl }
+        );
 
         const result = await auth(this._oauthProvider, {
             serverUrl: this._url,
             authorizationCode,
-            iss,
+            iss: issParam,
             resourceMetadataUrl: this._resourceMetadataUrl,
             scope: this._scope,
             fetchFn: this._fetchWithInit,