diff --git a/.changeset/sep-2468-iss-validation.md b/.changeset/sep-2468-iss-validation.md new file mode 100644 index 0000000000..784df78b0e --- /dev/null +++ b/.changeset/sep-2468-iss-validation.md @@ -0,0 +1,10 @@ +--- +'@modelcontextprotocol/core': patch +'@modelcontextprotocol/client': minor +--- + +Add RFC 9207 `iss` parameter validation for authorization responses (SEP-2468). `OAuthMetadataSchema` and `OpenIdProviderMetadataSchema` now recognize `authorization_response_iss_parameter_supported`. The client exports a new `validateAuthorizationResponseIssuer()` helper, +`auth()` accepts an optional `iss`, and `StreamableHTTPClientTransport.finishAuth()` / `SSEClientTransport.finishAuth()` accept an optional `{ iss }` second argument. The `iss` option is tri-state: a string is validated by exact comparison against the issuer recorded in the +authorization server metadata before the authorization code is sent to any token endpoint (mismatch rejects the response without processing any other response parameters); `null` asserts the caller inspected the authorization response and it carried no `iss`, enabling the RFC +9207 fail-closed rejection when the AS advertises `authorization_response_iss_parameter_supported: true`; `undefined` (omitted) skips validation, so existing `finishAuth(code)` callers that never see the authorization response are unaffected. All additions are +backwards-compatible. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 5f55fb7a08..18e3cd5102 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -531,6 +531,71 @@ export async function parseErrorResponse(input: Response | string): Promise { + async finishAuth(authorizationCode: string, options?: { iss?: string | null }): Promise { if (!this._oauthProvider) { throw new UnauthorizedError('finishAuth requires an OAuthClientProvider'); } @@ -237,6 +246,7 @@ export class SSEClientTransport implements Transport { const result = await auth(this._oauthProvider, { serverUrl: this._url, authorizationCode, + iss: options?.iss, 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 3b8ddafe5a..027d191a23 100644 --- a/packages/client/src/client/streamableHttp.ts +++ b/packages/client/src/client/streamableHttp.ts @@ -489,8 +489,17 @@ 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. + * + * @param authorizationCode - The authorization code from the authorization response + * @param options.iss - The `iss` parameter from the authorization response. Pass the string + * value when present; pass `null` to assert the authorization response was inspected and + * contained no `iss` (this enables the RFC 9207 fail-closed rejection when the AS advertises + * `authorization_response_iss_parameter_supported: true`). Leave `undefined` when the response + * parameters were not available — validation is then skipped. When provided, the value is + * validated against the issuer recorded in the authorization server metadata per RFC 9207 + * before the code is exchanged. */ - async finishAuth(authorizationCode: string): Promise { + async finishAuth(authorizationCode: string, options?: { iss?: string | null }): Promise { if (!this._oauthProvider) { throw new UnauthorizedError('finishAuth requires an OAuthClientProvider'); } @@ -498,6 +507,7 @@ export class StreamableHTTPClientTransport implements Transport { const result = await auth(this._oauthProvider, { serverUrl: this._url, authorizationCode, + iss: options?.iss, resourceMetadataUrl: this._resourceMetadataUrl, scope: this._scope, fetchFn: this._fetchWithInit diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 8a08e8fd79..78dbf0659c 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -35,6 +35,7 @@ export { selectResourceURL, startAuthorization, UnauthorizedError, + validateAuthorizationResponseIssuer, validateClientMetadataUrl } from './client/auth.js'; export type { diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 04d7f4a3fb..4e5fc9c1e3 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -19,6 +19,7 @@ import { registerClient, selectClientAuthMethod, startAuthorization, + validateAuthorizationResponseIssuer, validateClientMetadataUrl } from '../../src/client/auth.js'; import { createPrivateKeyJwtAuth } from '../../src/client/authExtensions.js'; @@ -4131,3 +4132,249 @@ describe('OAuth Authorization', () => { }); }); }); + +describe('SEP-2468: RFC 9207 authorization response iss validation', () => { + const issuer = 'https://auth.example.com'; + + describe('validateAuthorizationResponseIssuer', () => { + // RFC 9207 Section 2.4, row 1: advertised but absent -> reject. The caller signals + // "I inspected the authorization response and it had no iss" by passing null. + it('rejects when the AS advertises iss support but the inspected response lacks iss (null)', () => { + expect(() => + validateAuthorizationResponseIssuer({ issuer, authorization_response_iss_parameter_supported: true }, null) + ).toThrow(/did not include an iss parameter/); + }); + + // undefined means the caller never had access to the authorization response + // parameters, so the SDK cannot fail closed on the caller's behalf. + it('skips validation entirely when the caller provides no response parameters (undefined)', () => { + expect(() => + validateAuthorizationResponseIssuer({ issuer, authorization_response_iss_parameter_supported: true }, undefined) + ).not.toThrow(); + expect(() => validateAuthorizationResponseIssuer({ issuer }, undefined)).not.toThrow(); + expect(() => validateAuthorizationResponseIssuer(undefined, undefined)).not.toThrow(); + }); + + // RFC 9207 Section 2.4, row 2: present (advertised) -> exact match required + it('accepts an exactly matching iss when support is advertised', () => { + expect(() => + validateAuthorizationResponseIssuer({ issuer, authorization_response_iss_parameter_supported: true }, issuer) + ).not.toThrow(); + }); + + // RFC 9207 Section 2.4, row 2: present even without advertisement -> still compared + it('accepts an exactly matching iss even when support is not advertised', () => { + expect(() => validateAuthorizationResponseIssuer({ issuer }, issuer)).not.toThrow(); + }); + + it('rejects a mismatched iss regardless of advertisement', () => { + expect(() => validateAuthorizationResponseIssuer({ issuer }, 'https://attacker.example.com')).toThrow( + /does not match the expected issuer/ + ); + expect(() => + validateAuthorizationResponseIssuer( + { issuer, authorization_response_iss_parameter_supported: true }, + 'https://attacker.example.com' + ) + ).toThrow(/does not match the expected issuer/); + }); + + it('uses exact string comparison with no normalization', () => { + // Trailing slash and case differences are equivalent URLs but MUST be rejected + expect(() => validateAuthorizationResponseIssuer({ issuer }, `${issuer}/`)).toThrow(/does not match the expected issuer/); + expect(() => validateAuthorizationResponseIssuer({ issuer }, 'https://AUTH.example.com')).toThrow( + /does not match the expected issuer/ + ); + }); + + // RFC 9207 Section 2.4, row 3: neither advertised nor present -> proceed + it('proceeds when iss support is not advertised and the inspected response has no iss', () => { + expect(() => validateAuthorizationResponseIssuer({ issuer }, null)).not.toThrow(); + expect(() => + validateAuthorizationResponseIssuer({ issuer, authorization_response_iss_parameter_supported: false }, null) + ).not.toThrow(); + }); + + it('proceeds when no metadata is recorded and the inspected response has no iss', () => { + expect(() => validateAuthorizationResponseIssuer(undefined, null)).not.toThrow(); + }); + + it('rejects when an iss is present but no metadata was recorded to validate against', () => { + expect(() => validateAuthorizationResponseIssuer(undefined, issuer)).toThrow(/no authorization server metadata was recorded/); + }); + }); + + describe('auth() with an authorization code', () => { + const resourceMetadata = { + resource: 'https://resource.example.com', + authorization_servers: [issuer] + }; + + const authServerMetadata: AuthorizationServerMetadata = { + issuer, + authorization_endpoint: `${issuer}/authorize`, + token_endpoint: `${issuer}/token`, + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'], + authorization_response_iss_parameter_supported: true + }; + + function createMockProvider(metadata: AuthorizationServerMetadata = authServerMetadata): OAuthClientProvider { + return { + get redirectUrl() { + return 'http://localhost:3000/callback'; + }, + get clientMetadata() { + return { + redirect_uris: ['http://localhost:3000/callback'], + client_name: 'Test Client' + }; + }, + clientInformation: vi.fn().mockResolvedValue({ + client_id: 'test-client-id', + client_secret: 'test-client-secret' + }), + tokens: vi.fn().mockResolvedValue(undefined), + saveTokens: vi.fn(), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn().mockResolvedValue('test-verifier'), + // Discovery state recorded before the redirect, including the validated issuer + discoveryState: vi.fn().mockResolvedValue({ + authorizationServerUrl: issuer, + resourceMetadata, + authorizationServerMetadata: metadata + }) + }; + } + + beforeEach(() => { + mockFetch.mockReset(); + mockFetch.mockImplementation(url => { + const urlString = url.toString(); + if (urlString.includes('/token')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => ({ + access_token: 'access123', + token_type: 'Bearer', + expires_in: 3600 + }) + }); + } + return Promise.reject(new Error(`Unexpected fetch: ${urlString}`)); + }); + }); + + function tokenEndpointCalls(): unknown[][] { + return mockFetch.mock.calls.filter(call => call[0].toString().includes('/token')); + } + + it('exchanges the code when the response iss matches the recorded issuer', async () => { + const provider = createMockProvider(); + + const result = await auth(provider, { + serverUrl: 'https://resource.example.com', + authorizationCode: 'code123', + iss: issuer + }); + + expect(result).toBe('AUTHORIZED'); + expect(tokenEndpointCalls()).toHaveLength(1); + expect(provider.saveTokens).toHaveBeenCalled(); + }); + + it('rejects a mismatched iss before the code reaches any token endpoint', async () => { + const provider = createMockProvider(); + + await expect( + auth(provider, { + serverUrl: 'https://resource.example.com', + authorizationCode: 'code123', + iss: 'https://attacker.example.com' + }) + ).rejects.toThrow(/does not match the expected issuer/); + + expect(tokenEndpointCalls()).toHaveLength(0); + expect(provider.saveTokens).not.toHaveBeenCalled(); + }); + + it('rejects when the AS advertises iss support and the caller reports a response without iss (null)', async () => { + const provider = createMockProvider(); + + await expect( + auth(provider, { + serverUrl: 'https://resource.example.com', + authorizationCode: 'code123', + iss: null + }) + ).rejects.toThrow(/did not include an iss parameter/); + + expect(tokenEndpointCalls()).toHaveLength(0); + }); + + it('proceeds when iss is omitted entirely, even when the AS advertises support', async () => { + // Callers that never had access to the authorization response (legacy + // finishAuth(code) plumbing) must not be failed closed on their behalf. + const provider = createMockProvider(); + + const result = await auth(provider, { + serverUrl: 'https://resource.example.com', + authorizationCode: 'code123' + }); + + expect(result).toBe('AUTHORIZED'); + expect(tokenEndpointCalls()).toHaveLength(1); + }); + + it('proceeds without an iss when the AS does not advertise support', async () => { + const provider = createMockProvider({ + ...authServerMetadata, + authorization_response_iss_parameter_supported: undefined + }); + + const result = await auth(provider, { + serverUrl: 'https://resource.example.com', + authorizationCode: 'code123' + }); + + expect(result).toBe('AUTHORIZED'); + expect(tokenEndpointCalls()).toHaveLength(1); + }); + + it('does not surface error content from a mismatched-issuer error response', async () => { + // RFC 9207: on issuer mismatch the client MUST NOT process the rest of the + // authorization response — including error/error_description parameters. + // Simulate a forged callback carrying both a mismatched iss and attacker- + // controlled error content alongside the code. + const forgedAuthorizationResponse = { + code: 'code123', + iss: 'https://attacker.example.com', + error: 'access_denied', + error_description: 'ATTACKER CONTROLLED MESSAGE' + }; + + const provider = createMockProvider(); + + const error = await auth(provider, { + serverUrl: 'https://resource.example.com', + authorizationCode: forgedAuthorizationResponse.code, + iss: forgedAuthorizationResponse.iss + }).then( + () => { + throw new Error('expected auth() to reject'); + }, + (e: unknown) => e as Error + ); + + // Rejected for the issuer mismatch, without echoing the forged error params + expect(error.message).toMatch(/does not match the expected issuer/); + expect(error.message).not.toContain(forgedAuthorizationResponse.error_description); + expect(error.message).not.toContain('access_denied'); + + // And the code was never sent to a token endpoint + expect(tokenEndpointCalls()).toHaveLength(0); + }); + }); +}); diff --git a/packages/client/test/client/streamableHttp.test.ts b/packages/client/test/client/streamableHttp.test.ts index 0edf8b75ac..b7ed274e50 100644 --- a/packages/client/test/client/streamableHttp.test.ts +++ b/packages/client/test/client/streamableHttp.test.ts @@ -1609,6 +1609,119 @@ describe('StreamableHTTPClientTransport', () => { }); }); + describe('finishAuth iss validation (SEP-2468 / RFC 9207)', () => { + const issuer = 'http://localhost:1234'; + + function createOAuthFetchMock(): Mock { + return ( + vi + .fn() + // Protected resource metadata discovery + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + authorization_servers: [issuer], + resource: 'http://localhost:1234/mcp' + }) + }) + // OAuth metadata discovery — advertises RFC 9207 iss support + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + issuer, + authorization_endpoint: 'http://localhost:1234/authorize', + token_endpoint: 'http://localhost:1234/token', + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'], + authorization_response_iss_parameter_supported: true + }) + }) + // Code exchange + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({ + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + token_type: 'Bearer', + expires_in: 3600 + }) + }) + ); + } + + it('plumbs a matching iss through to validation and completes the exchange', async () => { + const customFetch = createOAuthFetchMock(); + transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + authProvider: mockAuthProvider, + fetch: customFetch + }); + + await transport.finishAuth('test-auth-code', { iss: issuer }); + + // The code reached the token endpoint and tokens were saved + const tokenCalls = customFetch.mock.calls.filter( + ([url, options]) => url.toString().includes('/token') && options?.method === 'POST' + ); + expect(tokenCalls).toHaveLength(1); + expect(mockAuthProvider.saveTokens).toHaveBeenCalledWith({ + access_token: 'new-access-token', + token_type: 'Bearer', + expires_in: 3600, + refresh_token: 'new-refresh-token' + }); + }); + + it('rejects a mismatched iss before the code reaches the token endpoint', async () => { + const customFetch = createOAuthFetchMock(); + transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + authProvider: mockAuthProvider, + fetch: customFetch + }); + + await expect(transport.finishAuth('test-auth-code', { iss: 'https://attacker.example.com' })).rejects.toThrow( + /does not match the expected issuer/ + ); + + const tokenCalls = customFetch.mock.calls.filter(([url]) => url.toString().includes('/token')); + expect(tokenCalls).toHaveLength(0); + expect(mockAuthProvider.saveTokens).not.toHaveBeenCalled(); + }); + + it('rejects when the AS advertises iss support and the caller reports a response without iss (iss: null)', async () => { + const customFetch = createOAuthFetchMock(); + transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + authProvider: mockAuthProvider, + fetch: customFetch + }); + + await expect(transport.finishAuth('test-auth-code', { iss: null })).rejects.toThrow(/did not include an iss parameter/); + + const tokenCalls = customFetch.mock.calls.filter(([url]) => url.toString().includes('/token')); + expect(tokenCalls).toHaveLength(0); + }); + + it('completes the exchange when finishAuth receives no iss at all, even though the AS advertises support', async () => { + // Legacy finishAuth(code) callers cannot see the authorization response; + // the SDK must not fail closed on their behalf. + const customFetch = createOAuthFetchMock(); + transport = new StreamableHTTPClientTransport(new URL('http://localhost:1234/mcp'), { + authProvider: mockAuthProvider, + fetch: customFetch + }); + + await transport.finishAuth('test-auth-code'); + + const tokenCalls = customFetch.mock.calls.filter( + ([url, options]) => url.toString().includes('/token') && options?.method === 'POST' + ); + expect(tokenCalls).toHaveLength(1); + expect(mockAuthProvider.saveTokens).toHaveBeenCalled(); + }); + }); + describe('SSE retry field handling', () => { beforeEach(() => { vi.useFakeTimers(); diff --git a/packages/core/src/shared/auth.ts b/packages/core/src/shared/auth.ts index deee583aa1..6d621ad48b 100644 --- a/packages/core/src/shared/auth.ts +++ b/packages/core/src/shared/auth.ts @@ -66,7 +66,8 @@ export const OAuthMetadataSchema = z.looseObject({ introspection_endpoint_auth_methods_supported: z.array(z.string()).optional(), introspection_endpoint_auth_signing_alg_values_supported: z.array(z.string()).optional(), code_challenge_methods_supported: z.array(z.string()).optional(), - client_id_metadata_document_supported: z.boolean().optional() + client_id_metadata_document_supported: z.boolean().optional(), + authorization_response_iss_parameter_supported: z.boolean().optional() }); /** @@ -110,7 +111,8 @@ export const OpenIdProviderMetadataSchema = z.looseObject({ require_request_uri_registration: z.boolean().optional(), op_policy_uri: SafeUrlSchema.optional(), op_tos_uri: SafeUrlSchema.optional(), - client_id_metadata_document_supported: z.boolean().optional() + client_id_metadata_document_supported: z.boolean().optional(), + authorization_response_iss_parameter_supported: z.boolean().optional() }); /**