From 5efd11c9854e184d555ca8600d634306e1fe81ff Mon Sep 17 00:00:00 2001 From: Felix Weinberger Date: Fri, 19 Jun 2026 17:11:14 +0000 Subject: [PATCH] feat(client,server-legacy): SEP-2468 server iss emission + finishAuth(URLSearchParams) overload Claude-Session: https://claude.ai/code/session_01XBib5gRe8AMPPJhySCz3EJ --- .changeset/auth-iss-server-and-overload.md | 6 ++ docs/migration.md | 11 ++- examples/shared/src/authServer.ts | 37 +++++++++- packages/client/src/client/auth.ts | 63 ++++++++++++++++ packages/client/src/client/sse.ts | 40 ++++++---- packages/client/src/client/streamableHttp.ts | 31 ++++---- packages/client/test/client/auth.test.ts | 41 ++++++++++ .../src/auth/handlers/authorize.ts | 70 +++++++++++++++--- packages/server-legacy/src/auth/provider.ts | 27 +++++++ .../src/auth/providers/proxyProvider.ts | 11 +++ packages/server-legacy/src/auth/router.ts | 13 +++- .../test/auth/handlers/authorize.test.ts | 74 +++++++++++++++++++ .../test/auth/providers/proxyProvider.test.ts | 8 ++ .../server-legacy/test/auth/router.test.ts | 16 ++++ test/e2e/helpers/index.ts | 6 ++ test/e2e/requirements.ts | 16 ++++ test/e2e/scenarios/client-auth.test.ts | 56 +++++++++++--- test/e2e/scenarios/hosting-express.test.ts | 67 +++++++++++++++++ test/e2e/scenarios/transport-http.test.ts | 8 +- test/e2e/scenarios/transport-raw.test.ts | 8 +- test/e2e/tsconfig.json | 1 + 21 files changed, 539 insertions(+), 71 deletions(-) create mode 100644 .changeset/auth-iss-server-and-overload.md diff --git a/.changeset/auth-iss-server-and-overload.md b/.changeset/auth-iss-server-and-overload.md new file mode 100644 index 0000000000..af376f4c8f --- /dev/null +++ b/.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. diff --git a/docs/migration.md b/docs/migration.md index c1695c364c..b543911b95 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1601,16 +1601,19 @@ New TypeScript-only aliases `StoredOAuthTokens` and `StoredOAuthClientInformatio `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). diff --git a/examples/shared/src/authServer.ts b/examples/shared/src/authServer.ts index 5f895087e0..dc7d602cee 100644 --- a/examples/shared/src/authServer.ts +++ b/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 | 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; + 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()); diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 950075946b..2c1169f238 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -540,6 +540,69 @@ export function isStrictScopeSuperset(union: string | undefined, current: string 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 { diff --git a/packages/client/src/client/sse.ts b/packages/client/src/client/sse.ts index 70ba6d6d56..af7a1276ba 100644 --- a/packages/client/src/client/sse.ts +++ b/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, diff --git a/packages/client/src/client/streamableHttp.ts b/packages/client/src/client/streamableHttp.ts index b7760f4036..b3b8da915d 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/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, diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index f90f4183ec..d92724f511 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -23,9 +23,11 @@ import { refreshAuthorization, registerClient, RegistrationRejectedError, + resolveAuthorizationCallbackParams, resolveClientMetadata, selectClientAuthMethod, startAuthorization, + UnauthorizedError, validateAuthorizationResponseIssuer, validateClientMetadataUrl } from '../../src/client/auth.js'; @@ -1261,6 +1263,45 @@ describe('OAuth Authorization', () => { }); }); + describe('resolveAuthorizationCallbackParams', () => { + const issuer = 'https://auth.example.com'; + const provider = { + discoveryState: async () => ({ + authorizationServerMetadata: { + issuer, + authorization_endpoint: `${issuer}/authorize`, + token_endpoint: `${issuer}/token`, + response_types_supported: ['code'], + authorization_response_iss_parameter_supported: true + } + }) + } as unknown as OAuthClientProvider; + + it('treats an empty ?code= as no-code (falls through to the error/neither diagnostic)', async () => { + // URLSearchParams.get('code') returns '' (not null) for `?code=`, so a `!== null` + // check would have POSTed `code=` to the token endpoint and lost the explicit + // diagnostic. The truthy check restores the pre-PR behavior. + await expect( + resolveAuthorizationCallbackParams(new URLSearchParams(`code=&state=x&iss=${issuer}`), undefined, provider, issuer) + ).rejects.toThrow(UnauthorizedError); + // With an `error` param present, surfaces the gated OAuthError instead. + await expect( + resolveAuthorizationCallbackParams( + new URLSearchParams(`code=&error=access_denied&iss=${issuer}`), + undefined, + provider, + issuer + ) + ).rejects.toThrow(OAuthError); + }); + + it('returns {authorizationCode, iss} when a non-empty code is present', async () => { + await expect( + resolveAuthorizationCallbackParams(new URLSearchParams(`code=abc&iss=${issuer}`), undefined, provider, issuer) + ).resolves.toEqual({ authorizationCode: 'abc', iss: issuer }); + }); + }); + describe('discoverOAuthServerInfo', () => { const validResourceMetadata = { resource: 'https://resource.example.com', diff --git a/packages/server-legacy/src/auth/handlers/authorize.ts b/packages/server-legacy/src/auth/handlers/authorize.ts index 15dfe3cb90..adbfbfa053 100644 --- a/packages/server-legacy/src/auth/handlers/authorize.ts +++ b/packages/server-legacy/src/auth/handlers/authorize.ts @@ -1,4 +1,4 @@ -import type { RequestHandler } from 'express'; +import type { RequestHandler, Response } from 'express'; import express from 'express'; import type { Options as RateLimitOptions } from 'express-rate-limit'; import { rateLimit } from 'express-rate-limit'; @@ -6,10 +6,19 @@ import * as z from 'zod/v4'; import { InvalidClientError, InvalidRequestError, OAuthError, ServerError, TooManyRequestsError } from '../errors.js'; import { allowedMethods } from '../middleware/allowedMethods.js'; -import type { OAuthServerProvider } from '../provider.js'; +// eslint-disable-next-line @typescript-eslint/no-unused-vars -- AuthorizationParams referenced in JSDoc {@linkcode} +import type { AuthorizationParams, OAuthServerProvider } from '../provider.js'; export type AuthorizationHandlerOptions = { provider: OAuthServerProvider; + /** + * The authorization server's issuer identifier. When set, the handler appends it as the + * `iss` query parameter (RFC 9207) to any redirect — success or error — that targets the + * client's validated `redirect_uri`, and also supplies it to the provider as + * {@linkcode AuthorizationParams.issuer}. `mcpAuthRouter` always sets this from its + * `issuerUrl`. + */ + issuerUrl?: URL; /** * Rate limiting configuration for the authorization endpoint. * Set to false to disable rate limiting for this endpoint. @@ -69,7 +78,8 @@ const RequestAuthorizationParamsSchema = z.object({ resource: z.string().url().optional() }); -export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler { +export function authorizationHandler({ provider, issuerUrl, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler { + const issuer = issuerUrl?.href; // Create a router to apply middleware const router = express.Router(); router.use(allowedMethods(['GET', 'POST'])); @@ -158,7 +168,11 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A requestedScopes = scope.split(' '); } - // All validation passed, proceed with authorization + // All validation passed, proceed with authorization. RFC 9207: the metadata + // advertises `authorization_response_iss_parameter_supported`, so make that claim + // true from SDK code by appending `iss` to whatever redirect the provider issues + // back to the client's validated redirect_uri — the provider need not do anything. + // Redirects elsewhere (e.g. to an upstream authorize endpoint) are left untouched. await provider.authorize( client, { @@ -166,17 +180,18 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A scopes: requestedScopes, redirectUri: redirect_uri!, codeChallenge: code_challenge, - resource: resource ? new URL(resource) : undefined + resource: resource ? new URL(resource) : undefined, + issuer }, - res + issuer ? withIssOnCallbackRedirect(res, redirect_uri!, issuer) : res ); } catch (error) { // Post-redirect errors - redirect with error parameters if (error instanceof OAuthError) { - res.redirect(302, createErrorRedirect(redirect_uri!, error, state)); + res.redirect(302, createErrorRedirect(redirect_uri!, error, state, issuer)); } else { const serverError = new ServerError('Internal Server Error'); - res.redirect(302, createErrorRedirect(redirect_uri!, serverError, state)); + res.redirect(302, createErrorRedirect(redirect_uri!, serverError, state, issuer)); } } }); @@ -184,10 +199,43 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A return router; } +/** + * Wraps `res.redirect` so that when the provider redirects to the client's validated + * `redirect_uri` (i.e. the OAuth authorization response), `iss` is appended per RFC 9207. + * Only redirects whose origin and path match `redirectUri` are touched; an `iss` already + * set by the provider is preserved. This is what backs the + * `authorization_response_iss_parameter_supported: true` metadata claim without requiring + * `OAuthServerProvider.authorize()` implementations to change. + */ +function withIssOnCallbackRedirect(res: Response, redirectUri: string, issuer: string): Response { + const cb = new URL(redirectUri); + const appendIss = (url: string): string => { + let target: URL; + try { + target = new URL(url); + } catch { + return url; + } + if (target.origin === cb.origin && target.pathname === cb.pathname && !target.searchParams.has('iss')) { + target.searchParams.set('iss', issuer); + return target.href; + } + return url; + }; + const original = res.redirect.bind(res) as (...args: unknown[]) => void; + res.redirect = ((statusOrUrl: number | string, maybeUrl?: string | number): void => { + if (typeof statusOrUrl === 'number') original(statusOrUrl, appendIss(String(maybeUrl))); + // Express 4 still accepts the deprecated reversed form `res.redirect(url, status)`. + else if (typeof maybeUrl === 'number') original(appendIss(statusOrUrl), maybeUrl); + else original(appendIss(statusOrUrl)); + }) as Response['redirect']; + return res; +} + /** * Helper function to create redirect URL with error parameters */ -function createErrorRedirect(redirectUri: string, error: OAuthError, state?: string): string { +function createErrorRedirect(redirectUri: string, error: OAuthError, state?: string, issuer?: string): string { const errorUrl = new URL(redirectUri); errorUrl.searchParams.set('error', error.errorCode); errorUrl.searchParams.set('error_description', error.message); @@ -197,5 +245,9 @@ function createErrorRedirect(redirectUri: string, error: OAuthError, state?: str if (state) { errorUrl.searchParams.set('state', state); } + if (issuer) { + // RFC 9207 §2: the iss parameter is required on error responses too. + errorUrl.searchParams.set('iss', issuer); + } return errorUrl.href; } diff --git a/packages/server-legacy/src/auth/provider.ts b/packages/server-legacy/src/auth/provider.ts index 528e8d27bc..e197491025 100644 --- a/packages/server-legacy/src/auth/provider.ts +++ b/packages/server-legacy/src/auth/provider.ts @@ -10,6 +10,15 @@ export type AuthorizationParams = { codeChallenge: string; redirectUri: string; resource?: URL; + /** + * The authorization server's own issuer identifier (the `issuerUrl` configured on + * `mcpAuthRouter`). Informational: the bundled `authorizationHandler` already appends + * this as the `iss` query parameter (RFC 9207 §2) to any `res.redirect(...)` your + * `authorize()` issues to {@linkcode AuthorizationParams.redirectUri | redirectUri}. You + * only need to append it yourself when the final callback redirect is issued from a + * different response (e.g. after a separate consent-page POST). + */ + issuer?: string; }; /** @@ -27,6 +36,14 @@ export interface OAuthServerProvider { * This server must eventually issue a redirect with an authorization response or an error response to the given redirect URI. Per OAuth 2.1: * - In the successful case, the redirect MUST include the `code` and `state` (if present) query parameters. * - In the error case, the redirect MUST include the `error` query parameter, and MAY include an optional `error_description` query parameter. + * + * RFC 9207: the bundled `authorizationHandler` appends `iss` **only** to `res.redirect(...)` calls you issue + * on the supplied `res` to `params.redirectUri`, so an implementation that redirects that way requires no + * change. If you emit the `Location` header another way (e.g. `res.writeHead(302, { Location: ... })`), or + * issue the final callback redirect from a different response (e.g. after a separate consent step), append + * {@linkcode AuthorizationParams.issuer | params.issuer} as `iss` yourself, or set + * {@linkcode OAuthServerProvider.authorizationResponseIssParameterSupported} to `false` so the metadata does + * not over-claim. */ authorize(client: OAuthClientInformationFull, params: AuthorizationParams, res: Response): Promise; @@ -63,6 +80,16 @@ export interface OAuthServerProvider { */ revokeToken?(client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest): Promise; + /** + * Whether this provider's authorization responses carry the RFC 9207 `iss` parameter. + * Drives the `authorization_response_iss_parameter_supported` metadata field. Defaults to + * `true` — the bundled `authorizationHandler` appends `iss` to redirects it issues to the + * client's `redirect_uri`. Set to `false` when the callback is issued by an upstream + * authorization server this provider delegates to (e.g. `ProxyOAuthServerProvider`), so the + * published metadata does not over-claim support. + */ + authorizationResponseIssParameterSupported?: boolean; + /** * Whether to skip local PKCE validation. * diff --git a/packages/server-legacy/src/auth/providers/proxyProvider.ts b/packages/server-legacy/src/auth/providers/proxyProvider.ts index b469ce6df0..495eab3c67 100644 --- a/packages/server-legacy/src/auth/providers/proxyProvider.ts +++ b/packages/server-legacy/src/auth/providers/proxyProvider.ts @@ -47,6 +47,17 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { skipLocalPkceValidation = true; + /** + * The proxy redirects the browser to the upstream AS's authorize endpoint with + * `redirect_uri = params.redirectUri`, so the upstream — not this proxy — issues the + * callback. The proxy cannot append its own `iss`, and any `iss` the upstream emits is the + * upstream's issuer, not `issuerUrl`. Advertise `false` so the metadata does not over-claim — + * a callback *without* `iss` then passes validation. Note: an upstream that *does* emit its + * own `iss` will still mismatch this proxy's issuer and be rejected by RFC 9207 clients + * regardless of this flag. + */ + authorizationResponseIssParameterSupported = false; + revokeToken?: (client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest) => Promise; constructor(options: ProxyOptions) { diff --git a/packages/server-legacy/src/auth/router.ts b/packages/server-legacy/src/auth/router.ts index ba8b030e08..02755cc7d7 100644 --- a/packages/server-legacy/src/auth/router.ts +++ b/packages/server-legacy/src/auth/router.ts @@ -61,7 +61,7 @@ export type AuthRouterOptions = { resourceServerUrl?: URL; // Individual options per route - authorizationOptions?: Omit; + authorizationOptions?: Omit; clientRegistrationOptions?: Omit; revocationOptions?: Omit; tokenOptions?: Omit; @@ -114,7 +114,14 @@ export const createOAuthMetadata = (options: { revocation_endpoint: revocation_endpoint ? new URL(revocation_endpoint, baseUrl || issuer).href : undefined, revocation_endpoint_auth_methods_supported: revocation_endpoint ? ['client_secret_post'] : undefined, - registration_endpoint: registration_endpoint ? new URL(registration_endpoint, baseUrl || issuer).href : undefined + registration_endpoint: registration_endpoint ? new URL(registration_endpoint, baseUrl || issuer).href : undefined, + + // RFC 9207: the bundled authorize handler appends `iss` to any redirect the provider + // issues via `res.redirect(...)` to the client's validated redirect_uri, so the default + // is `true`. Providers whose callback is issued by an upstream AS (e.g. the proxy + // provider) override this to `false` so we don't over-claim — SEP-2468 clients reject + // a callback that omits `iss` when support is advertised. + authorization_response_iss_parameter_supported: options.provider.authorizationResponseIssParameterSupported ?? true }; return metadata; @@ -139,7 +146,7 @@ export function mcpAuthRouter(options: AuthRouterOptions): RequestHandler { router.use( new URL(oauthMetadata.authorization_endpoint).pathname, - authorizationHandler({ provider: options.provider, ...options.authorizationOptions }) + authorizationHandler({ provider: options.provider, issuerUrl: options.issuerUrl, ...options.authorizationOptions }) ); router.use(new URL(oauthMetadata.token_endpoint).pathname, tokenHandler({ provider: options.provider, ...options.tokenOptions })); diff --git a/packages/server-legacy/test/auth/handlers/authorize.test.ts b/packages/server-legacy/test/auth/handlers/authorize.test.ts index cacecd8881..9a1edcff49 100644 --- a/packages/server-legacy/test/auth/handlers/authorize.test.ts +++ b/packages/server-legacy/test/auth/handlers/authorize.test.ts @@ -397,4 +397,78 @@ describe('Authorization Handler', () => { expect(location.searchParams.has('code')).toBe(true); }); }); + + describe('RFC 9207 iss parameter', () => { + const ISSUER = 'https://auth.example.com/'; + let issApp: express.Express; + + beforeEach(() => { + issApp = express(); + issApp.use('/authorize', authorizationHandler({ provider: mockProvider, issuerUrl: new URL(ISSUER) })); + }); + + it("appends iss to the provider's success redirect and supplies issuer to provider.authorize()", async () => { + // mockProvider.authorize() does NOT set `iss` itself — the handler must add it. + const spy = vi.spyOn(mockProvider, 'authorize'); + const response = await supertest(issApp).get('/authorize').query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + code_challenge: 'challenge123', + code_challenge_method: 'S256' + }); + expect(response.status).toBe(302); + const location = new URL(response.header.location!); + expect(location.searchParams.get('code')).toBe('mock_auth_code'); + expect(location.searchParams.get('iss')).toBe(ISSUER); + expect(spy).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ issuer: ISSUER }), expect.anything()); + spy.mockRestore(); + }); + + it('leaves redirects to non-callback targets untouched', async () => { + // A provider that hops to an upstream authorize endpoint (proxy pattern) — the + // handler must not append `iss` to that hop. + const upstream = 'https://upstream.example.com/authorize?client_id=x'; + const proxyLike: OAuthServerProvider = { + ...mockProvider, + async authorize(_client, _params, res) { + res.redirect(upstream); + } + }; + const proxyApp = express(); + proxyApp.use('/authorize', authorizationHandler({ provider: proxyLike, issuerUrl: new URL(ISSUER) })); + const response = await supertest(proxyApp).get('/authorize').query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + code_challenge: 'challenge123', + code_challenge_method: 'S256' + }); + expect(response.status).toBe(302); + expect(response.header.location).toBe(upstream); + }); + + it('appends iss to error redirects', async () => { + const response = await supertest(issApp).get('/authorize').query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'token' // invalid → error redirect + }); + expect(response.status).toBe(302); + const location = new URL(response.header.location!); + expect(location.searchParams.get('error')).toBe('invalid_request'); + expect(location.searchParams.get('iss')).toBe(ISSUER); + }); + + it('omits iss when issuerUrl is not configured', async () => { + const response = await supertest(app).get('/authorize').query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'token' + }); + expect(response.status).toBe(302); + const location = new URL(response.header.location!); + expect(location.searchParams.has('iss')).toBe(false); + }); + }); }); diff --git a/packages/server-legacy/test/auth/providers/proxyProvider.test.ts b/packages/server-legacy/test/auth/providers/proxyProvider.test.ts index 1214ef5a05..e261925be4 100644 --- a/packages/server-legacy/test/auth/providers/proxyProvider.test.ts +++ b/packages/server-legacy/test/auth/providers/proxyProvider.test.ts @@ -106,6 +106,14 @@ describe('Proxy OAuth Server Provider', () => { expect(mockResponse.redirect).toHaveBeenCalledWith(expectedUrl.toString()); }); + + it('reports authorizationResponseIssParameterSupported = false (upstream issues the callback)', () => { + // The proxy cannot guarantee its own `iss` on the callback, so the metadata flag + // derived from the provider must be false — otherwise RFC 9207 clients reject any + // callback that arrives *without* `iss`. (A present upstream `iss` still mismatches + // the proxy issuer regardless of this flag.) + expect(provider.authorizationResponseIssParameterSupported).toBe(false); + }); }); describe('token exchange', () => { diff --git a/packages/server-legacy/test/auth/router.test.ts b/packages/server-legacy/test/auth/router.test.ts index f4c4472d08..b29f7509ad 100644 --- a/packages/server-legacy/test/auth/router.test.ts +++ b/packages/server-legacy/test/auth/router.test.ts @@ -218,6 +218,22 @@ describe('MCP Auth Router', () => { // Verify optional fields expect(response.body.service_documentation).toBe('https://docs.example.com/'); + + // RFC 9207: the bundled authorize handler emits `iss`, so the metadata advertises it. + expect(response.body.authorization_response_iss_parameter_supported).toBe(true); + }); + + it('derives authorization_response_iss_parameter_supported from the provider', async () => { + const optOutApp = express(); + optOutApp.use( + mcpAuthRouter({ + provider: { ...mockProvider, authorizationResponseIssParameterSupported: false }, + issuerUrl: new URL('https://auth.example.com') + }) + ); + const response = await supertest(optOutApp).get('/.well-known/oauth-authorization-server'); + expect(response.status).toBe(200); + expect(response.body.authorization_response_iss_parameter_supported).toBe(false); }); it('returns minimal metadata for minimal router', async () => { diff --git a/test/e2e/helpers/index.ts b/test/e2e/helpers/index.ts index 46e23f86ae..68935b8f2d 100644 --- a/test/e2e/helpers/index.ts +++ b/test/e2e/helpers/index.ts @@ -39,6 +39,12 @@ import { startLegacySseHost } from './sse-host.js'; import type { SnifferOptions } from './wire-sniffer.js'; import { sniffTransport } from './wire-sniffer.js'; +/** Narrows away `null`/`undefined` for values the surrounding test has already proven exist (replaces non-null assertions). */ +export function defined(value: T | null | undefined, label: string): NonNullable { + if (value === null || value === undefined) throw new Error(`expected ${label} to be defined`); + return value; +} + export type ServerFactory = () => McpServer | Server; /** diff --git a/test/e2e/requirements.ts b/test/e2e/requirements.ts index 229c924fff..97ccb04ac0 100644 --- a/test/e2e/requirements.ts +++ b/test/e2e/requirements.ts @@ -2263,6 +2263,22 @@ export const REQUIREMENTS: Record = { transports: ['streamableHttp'], note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.' }, + 'client-auth:finishauth:urlsearchparams-sanitizes': { + addedInSpecVersion: '2026-07-28', + source: 'https://modelcontextprotocol.io/specification/draft/basic/authorization#authorization-response-issuer-validation', + behavior: + "transport.finishAuth(URLSearchParams) extracts code and iss, validates iss against the recorded issuer first, and on mismatch throws IssuerMismatchError without surfacing the callback's error/error_description/error_uri values; the authorization code is never sent to a token endpoint.", + transports: ['streamableHttp'], + note: 'This exercises the HTTP hosting/auth layer and OAuth client; the matrix transport arg is ignored, so it runs as a single streamableHttp-labelled cell to avoid duplicate runs.' + }, + 'hosting:auth:as-iss-emission': { + addedInSpecVersion: '2026-07-28', + source: 'https://modelcontextprotocol.io/specification/draft/basic/authorization#authorization-response-issuer-validation', + behavior: + "The bundled authorization server (mcpAuthRouter from @modelcontextprotocol/server-legacy) advertises authorization_response_iss_parameter_supported (default true; derived from the provider) and its authorize handler appends iss (RFC 9207 §2) to every redirect — success and error — issued to the client's redirect_uri without requiring OAuthServerProvider.authorize() to do so.", + transports: ['streamableHttp'], + note: 'These exercise the HTTP hosting/auth layer (mostly over real Express); the matrix transport arg is ignored, so they run as a single streamableHttp-labelled cell to avoid duplicate runs.' + }, 'client-auth:prm-discovery:no-prm-fallback': { source: 'sdk', behavior: diff --git a/test/e2e/scenarios/client-auth.test.ts b/test/e2e/scenarios/client-auth.test.ts index cd4ee8e9cc..b2fa498fa1 100644 --- a/test/e2e/scenarios/client-auth.test.ts +++ b/test/e2e/scenarios/client-auth.test.ts @@ -53,7 +53,7 @@ import { importSPKI, jwtVerify } from 'jose'; import { expect, vi } from 'vitest'; import { z } from 'zod/v4'; -import { hostPerSession } from '../helpers/index.js'; +import { defined, hostPerSession } from '../helpers/index.js'; import { verifies } from '../helpers/verifies.js'; import type { TestArgs } from '../types.js'; @@ -61,12 +61,6 @@ const ISSUER = 'https://auth.example.com'; const MCP_URL = 'http://in-process/mcp'; const RESOURCE = 'http://in-process/mcp'; -// Narrows indexed-access results that the surrounding count assertions have already proven to exist. -function defined(value: T | undefined, label: string): T { - if (value === undefined) throw new Error(`Expected ${label} to be defined`); - return value; -} - interface MockASConfig { tokenResponses?: Array>; tokenErrorResponses?: Array<{ error: string; error_description?: string }>; @@ -743,10 +737,12 @@ verifies('client-auth:as-metadata-discovery:issuer-validation', async (_args: Te /** * Runs the redirect leg of the OAuth flow against a mock AS configured with `asMetadata`, then - * calls `transport.finishAuth(code, iss)`. Returns the thrown error (or undefined on success) - * and the recorded token-endpoint calls so the caller can assert whether the code went on the wire. + * calls `transport.finishAuth(...)`. When `callback` is a string (or undefined) it is passed as + * `finishAuth('granted-code', iss)`; when it is a `URLSearchParams` it is passed verbatim to the + * overload. Returns the thrown error (or undefined on success) and the recorded token-endpoint + * calls so the caller can assert whether the code went on the wire. */ -async function runFinishAuthScenario(asMetadata: Partial, iss: string | undefined) { +async function runFinishAuthScenario(asMetadata: Partial, callback: string | undefined | URLSearchParams) { const as = createMockAuthorizationServer({ asMetadata, tokenResponses: [{ access_token: 'iss-flow-token', token_type: 'Bearer' }] }); const provider = new RecordingOAuthClientProvider(); const mcpHost = createAuthenticatedHost('iss-flow-token'); @@ -761,7 +757,7 @@ async function runFinishAuthScenario(asMetadata: Partial { expect(as.tokenCalls).toHaveLength(0); }); +verifies('client-auth:finishauth:urlsearchparams-sanitizes', async (_args: TestArgs) => { + const ATTACKER_TEXT = 'ATTACKER_CONTROLLED_DO_NOT_DISPLAY'; + const ATTACKER_URI = 'https://attacker.example.com/phish'; + + // Mismatched-iss callback that ALSO carries error/error_description/error_uri — a mix-up + // attacker controls all of these. The overload must throw IssuerMismatchError before reading + // them, so none of the attacker text appears on the thrown error. + const mixed = await runFinishAuthScenario( + { authorization_response_iss_parameter_supported: true }, + new URLSearchParams({ + code: 'granted-code', + state: 'state-123', + iss: 'https://attacker.example.com', + error: 'server_error', + error_description: ATTACKER_TEXT, + error_uri: ATTACKER_URI + }) + ); + expect(mixed.thrown).toBeInstanceOf(IssuerMismatchError); + const err = mixed.thrown as IssuerMismatchError; + expect(err.kind).toBe('authorization_response'); + expect(err.message).not.toContain(ATTACKER_TEXT); + expect(err.message).not.toContain(ATTACKER_URI); + expect(JSON.stringify(err)).not.toContain(ATTACKER_TEXT); + // The poisoned code never reached a token endpoint. + expect(mixed.tokenCalls).toHaveLength(0); + + // Happy path: matching iss → SDK extracts code and redeems it. + const ok = await runFinishAuthScenario( + { authorization_response_iss_parameter_supported: true }, + new URLSearchParams({ code: 'granted-code', state: 'state-123', iss: ISSUER }) + ); + expect(ok.thrown).toBeUndefined(); + expect(ok.tokenCalls).toHaveLength(1); + expect(defined(ok.tokenCalls[0], 'token call').body.get('code')).toBe('granted-code'); + expect(ok.provider.saved.tokens?.access_token).toBe('iss-flow-token'); +}); + verifies('client-auth:bearer-header:every-request', async (_args: TestArgs) => { const validToken = 'bearer-test-token'; const provider = new RecordingOAuthClientProvider({ diff --git a/test/e2e/scenarios/hosting-express.test.ts b/test/e2e/scenarios/hosting-express.test.ts index f4a3141a78..85c106f75d 100644 --- a/test/e2e/scenarios/hosting-express.test.ts +++ b/test/e2e/scenarios/hosting-express.test.ts @@ -25,12 +25,15 @@ import { createMcpExpressApp, mcpAuthMetadataRouter, requireBearerAuth } from '@ import { NodeStreamableHTTPServerTransport } from '@modelcontextprotocol/node'; import type { OAuthMetadata } from '@modelcontextprotocol/server'; import { LATEST_PROTOCOL_VERSION, McpServer, OAuthError, OAuthErrorCode } from '@modelcontextprotocol/server'; +import type { AuthorizationParams, OAuthServerProvider } from '@modelcontextprotocol/server-legacy'; +import { mcpAuthRouter } from '@modelcontextprotocol/server-legacy'; import type { Express, RequestHandler } from 'express'; import express from 'express'; import { expect } from 'vitest'; import { z } from 'zod/v4'; import { startExpressMinimal, startExpressWithHostValidation } from '../helpers/express.js'; +import { defined } from '../helpers/index.js'; import { verifies } from '../helpers/verifies.js'; import type { TestArgs } from '../types.js'; @@ -387,6 +390,70 @@ verifies('hosting:auth:query-token-ignored', async (_args: TestArgs) => { expect(headerRes.status).toBe(200); }); +verifies('hosting:auth:as-iss-emission', async (_args: TestArgs) => { + // Minimal OAuthServerProvider whose authorize() issues a plain success redirect WITHOUT + // appending `iss` itself — the bundled handler must add it so the metadata claim is true + // without provider action. + const seenIssuer: Array = []; + const provider: OAuthServerProvider = { + clientsStore: { + // eslint-disable-next-line @typescript-eslint/require-await + async getClient(clientId) { + return clientId === 'demo-client' + ? { client_id: 'demo-client', redirect_uris: ['https://client.example.com/cb'] } + : undefined; + } + }, + // eslint-disable-next-line @typescript-eslint/require-await + async authorize(_client, params: AuthorizationParams, res) { + seenIssuer.push(params.issuer); + const u = new URL(params.redirectUri); + u.searchParams.set('code', 'demo-auth-code'); + if (params.state) u.searchParams.set('state', params.state); + res.redirect(302, u.href); + }, + challengeForAuthorizationCode: async () => 'challenge', + exchangeAuthorizationCode: async () => ({ access_token: 't', token_type: 'Bearer' }), + exchangeRefreshToken: async () => ({ access_token: 't', token_type: 'Bearer' }), + verifyAccessToken: async token => ({ token, clientId: 'demo-client', scopes: [] }) + }; + + const app = express(); + app.use(mcpAuthRouter({ provider, issuerUrl: new URL('http://localhost/'), authorizationOptions: { rateLimit: false } })); + await using host = await startExpressMinimal(app); + + // Metadata advertises RFC 9207 support. + const md = await fetch(new URL('/.well-known/oauth-authorization-server', host.baseUrl)); + expect(md.status).toBe(200); + const mdBody = (await md.json()) as { issuer: string; authorization_response_iss_parameter_supported?: boolean }; + expect(mdBody.authorization_response_iss_parameter_supported).toBe(true); + + // Success redirect: handler supplies issuer to provider.authorize() and appends `iss` to + // the provider's redirect itself (provider did not set it). + const ok = await fetch( + new URL( + '/authorize?client_id=demo-client&redirect_uri=https%3A%2F%2Fclient.example.com%2Fcb&response_type=code&code_challenge=abc&code_challenge_method=S256&state=xyz', + host.baseUrl + ), + { redirect: 'manual' } + ); + expect(ok.status).toBe(302); + const okLoc = new URL(defined(ok.headers.get('location'), 'location')); + expect(okLoc.searchParams.get('code')).toBe('demo-auth-code'); + expect(okLoc.searchParams.get('iss')).toBe(mdBody.issuer); + expect(seenIssuer).toEqual([mdBody.issuer]); + + // Error redirect (missing code_challenge → invalid_request): handler appends iss itself. + const err = await fetch( + new URL('/authorize?client_id=demo-client&redirect_uri=https%3A%2F%2Fclient.example.com%2Fcb&state=xyz', host.baseUrl), + { redirect: 'manual' } + ); + expect(err.status).toBe(302); + const errLoc = new URL(defined(err.headers.get('location'), 'location')); + expect(errLoc.searchParams.get('error')).toBe('invalid_request'); + expect(errLoc.searchParams.get('iss')).toBe(mdBody.issuer); +}); + /** Listen `app` (already fully configured by the adapter under test) on an ephemeral 127.0.0.1 port; callers close() in finally. */ function listenExpressApp(app: Express): Promise<{ baseUrl: URL; close: () => Promise }> { return new Promise((resolve, reject) => { diff --git a/test/e2e/scenarios/transport-http.test.ts b/test/e2e/scenarios/transport-http.test.ts index b0bd8abbd8..26c7bce868 100644 --- a/test/e2e/scenarios/transport-http.test.ts +++ b/test/e2e/scenarios/transport-http.test.ts @@ -30,7 +30,7 @@ import { expect, vi } from 'vitest'; import { z } from 'zod/v4'; import type { HttpHandler } from '../helpers/index.js'; -import { hostPerSession, hostStateless } from '../helpers/index.js'; +import { defined, hostPerSession, hostStateless } from '../helpers/index.js'; import { verifies } from '../helpers/verifies.js'; import type { TestArgs } from '../types.js'; @@ -74,12 +74,6 @@ function recordingFetch( }; } -/** Narrows away `undefined` for values the surrounding test has already proven exist (replaces non-null assertions). */ -function defined(value: T | undefined, label: string): T { - if (value === undefined) throw new Error(`expected ${label} to be defined`); - return value; -} - verifies('client-transport:http:session-stored', async (_args: TestArgs) => { const records: RecordedRequest[] = []; const handle = hostPerSession(() => echoServer()); diff --git a/test/e2e/scenarios/transport-raw.test.ts b/test/e2e/scenarios/transport-raw.test.ts index bef7300943..6848efafd2 100644 --- a/test/e2e/scenarios/transport-raw.test.ts +++ b/test/e2e/scenarios/transport-raw.test.ts @@ -28,7 +28,7 @@ import { InMemoryTransport, McpServer } from '@modelcontextprotocol/server'; import { expect, vi } from 'vitest'; import { z } from 'zod/v4'; -import { hostPerSession, hostStateless } from '../helpers/index.js'; +import { defined, hostPerSession, hostStateless } from '../helpers/index.js'; import { verifies } from '../helpers/verifies.js'; import type { TestArgs } from '../types.js'; @@ -74,12 +74,6 @@ function echoServer(): McpServer { return s; } -/** Narrows away `undefined` for values the surrounding test has already proven exist (replaces non-null assertions). */ -function defined(value: T | undefined, label: string): T { - if (value === undefined) throw new Error(`expected ${label} to be defined`); - return value; -} - /** Asserts the message observed via onmessage is the initialize response for `id` with the negotiated version. */ function expectInitializeResponse(message: JSONRPCMessage, id: number, protocolVersion: string, serverName: string): void { const response = JSONRPCResultResponseSchema.parse(message); diff --git a/test/e2e/tsconfig.json b/test/e2e/tsconfig.json index 453684bc8b..c9440778bf 100644 --- a/test/e2e/tsconfig.json +++ b/test/e2e/tsconfig.json @@ -20,6 +20,7 @@ "@modelcontextprotocol/server/_shims": ["./node_modules/@modelcontextprotocol/server/src/shimsNode.ts"], "@modelcontextprotocol/server/validators/ajv": ["./node_modules/@modelcontextprotocol/server/src/validators/ajv.ts"], "@modelcontextprotocol/server/validators/cf-worker": ["./node_modules/@modelcontextprotocol/server/src/validators/cfWorker.ts"], + "@modelcontextprotocol/server-legacy": ["./node_modules/@modelcontextprotocol/server-legacy/src/index.ts"], "@modelcontextprotocol/server-legacy/sse": ["./node_modules/@modelcontextprotocol/server-legacy/src/sse/index.ts"], "@modelcontextprotocol/express": ["./node_modules/@modelcontextprotocol/express/src/index.ts"], "@modelcontextprotocol/fastify": ["./node_modules/@modelcontextprotocol/fastify/src/index.ts"],