diff --git a/.changeset/bind-oauth-credentials-to-issuer.md b/.changeset/bind-oauth-credentials-to-issuer.md new file mode 100644 index 0000000000..7025217c6f --- /dev/null +++ b/.changeset/bind-oauth-credentials-to-issuer.md @@ -0,0 +1,7 @@ +--- +'@modelcontextprotocol/sdk': patch +--- + +Bind stored OAuth client credentials to the authorization server that issued them. `auth()` adds an `issuer` property to what it passes to `saveClientInformation()` / `saveTokens()` and does not reuse a stored value with a different authorization server: the client registers or +authorizes again, or `auth()` throws when the credentials cannot be re-created by registering. `OAuthTokensSchema` and `OAuthClientInformationSchema` accept the optional `issuer`. `ClientCredentialsProvider`, `PrivateKeyJwtProvider` and `StaticPrivateKeyJwtProvider` accept +`expectedIssuer`; omitting it is deprecated. diff --git a/docs/client.md b/docs/client.md index 8f1c653273..c1c4739ba3 100644 --- a/docs/client.md +++ b/docs/client.md @@ -46,6 +46,11 @@ For OAuth-secured MCP servers, the client `auth` module exposes: - `PrivateKeyJwtProvider` - `StaticPrivateKeyJwtProvider` +Pass `expectedIssuer` (the issuer URL of the authorization server the credentials were registered with) to each of these. `auth()` then only presents the credentials to that authorization server and throws if the MCP server advertises a different one. Omitting it is deprecated. + +If you implement `OAuthClientProvider` yourself, store the objects passed to `saveClientInformation()` and `saveTokens()` unchanged: they carry an `issuer` property naming the authorization server they came from, and `auth()` does not reuse them with a different one. If +`clientInformation()` returns pre-registered credentials, include `issuer` yourself. + Examples: - [`simpleOAuthClient.ts`](../src/examples/client/simpleOAuthClient.ts) diff --git a/src/client/auth-extensions.ts b/src/client/auth-extensions.ts index c90404e940..6b8a875667 100644 --- a/src/client/auth-extensions.ts +++ b/src/client/auth-extensions.ts @@ -90,6 +90,24 @@ export function createPrivateKeyJwtAuth(options: { }; } +/** + * The `issuer` stamp for constructor-supplied client information. Omitting `expectedIssuer` is + * deprecated: nothing else says which authorization server such credentials belong to. + */ +function checkedExpectedIssuer(expectedIssuer: string | undefined): string | undefined { + if (expectedIssuer === undefined) { + // eslint-disable-next-line no-console + console.warn( + '[mcp-sdk] Omitting `expectedIssuer` is deprecated. Without it, the MCP server decides which authorization ' + + "server receives this client's credentials; pass your authorization server's issuer URL as " + + '`expectedIssuer` so they are only sent there.' + ); + } else if (typeof expectedIssuer !== 'string' || !expectedIssuer) { + throw new Error("expectedIssuer must be the authorization server's issuer URL"); + } + return expectedIssuer; +} + /** * Options for creating a ClientCredentialsProvider. */ @@ -113,6 +131,16 @@ export interface ClientCredentialsProviderOptions { * Space-separated scopes values requested by the client. */ scope?: string; + + /** + * The issuer URL of the authorization server these credentials were registered with. + * When set, `auth()` only presents them to that authorization server and throws if the + * MCP server advertises a different URL (one trailing slash aside). + * + * Omitting it is deprecated: the credentials then go to whichever authorization server + * the MCP server advertises first. + */ + expectedIssuer?: string; } /** @@ -124,7 +152,8 @@ export interface ClientCredentialsProviderOptions { * @example * const provider = new ClientCredentialsProvider({ * clientId: 'my-client', - * clientSecret: 'my-secret' + * clientSecret: 'my-secret', + * expectedIssuer: 'https://auth.example.com' * }); * * const transport = new StreamableHTTPClientTransport(serverUrl, { @@ -136,10 +165,14 @@ export class ClientCredentialsProvider implements OAuthClientProvider { private _clientInfo: OAuthClientInformation; private _clientMetadata: OAuthClientMetadata; + constructor(options: ClientCredentialsProviderOptions & { expectedIssuer: string }); + /** @deprecated Pass `expectedIssuer` so the credentials are only sent to that authorization server. */ + constructor(options: ClientCredentialsProviderOptions); constructor(options: ClientCredentialsProviderOptions) { this._clientInfo = { client_id: options.clientId, - client_secret: options.clientSecret + client_secret: options.clientSecret, + issuer: checkedExpectedIssuer(options.expectedIssuer) }; this._clientMetadata = { client_name: options.clientName ?? 'client-credentials-client', @@ -227,6 +260,16 @@ export interface PrivateKeyJwtProviderOptions { * Space-separated scopes values requested by the client. */ scope?: string; + + /** + * The issuer URL of the authorization server these credentials were registered with. + * When set, `auth()` only presents them to that authorization server and throws if the + * MCP server advertises a different URL (one trailing slash aside). + * + * Omitting it is deprecated: the credentials then go to whichever authorization server + * the MCP server advertises first. + */ + expectedIssuer?: string; } /** @@ -239,7 +282,8 @@ export interface PrivateKeyJwtProviderOptions { * const provider = new PrivateKeyJwtProvider({ * clientId: 'my-client', * privateKey: pemEncodedPrivateKey, - * algorithm: 'RS256' + * algorithm: 'RS256', + * expectedIssuer: 'https://auth.example.com' * }); * * const transport = new StreamableHTTPClientTransport(serverUrl, { @@ -252,9 +296,13 @@ export class PrivateKeyJwtProvider implements OAuthClientProvider { private _clientMetadata: OAuthClientMetadata; addClientAuthentication: AddClientAuthentication; + constructor(options: PrivateKeyJwtProviderOptions & { expectedIssuer: string }); + /** @deprecated Pass `expectedIssuer` so the credentials are only sent to that authorization server. */ + constructor(options: PrivateKeyJwtProviderOptions); constructor(options: PrivateKeyJwtProviderOptions) { this._clientInfo = { - client_id: options.clientId + client_id: options.clientId, + issuer: checkedExpectedIssuer(options.expectedIssuer) }; this._clientMetadata = { client_name: options.clientName ?? 'private-key-jwt-client', @@ -341,6 +389,16 @@ export interface StaticPrivateKeyJwtProviderOptions { * Space-separated scopes values requested by the client. */ scope?: string; + + /** + * The issuer URL of the authorization server these credentials were registered with. + * When set, `auth()` only presents them to that authorization server and throws if the + * MCP server advertises a different URL (one trailing slash aside). + * + * Omitting it is deprecated: the credentials then go to whichever authorization server + * the MCP server advertises first. + */ + expectedIssuer?: string; } /** @@ -356,9 +414,13 @@ export class StaticPrivateKeyJwtProvider implements OAuthClientProvider { private _clientMetadata: OAuthClientMetadata; addClientAuthentication: AddClientAuthentication; + constructor(options: StaticPrivateKeyJwtProviderOptions & { expectedIssuer: string }); + /** @deprecated Pass `expectedIssuer` so the credentials are only sent to that authorization server. */ + constructor(options: StaticPrivateKeyJwtProviderOptions); constructor(options: StaticPrivateKeyJwtProviderOptions) { this._clientInfo = { - client_id: options.clientId + client_id: options.clientId, + issuer: checkedExpectedIssuer(options.expectedIssuer) }; this._clientMetadata = { client_name: options.clientName ?? 'static-private-key-jwt-client', diff --git a/src/client/auth.ts b/src/client/auth.ts index 214a289cb7..19c4d23f0d 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -74,6 +74,10 @@ export interface OAuthClientProvider { * Loads information about this OAuth client, as registered already with the * server, or returns `undefined` if the client is not registered with the * server. + * + * A provider that returns pre-registered credentials should include `issuer`, the URL of + * the authorization server they were registered with; {@linkcode auth} then does not + * present them to any other. */ clientInformation(): OAuthClientInformationMixed | undefined | Promise; @@ -84,6 +88,10 @@ export interface OAuthClientProvider { * * This method is not required to be implemented if client information is * statically known (e.g., pre-registered). + * + * The object carries `issuer`, the authorization server it was obtained from. Store it + * unchanged so that {@linkcode auth} does not reuse the registration with a different one. + * Client information stored without `issuer` is passed here, with it, after its first successful use. */ saveClientInformation?(clientInformation: OAuthClientInformationMixed): void | Promise; @@ -96,6 +104,9 @@ export interface OAuthClientProvider { /** * Stores new OAuth tokens for the current session, after a successful * authorization. + * + * The object carries `issuer`, the authorization server that issued the tokens. Store it + * unchanged so that {@linkcode auth} does not present the refresh token to a different one. */ saveTokens(tokens: OAuthTokens): void | Promise; @@ -392,6 +403,47 @@ export async function parseErrorResponse(input: Response | string): Promise(stored: T | null | undefined, issuer: string): T | undefined { + // `null`: a `JSON.parse(storage.getItem(...))`-style getter with nothing stored. + if (!stored) return undefined; + // A stamp that is not a string (raw storage) counts as no stamp. + if (typeof stored.issuer !== 'string') return stored.issuer == null ? stored : { ...stored, issuer: undefined }; + return issuersMatch(stored.issuer, issuer) ? stored : undefined; +} + +// `issuer` is added by the client when it stores a value; authorization server responses are parsed without it. +const TokenResponseSchema = OAuthTokensSchema.omit({ issuer: true }); +const RegistrationResponseSchema = OAuthClientInformationFullSchema.omit({ issuer: true }); + +function boundElsewhereError(stored: { issuer?: string }, issuer: string): Error { + return new Error( + `OAuth client information is bound to authorization server ${stored.issuer} and is not presented to ${issuer}. ` + + 'Clear the stored client information, or correct `expectedIssuer`, if the authorization server has moved.' + ); +} + /** * Orchestrates the full auth flow with a server. * @@ -503,6 +555,12 @@ async function authInternal( }); } + // Binding key for stored credentials: the authorization server URL that discovery used + // (advertised by the resource server, restored from discoveryState(), or the legacy + // resource-origin fallback). `metadata.issuer` is deliberately not used: this version does + // not check that the metadata document echoes the URL it was fetched from. + const issuer = String(authorizationServerUrl); + // Send the metadata's resource indicator verbatim: `selectResourceURL` returns a parsed // `URL`, and `URL.href` appends "/" to a pathless indicator such as `https://example.com`, // which exact-match authorization servers reject (#1968). A URL returned by the @@ -518,8 +576,26 @@ async function authInternal( // The resolved scope is used consistently for both DCR and the authorization request. const resolvedScope = scope || resourceMetadata?.scopes_supported?.join(' ') || provider.clientMetadata.scope; - // Handle client registration if needed - let clientInformation = await Promise.resolve(provider.clientInformation()); + // Handle client registration if needed. Client information stamped for a different + // authorization server reads back as `undefined`, so the flow re-registers exactly as if + // nothing were stored. + const storedClientInformation = await Promise.resolve(provider.clientInformation()); + let clientInformation = discardIfIssuerMismatch(storedClientInformation, issuer); + const canRegisterAgain = provider.saveClientInformation !== undefined && !!provider.redirectUrl; + if (storedClientInformation && !clientInformation && !canRegisterAgain) { + // Pre-registered credentials, or a non-interactive client running on configured + // credentials: neither can be re-created by registering with this authorization server. + throw boundElsewhereError(storedClientInformation, issuer); + } + // Saved by an earlier version: bound to the first authorization server that accepts it. + const unstampedClientInformation = clientInformation?.issuer == null ? clientInformation : undefined; + const bindClientInformation = async () => { + try { + if (unstampedClientInformation) await provider.saveClientInformation?.({ ...unstampedClientInformation, issuer }); + } catch { + // A provider that only expects this call after a registration keeps working, unbound. + } + }; if (!clientInformation) { if (authorizationCode !== undefined) { throw new Error('Existing OAuth client information is required when exchanging an authorization code'); @@ -538,9 +614,7 @@ async function authInternal( if (shouldUseUrlBasedClientId) { // SEP-991: URL-based Client IDs - clientInformation = { - client_id: clientMetadataUrl - }; + clientInformation = { client_id: clientMetadataUrl, issuer }; await provider.saveClientInformation?.(clientInformation); } else { // Fallback to dynamic registration @@ -555,8 +629,8 @@ async function authInternal( fetchFn }); - await provider.saveClientInformation(fullInformation); - clientInformation = fullInformation; + clientInformation = { ...fullInformation, issuer }; + await provider.saveClientInformation(clientInformation); } } @@ -572,11 +646,22 @@ async function authInternal( fetchFn }); - await provider.saveTokens(tokens); + await bindClientInformation(); + await provider.saveTokens({ ...tokens, issuer }); return 'AUTHORIZED'; } - const tokens = await provider.tokens(); + // A refresh token stamped for a different authorization server reads back as `undefined`, + // so it is never posted to this one's token endpoint. + const tokens = discardIfIssuerMismatch(await provider.tokens(), issuer); + if (tokens?.refresh_token && tokens.issuer == null) { + // eslint-disable-next-line no-console + console.warn( + "[mcp-sdk] stored OAuth tokens have no 'issuer' property (saved by an earlier version, or by a provider that " + + 'does not keep it) and are used as-is; make sure your OAuthClientProvider stores what saveTokens() and ' + + 'saveClientInformation() receive unchanged.' + ); + } // Handle token refresh or new authorization if (tokens?.refresh_token) { @@ -591,7 +676,8 @@ async function authInternal( fetchFn }); - await provider.saveTokens(newTokens); + await bindClientInformation(); + await provider.saveTokens({ ...newTokens, issuer }); return 'AUTHORIZED'; } catch (error) { // If this is a ServerError, or an unknown type, log it out and try to continue. Otherwise, escalate so we can fix things and retry. @@ -1276,7 +1362,7 @@ async function executeTokenRequest( throw await parseErrorResponse(response); } - return OAuthTokensSchema.parse(await response.json()); + return TokenResponseSchema.parse(await response.json()); } /** @@ -1415,6 +1501,17 @@ export async function fetchToken( fetchFn?: FetchLike; } = {} ): Promise { + // Nothing is sent to an authorization server other than the one the client information is stamped for. + const readClientInformation = async () => { + const storedClientInformation = await provider.clientInformation(); + const checked = discardIfIssuerMismatch(storedClientInformation, String(authorizationServerUrl)); + if (storedClientInformation && !checked) { + throw boundElsewhereError(storedClientInformation, String(authorizationServerUrl)); + } + return checked; + }; + let clientInformation = await readClientInformation(); + const scope = provider.clientMetadata.scope; // Use provider's prepareTokenRequest if available, otherwise fall back to authorization_code @@ -1435,12 +1532,13 @@ export async function fetchToken( tokenRequestParams = prepareAuthorizationCodeRequest(authorizationCode, codeVerifier, provider.redirectUrl); } - const clientInformation = await provider.clientInformation(); + // A provider may fill in its client information while the request is prepared. + clientInformation ??= await readClientInformation(); return executeTokenRequest(authorizationServerUrl, { metadata, tokenRequestParams, - clientInformation: clientInformation ?? undefined, + clientInformation, addClientAuthentication: provider.addClientAuthentication, resource, fetchFn @@ -1495,5 +1593,5 @@ export async function registerClient( throw await parseErrorResponse(response); } - return OAuthClientInformationFullSchema.parse(await response.json()); + return RegistrationResponseSchema.parse(await response.json()); } diff --git a/src/examples/client/simpleClientCredentials.ts b/src/examples/client/simpleClientCredentials.ts index 7defcc41f9..70967f1fb4 100644 --- a/src/examples/client/simpleClientCredentials.ts +++ b/src/examples/client/simpleClientCredentials.ts @@ -16,6 +16,7 @@ * * Common: * MCP_SERVER_URL - Server URL (default: http://localhost:3000/mcp) + * MCP_EXPECTED_ISSUER - Issuer URL of the authorization server the credentials were registered with (required) */ import { Client } from '../../client/index.js'; @@ -32,6 +33,13 @@ function createProvider(): OAuthClientProvider { process.exit(1); } + // The authorization server these credentials were registered with + const expectedIssuer = process.env.MCP_EXPECTED_ISSUER; + if (!expectedIssuer) { + console.error('MCP_EXPECTED_ISSUER environment variable is required'); + process.exit(1); + } + // If private key is provided, use private_key_jwt authentication const privateKeyPem = process.env.MCP_CLIENT_PRIVATE_KEY_PEM; if (privateKeyPem) { @@ -40,7 +48,8 @@ function createProvider(): OAuthClientProvider { return new PrivateKeyJwtProvider({ clientId, privateKey: privateKeyPem, - algorithm + algorithm, + expectedIssuer }); } @@ -54,7 +63,8 @@ function createProvider(): OAuthClientProvider { console.log('Using client_secret_basic authentication'); return new ClientCredentialsProvider({ clientId, - clientSecret + clientSecret, + expectedIssuer }); } diff --git a/src/server/auth/providers/proxyProvider.ts b/src/server/auth/providers/proxyProvider.ts index 855856c89e..9e8e67ea82 100644 --- a/src/server/auth/providers/proxyProvider.ts +++ b/src/server/auth/providers/proxyProvider.ts @@ -12,6 +12,10 @@ import { AuthorizationParams, OAuthServerProvider } from '../provider.js'; import { ServerError } from '../errors.js'; import { FetchLike } from '../../../shared/transport.js'; +// `issuer` is added by a client when it stores a value; upstream responses are parsed without it. +const TokenResponseSchema = OAuthTokensSchema.omit({ issuer: true }); +const RegistrationResponseSchema = OAuthClientInformationFullSchema.omit({ issuer: true }); + export type ProxyEndpoints = { authorizationUrl: string; tokenUrl: string; @@ -113,7 +117,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } const data = await response.json(); - return OAuthClientInformationFullSchema.parse(data); + return RegistrationResponseSchema.parse(data); } }) }; @@ -188,7 +192,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } const data = await response.json(); - return OAuthTokensSchema.parse(data); + return TokenResponseSchema.parse(data); } async exchangeRefreshToken( @@ -229,7 +233,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } const data = await response.json(); - return OAuthTokensSchema.parse(data); + return TokenResponseSchema.parse(data); } async verifyAccessToken(token: string): Promise { diff --git a/src/shared/auth.ts b/src/shared/auth.ts index c546c8608b..99959be189 100644 --- a/src/shared/auth.ts +++ b/src/shared/auth.ts @@ -134,7 +134,12 @@ export const OAuthTokensSchema = z token_type: z.string(), expires_in: z.coerce.number().optional(), scope: z.string().optional(), - refresh_token: z.string().optional() + refresh_token: z.string().optional(), + /** + * Not part of the wire format: the authorization server this value was obtained from, added by + * the client's `auth()` before it is stored and compared when it is read back. + */ + issuer: z.string().optional().catch(undefined) }) .strip(); @@ -184,7 +189,12 @@ export const OAuthClientInformationSchema = z client_id: z.string(), client_secret: z.string().optional(), client_id_issued_at: z.number().optional(), - client_secret_expires_at: z.number().optional() + client_secret_expires_at: z.number().optional(), + /** + * Not part of the wire format: the authorization server this value was obtained from, added by + * the client's `auth()` before it is stored and compared when it is read back. + */ + issuer: z.string().optional().catch(undefined) }) .strip(); diff --git a/test/client/auth-extensions.test.ts b/test/client/auth-extensions.test.ts index 623d5e4da4..7d323f9c92 100644 --- a/test/client/auth-extensions.test.ts +++ b/test/client/auth-extensions.test.ts @@ -1,5 +1,5 @@ -import { describe, it, expect } from 'vitest'; -import { auth } from '../../src/client/auth.js'; +import { afterEach, beforeEach, describe, it, expect, vi, type MockInstance } from 'vitest'; +import { auth, type OAuthClientProvider } from '../../src/client/auth.js'; import { ClientCredentialsProvider, PrivateKeyJwtProvider, @@ -14,6 +14,7 @@ const AUTH_SERVER_URL = 'https://auth.example.com'; describe('auth-extensions providers (end-to-end with auth())', () => { it('authenticates using ClientCredentialsProvider with client_secret_basic', async () => { const provider = new ClientCredentialsProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'my-client', clientSecret: 'my-secret', clientName: 'test-client' @@ -51,6 +52,7 @@ describe('auth-extensions providers (end-to-end with auth())', () => { it('sends scope in token request when ClientCredentialsProvider is configured with scope', async () => { const provider = new ClientCredentialsProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'my-client', clientSecret: 'my-secret', clientName: 'test-client', @@ -80,6 +82,7 @@ describe('auth-extensions providers (end-to-end with auth())', () => { it('authenticates using PrivateKeyJwtProvider with private_key_jwt', async () => { const provider = new PrivateKeyJwtProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'client-id', privateKey: 'a-string-secret-at-least-256-bits-long', algorithm: 'HS256', @@ -123,6 +126,7 @@ describe('auth-extensions providers (end-to-end with auth())', () => { it('sends scope in token request when PrivateKeyJwtProvider is configured with scope', async () => { const provider = new PrivateKeyJwtProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'client-id', privateKey: 'a-string-secret-at-least-256-bits-long', algorithm: 'HS256', @@ -155,6 +159,7 @@ describe('auth-extensions providers (end-to-end with auth())', () => { it('fails when PrivateKeyJwtProvider is configured with an unsupported algorithm', async () => { const provider = new PrivateKeyJwtProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'client-id', privateKey: 'a-string-secret-at-least-256-bits-long', algorithm: 'none', @@ -178,6 +183,7 @@ describe('auth-extensions providers (end-to-end with auth())', () => { const staticAssertion = 'header.payload.signature'; const provider = new StaticPrivateKeyJwtProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'static-client', jwtBearerAssertion: staticAssertion, clientName: 'static-private-key-jwt-client' @@ -215,6 +221,7 @@ describe('auth-extensions providers (end-to-end with auth())', () => { const staticAssertion = 'header.payload.signature'; const provider = new StaticPrivateKeyJwtProvider({ + expectedIssuer: AUTH_SERVER_URL, clientId: 'static-client', jwtBearerAssertion: staticAssertion, clientName: 'static-private-key-jwt-client', @@ -245,6 +252,151 @@ describe('auth-extensions providers (end-to-end with auth())', () => { }); }); +describe('auth-extensions providers are bound to one authorization server', () => { + const SERVER_URL = 'https://api.example.com/mcp'; + const AS_ONE = 'https://as-one.example.com'; + const AS_TWO = 'https://as-two.example.com'; + + function createMigratingFetch() { + let active = AS_ONE; + const requests: string[] = []; + const tokenCalls: Array<{ origin: string; body: string; authorization: string | null }> = []; + const fetchFn = async (url: string | URL, init?: RequestInit): Promise => { + const u = new URL(String(url)); + requests.push(`${init?.method ?? 'GET'} ${u.origin}${u.pathname}`); + if (u.pathname.includes('/.well-known/oauth-protected-resource')) { + return Response.json({ resource: SERVER_URL, authorization_servers: [active] }); + } + if (u.pathname.includes('/.well-known/')) { + return Response.json({ + issuer: u.origin, + authorization_endpoint: `${u.origin}/authorize`, + token_endpoint: `${u.origin}/token`, + registration_endpoint: `${u.origin}/register`, + response_types_supported: ['code'], + grant_types_supported: ['client_credentials'] + }); + } + if (u.pathname === '/register') { + return Response.json({ client_id: `cid-${u.host}`, client_secret: `secret-${u.host}`, redirect_uris: [] }, { status: 201 }); + } + if (u.pathname === '/token') { + tokenCalls.push({ + origin: u.origin, + body: String(init?.body), + authorization: new Headers(init?.headers).get('authorization') + }); + return Response.json({ access_token: `at-${u.host}`, token_type: 'Bearer' }); + } + return new Response(null, { status: 404 }); + }; + return { fetchFn, requests, tokenCalls, switchTo: (as: string) => (active = as) }; + } + + type Options = { expectedIssuer?: string }; + const providers: Array<[string, (options: Options) => OAuthClientProvider]> = [ + [ + 'ClientCredentialsProvider', + o => new ClientCredentialsProvider({ clientId: 'client-one', clientSecret: 'configured-secret', ...o }) + ], + [ + 'PrivateKeyJwtProvider', + o => new PrivateKeyJwtProvider({ clientId: 'client-one', privateKey: 'k'.repeat(64), algorithm: 'HS256', ...o }) + ], + [ + 'StaticPrivateKeyJwtProvider', + o => new StaticPrivateKeyJwtProvider({ clientId: 'client-one', jwtBearerAssertion: 'assertion-one', ...o }) + ] + ]; + + let warn: MockInstance; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warn.mockRestore(); + }); + + it.each(providers)('%s with expectedIssuer is only used with that authorization server', async (_name, create) => { + const srv = createMigratingFetch(); + const provider = create({ expectedIssuer: AS_ONE }); + + srv.switchTo(AS_TWO); + await expect(auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).rejects.toThrow( + `OAuth client information is bound to authorization server ${AS_ONE} and is not presented to ${AS_TWO}` + ); + expect(srv.requests.filter(r => r.includes(AS_TWO))).toEqual([`GET ${AS_TWO}/.well-known/oauth-authorization-server`]); + + srv.switchTo(AS_ONE); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(srv.tokenCalls.map(c => c.origin)).toEqual([AS_ONE]); + expect(warn).not.toHaveBeenCalled(); + }); + + it('accepts expectedIssuer spelled with a trailing slash', async () => { + const srv = createMigratingFetch(); + const provider = new ClientCredentialsProvider({ + clientId: 'client-one', + clientSecret: 'configured-secret', + expectedIssuer: `${AS_ONE}/` + }); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + }); + + it.each(['https://AS-ONE.example.com', 'https://as-one.example.com:443'])('accepts expectedIssuer spelled %s', async expectedIssuer => { + const srv = createMigratingFetch(); + const provider = new ClientCredentialsProvider({ clientId: 'client-one', clientSecret: 'configured-secret', expectedIssuer }); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(srv.tokenCalls.map(c => c.origin)).toEqual([AS_ONE]); + }); + + it.each(providers)('%s without expectedIssuer stays with the first authorization server it is used with', async (_name, create) => { + const srv = createMigratingFetch(); + const provider = create({}); + + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + + srv.switchTo(AS_TWO); + await expect(auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).rejects.toThrow( + `OAuth client information is bound to authorization server ${AS_ONE} and is not presented to ${AS_TWO}` + ); + expect(srv.requests.filter(r => r.includes(AS_TWO))).toEqual([`GET ${AS_TWO}/.well-known/oauth-authorization-server`]); + + // Back at the first authorization server the configured credential is used as before. + srv.switchTo(AS_ONE); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(srv.tokenCalls.map(c => c.origin)).toEqual([AS_ONE, AS_ONE]); + }); + + it.each(providers)('%s without expectedIssuer recovers after a failed first attempt', async (_name, create) => { + const srv = createMigratingFetch(); + const provider = create({}); + + // Discovery fails once and falls back to the resource origin, which has no token endpoint. + const unavailable = async (url: string | URL) => + new Response(null, { status: String(url).includes('oauth-protected-resource') ? 503 : 404 }); + await expect(auth(provider, { serverUrl: SERVER_URL, fetchFn: unavailable })).rejects.toThrow(); + + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + }); + + it.each(providers)('%s logs one deprecation message at construction when expectedIssuer is omitted', async (_name, create) => { + const srv = createMigratingFetch(); + const provider = create({}); + await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('Omitting `expectedIssuer` is deprecated')); + }); + + it.each([null, '', 42, new URL(AS_ONE)])('rejects expectedIssuer %j at construction', value => { + expect(() => new ClientCredentialsProvider({ clientId: 'c', clientSecret: 's', expectedIssuer: value as string })).toThrow( + 'expectedIssuer must be' + ); + }); +}); + describe('createPrivateKeyJwtAuth', () => { const baseOptions = { issuer: 'client-id', diff --git a/test/client/auth.test.ts b/test/client/auth.test.ts index 56d1c5b943..45ec2806d0 100644 --- a/test/client/auth.test.ts +++ b/test/client/auth.test.ts @@ -11,14 +11,23 @@ import { discoverOAuthServerInfo, extractWWWAuthenticateParams, auth, + fetchToken, type OAuthClientProvider, + type OAuthDiscoveryState, selectClientAuthMethod, isHttpsUrl } from '../../src/client/auth.js'; import { createPrivateKeyJwtAuth } from '../../src/client/auth-extensions.js'; import { InvalidClientMetadataError, ServerError } from '../../src/server/auth/errors.js'; -import { AuthorizationServerMetadata, OAuthClientMetadata, OAuthTokens } from '../../src/shared/auth.js'; -import { expect, vi, type Mock } from 'vitest'; +import { + AuthorizationServerMetadata, + OAuthClientInformationMixed, + OAuthClientInformationSchema, + OAuthClientMetadata, + OAuthTokens, + OAuthTokensSchema +} from '../../src/shared/auth.js'; +import { expect, vi, type Mock, type MockInstance } from 'vitest'; // Mock pkce-challenge vi.mock('pkce-challenge', () => ({ @@ -3556,7 +3565,8 @@ describe('OAuth Authorization', () => { // Should save URL-based client info expect(mockProvider.saveClientInformation).toHaveBeenCalledWith({ - client_id: 'https://example.com/client-metadata.json' + client_id: 'https://example.com/client-metadata.json', + issuer: 'https://server.example.com/' }); }); @@ -3602,7 +3612,8 @@ describe('OAuth Authorization', () => { expect(mockProvider.saveClientInformation).toHaveBeenCalledWith({ client_id: 'generated-uuid', client_secret: 'generated-secret', - redirect_uris: ['http://localhost:3000/callback'] + redirect_uris: ['http://localhost:3000/callback'], + issuer: 'https://server.example.com/' }); }); @@ -3758,8 +3769,515 @@ describe('OAuth Authorization', () => { expect(mockProvider.saveClientInformation).toHaveBeenCalledWith({ client_id: 'generated-uuid', client_secret: 'generated-secret', - redirect_uris: ['http://localhost:3000/callback'] + redirect_uris: ['http://localhost:3000/callback'], + issuer: 'https://server.example.com/' + }); + }); + }); + describe('auth: credentials are bound to the authorization server that issued them', () => { + const SERVER_URL = 'https://api.example.com/mcp'; + const AS_ONE = 'https://as-one.example.com'; + const AS_TWO = 'https://as-two.example.com'; + + const asMetadata = (issuer: string, endpoints = issuer): AuthorizationServerMetadata => ({ + issuer, + authorization_endpoint: `${endpoints}/authorize`, + token_endpoint: `${endpoints}/token`, + registration_endpoint: `${endpoints}/register`, + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'], + grant_types_supported: ['authorization_code', 'refresh_token', 'client_credentials'] + }); + + /** + * Resource server whose protected resource metadata advertises `active` as the authorization + * server; every origin answers authorization server metadata for itself. Records where + * registrations and token requests were sent. + */ + function createMigratingFetch(opts: { prm?: boolean; claimedIssuer?: Record } = {}) { + let active = AS_ONE; + let rejecting: string | undefined; + const registerCalls: string[] = []; + const tokenCalls: Array<{ origin: string; body: URLSearchParams; authorization: string | null }> = []; + const requests: string[] = []; + const fetchFn = async (url: string | URL, init?: RequestInit): Promise => { + const u = new URL(String(url)); + requests.push(`${init?.method ?? 'GET'} ${u.origin}${u.pathname}`); + if (u.pathname.includes('/.well-known/oauth-protected-resource')) { + if (opts.prm === false) return new Response(null, { status: 404 }); + return Response.json({ resource: SERVER_URL, authorization_servers: [active] }); + } + if (u.pathname.includes('/.well-known/')) { + return Response.json(asMetadata(opts.claimedIssuer?.[u.origin] ?? u.origin, u.origin)); + } + if (u.pathname === '/register') { + registerCalls.push(u.origin); + return Response.json( + { + client_id: `cid-${u.host}`, + client_secret: `secret-${u.host}`, + redirect_uris: ['http://localhost:3000/callback'] + }, + { status: 201 } + ); + } + if (u.pathname === '/token') { + const body = new URLSearchParams(String(init?.body)); + tokenCalls.push({ origin: u.origin, body, authorization: new Headers(init?.headers).get('authorization') }); + if (u.origin === rejecting) return new Response(null, { status: 404 }); + return Response.json({ access_token: `at-${u.host}`, token_type: 'Bearer', refresh_token: `rt-${u.host}` }); + } + return new Response(null, { status: 404 }); + }; + return { + fetchFn, + registerCalls, + tokenCalls, + requests, + switchTo: (as: string) => (active = as), + rejectTokenRequestsAt: (origin: string) => (rejecting = origin) + }; + } + + type Stored = { info?: OAuthClientInformationMixed; tokens?: OAuthTokens }; + + /** Single-slot provider that round-trips whatever auth() saves. */ + function createBlobProvider(withDiscoveryState = true): OAuthClientProvider & { redirected: URL[]; stored: Stored } { + const stored: Stored = {}; + const redirected: URL[] = []; + let discovery: OAuthDiscoveryState | undefined; + let verifier: string | undefined; + return { + redirected, + stored, + get redirectUrl() { + return 'http://localhost:3000/callback'; + }, + get clientMetadata() { + return { client_name: 't', redirect_uris: ['http://localhost:3000/callback'] }; + }, + clientInformation: () => stored.info, + saveClientInformation: i => void (stored.info = i), + tokens: () => stored.tokens, + saveTokens: t => void (stored.tokens = t), + redirectToAuthorization: u => void redirected.push(u), + saveCodeVerifier: v => void (verifier = v), + codeVerifier: () => verifier ?? 'v', + ...(withDiscoveryState && { + saveDiscoveryState: (s: OAuthDiscoveryState) => void (discovery = s), + discoveryState: () => discovery, + invalidateCredentials: (s: string) => { + if (s === 'client' || s === 'all') stored.info = undefined; + if (s === 'tokens' || s === 'all') stored.tokens = undefined; + if (s === 'discovery' || s === 'all') discovery = undefined; + } + }) + }; + } + + /** Whether `needle` appears in the body or Basic credentials of any recorded token request (optionally ignoring one origin). */ + const sentAnywhere = (srv: ReturnType, needle: string, exceptOrigin?: string) => + srv.tokenCalls + .filter(c => c.origin !== exceptOrigin) + .some( + c => + String(c.body).includes(needle) || + (c.authorization !== null && atob(c.authorization.replace(/^Basic /, '')).includes(needle)) + ); + + let warn: MockInstance; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warn.mockRestore(); + }); + + it('stamps the authorization server onto saved client information and tokens', async () => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(); + + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('REDIRECT'); + expect(provider.stored.info).toEqual(expect.objectContaining({ client_id: 'cid-as-one.example.com', issuer: AS_ONE })); + + expect(await auth(provider, { serverUrl: SERVER_URL, authorizationCode: 'code', fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(provider.stored.tokens).toEqual({ + access_token: 'at-as-one.example.com', + token_type: 'Bearer', + refresh_token: 'rt-as-one.example.com', + issuer: AS_ONE + }); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a refresh token issued through AS-one is never posted to AS-two', async () => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(); + provider.stored.info = { client_id: 'cid', client_secret: 'secret-one', issuer: AS_ONE }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-one', issuer: AS_ONE }; + srv.switchTo(AS_TWO); + + const result = await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + expect(srv.tokenCalls.filter(c => c.origin === AS_TWO)).toHaveLength(0); + expect(sentAnywhere(srv, 'rt-one')).toBe(false); + expect(sentAnywhere(srv, 'secret-one')).toBe(false); + expect(result).toBe('REDIRECT'); + expect(provider.redirected.at(-1)?.origin).toBe(AS_TWO); + }); + + it('client information issued through AS-one is not reused at AS-two; the client re-registers', async () => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(); + + await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + expect(srv.registerCalls).toEqual([AS_ONE]); + + srv.switchTo(AS_TWO); + provider.invalidateCredentials?.('discovery'); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('REDIRECT'); + expect(srv.registerCalls).toEqual([AS_ONE, AS_TWO]); + expect(provider.stored.info).toEqual(expect.objectContaining({ client_id: 'cid-as-two.example.com', issuer: AS_TWO })); + expect(provider.redirected.at(-1)?.origin).toBe(AS_TWO); + expect(provider.redirected.at(-1)?.searchParams.get('client_id')).toBe('cid-as-two.example.com'); + }); + + it("the binding key is the discovery URL, not the metadata's issuer", async () => { + const srv = createMigratingFetch({ claimedIssuer: { [AS_TWO]: AS_ONE } }); + const provider = createBlobProvider(); + provider.stored.info = { client_id: 'cid', client_secret: 'secret-one', issuer: AS_ONE }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-one', issuer: AS_ONE }; + srv.switchTo(AS_TWO); + + const result = await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + expect(srv.tokenCalls).toHaveLength(0); + expect(sentAnywhere(srv, 'rt-one')).toBe(false); + expect(result).toBe('REDIRECT'); + expect(srv.registerCalls).toEqual([AS_TWO]); + }); + + it('the resource-origin fallback is a distinct binding key', async () => { + const srv = createMigratingFetch({ prm: false }); + const provider = createBlobProvider(); + provider.stored.info = { client_id: 'cid', client_secret: 'secret-one', issuer: AS_ONE }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-one', issuer: AS_ONE }; + + const result = await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + expect(srv.tokenCalls).toHaveLength(0); + expect(result).toBe('REDIRECT'); + expect(srv.registerCalls).toEqual(['https://api.example.com']); + expect(provider.stored.info?.issuer).toBe('https://api.example.com/'); + }); + + it('unstamped stored credentials are bound on first use', async () => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(); + provider.stored.info = { client_id: 'legacy-cid', client_secret: 'legacy-secret' }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-legacy' }; + + // First use: refreshed at the resolved authorization server and written back with its stamp. + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(srv.tokenCalls.map(c => c.origin)).toEqual([AS_ONE]); + expect(provider.stored.info).toEqual({ client_id: 'legacy-cid', client_secret: 'legacy-secret', issuer: AS_ONE }); + expect(provider.stored.tokens?.issuer).toBe(AS_ONE); + expect(srv.registerCalls).toHaveLength(0); + expect(warn.mock.calls.filter(c => String(c[0]).includes("no 'issuer' property"))).toHaveLength(1); + + // From then on the stamp applies. + srv.switchTo(AS_TWO); + provider.invalidateCredentials?.('discovery'); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('REDIRECT'); + expect(srv.tokenCalls.filter(c => c.origin === AS_TWO)).toHaveLength(0); + expect(sentAnywhere(srv, 'legacy-secret', AS_ONE)).toBe(false); + expect(sentAnywhere(srv, 'rt-as-one.example.com')).toBe(false); + expect(srv.registerCalls).toEqual([AS_TWO]); + }); + + it('unstamped stored credentials are only bound to an authorization server that accepted them', async () => { + const srv = createMigratingFetch(); + let prmAvailable = false; + const fetchFn = async (url: string | URL, init?: RequestInit) => + !prmAvailable && String(url).includes('oauth-protected-resource') + ? new Response(null, { status: 503 }) + : srv.fetchFn(url, init); + const provider = createBlobProvider(false); + provider.stored.info = { client_id: 'legacy-cid', client_secret: 'legacy-secret', issuer: null as unknown as undefined }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-legacy' }; + + // Discovery falls back to the resource origin, where the refresh is not accepted. + srv.rejectTokenRequestsAt('https://api.example.com'); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn })).toBe('REDIRECT'); + expect(provider.stored.info?.issuer).toBeNull(); + expect(provider.stored.tokens?.issuer).toBeUndefined(); + + prmAvailable = true; + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn })).toBe('AUTHORIZED'); + expect(srv.registerCalls).toEqual([]); + expect(provider.stored.info?.issuer).toBe(AS_ONE); + expect(provider.stored.tokens?.issuer).toBe(AS_ONE); + }); + + it('cached discovery state keeps its authorization server binding', async () => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(); + provider.stored.info = { client_id: 'cid', client_secret: 'secret-one', issuer: AS_ONE }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-one', issuer: AS_ONE }; + provider.saveDiscoveryState?.({ authorizationServerUrl: AS_ONE, authorizationServerMetadata: asMetadata(AS_ONE) }); + srv.switchTo(AS_TWO); + + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(srv.tokenCalls.map(c => c.origin)).toEqual([AS_ONE]); + expect(srv.requests.some(r => r.includes(AS_TWO))).toBe(false); + }); + + it('a provider that cannot re-register reports the authorization server its client information is bound to', async () => { + const srv = createMigratingFetch(); + const provider: OAuthClientProvider = { ...createBlobProvider(), saveClientInformation: undefined }; + provider.clientInformation = () => ({ client_id: 'cid', client_secret: 'secret-one', issuer: AS_ONE }); + srv.switchTo(AS_TWO); + + await expect(auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).rejects.toThrow( + `OAuth client information is bound to authorization server ${AS_ONE}` + ); + expect(srv.tokenCalls).toHaveLength(0); + }); + + it('fetchToken sends nothing to an authorization server other than the one the client information is bound to', async () => { + const srv = createMigratingFetch(); + const addClientAuthentication = vi.fn>(); + const provider: OAuthClientProvider = { ...createBlobProvider(), addClientAuthentication }; + provider.clientInformation = () => ({ client_id: 'cid', client_secret: 'bound-secret', issuer: AS_ONE }); + + await expect( + fetchToken(provider, AS_TWO, { metadata: asMetadata(AS_TWO), authorizationCode: 'code', fetchFn: srv.fetchFn }) + ).rejects.toThrow(`OAuth client information is bound to authorization server ${AS_ONE}`); + expect(srv.requests).toEqual([]); + expect(addClientAuthentication).not.toHaveBeenCalled(); + + // A matching stamp is used as before (one trailing slash is tolerated). + provider.addClientAuthentication = undefined; + await fetchToken(provider, `${AS_ONE}/`, { metadata: asMetadata(AS_ONE), authorizationCode: 'code', fetchFn: srv.fetchFn }); + expect(srv.tokenCalls.map(c => [c.origin, c.authorization])).toEqual([[AS_ONE, `Basic ${btoa('cid:bound-secret')}`]]); + expect(warn).not.toHaveBeenCalled(); + }); + + it('a provider that reads storage back through the SDK schemas keeps the binding', async () => { + const storage = new Map(); + const provider: OAuthClientProvider = { + ...createBlobProvider(false), + clientInformation: () => + storage.has('info') ? OAuthClientInformationSchema.parseAsync(JSON.parse(storage.get('info')!)) : undefined, + saveClientInformation: i => void storage.set('info', JSON.stringify(i)), + tokens: () => (storage.has('tokens') ? OAuthTokensSchema.parseAsync(JSON.parse(storage.get('tokens')!)) : undefined), + saveTokens: t => void storage.set('tokens', JSON.stringify(t)) + }; + const srv = createMigratingFetch(); + await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn }); + await auth(provider, { serverUrl: SERVER_URL, authorizationCode: 'code', fetchFn: srv.fetchFn }); + + srv.switchTo(AS_TWO); + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('REDIRECT'); + expect(srv.tokenCalls.map(c => c.origin)).toEqual([AS_ONE]); + expect(warn).not.toHaveBeenCalled(); + }); + + it('an issuer property in a registration or token response does not become the stamp', async () => { + const srv = createMigratingFetch(); + const fetchFn = async (url: string | URL, init?: RequestInit) => { + const response = await srv.fetchFn(url, init); + return init?.method === 'POST' ? Response.json({ ...(await response.json()), issuer: AS_ONE }, response) : response; + }; + const provider = createBlobProvider(false); + srv.switchTo(AS_TWO); + + await auth(provider, { serverUrl: SERVER_URL, fetchFn }); + await auth(provider, { serverUrl: SERVER_URL, authorizationCode: 'code', fetchFn }); + expect(provider.stored.info?.issuer).toBe(AS_TWO); + expect(provider.stored.tokens?.issuer).toBe(AS_TWO); + }); + + it('an issuer that is not a string is dropped from a response, and an empty stored one matches nothing', async () => { + expect(OAuthTokensSchema.parse({ access_token: 'at', token_type: 'Bearer', issuer: null })).toEqual({ + access_token: 'at', + token_type: 'Bearer' + }); + expect(OAuthClientInformationSchema.parse({ client_id: 'cid', issuer: 5 })).toEqual({ client_id: 'cid' }); + + const srv = createMigratingFetch(); + const provider: OAuthClientProvider = { ...createBlobProvider(false), saveClientInformation: undefined }; + provider.clientInformation = () => ({ client_id: 'cid', client_secret: 'secret-one', issuer: '' }); + await expect(auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).rejects.toThrow( + 'OAuth client information is bound' + ); + expect(srv.tokenCalls).toEqual([]); + }); + + it('binding unstamped client information never fails a flow that worked without it', async () => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(false); + provider.stored.info = { client_id: 'pre-registered' }; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt-legacy' }; + provider.saveClientInformation = vi.fn(() => { + throw new Error('client is pre-registered'); }); + + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(provider.saveClientInformation).toHaveBeenCalledWith({ client_id: 'pre-registered', issuer: AS_ONE }); + expect(provider.stored.tokens?.issuer).toBe(AS_ONE); + }); + + it('a provider whose storage getters return null is treated as having nothing stored', async () => { + // The `JSON.parse(storage.getItem(key))` idiom yields `null`, not `undefined`, for an empty slot. + const storage = new Map(); + const read = (key: string) => JSON.parse(storage.get(key) ?? 'null'); + const provider: OAuthClientProvider = { + ...createBlobProvider(), + clientInformation: () => read('info'), + saveClientInformation: i => void storage.set('info', JSON.stringify(i)), + tokens: () => read('tokens'), + saveTokens: t => void storage.set('tokens', JSON.stringify(t)) + }; + const srv = createMigratingFetch(); + + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('REDIRECT'); + expect(srv.registerCalls).toEqual([AS_ONE]); + expect(read('info')).toEqual(expect.objectContaining({ client_id: 'cid-as-one.example.com', issuer: AS_ONE })); + + // Registered, still no tokens: a fresh authorization is started again. + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('REDIRECT'); + expect(srv.registerCalls).toEqual([AS_ONE]); + + // Without saveClientInformation the pre-existing registration error is reported unchanged. + storage.clear(); + const preRegisteredOnly: OAuthClientProvider = { ...provider, saveClientInformation: undefined }; + await expect(auth(preRegisteredOnly, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).rejects.toThrow( + 'OAuth client information must be saveable for dynamic registration' + ); + + // fetchToken sends the request without client authentication rather than failing. + await fetchToken(provider, AS_ONE, { + metadata: asMetadata(AS_ONE), + authorizationCode: 'code', + fetchFn: srv.fetchFn + }); + expect(srv.tokenCalls.at(-1)?.authorization).toBeNull(); + }); + + it.each(['https://other.example.com', 42])('an issuer of %j in a token or registration response is not returned', async issuer => { + const srv = createMigratingFetch(); + const fetchFn = async (url: string | URL, init?: RequestInit) => { + const response = await srv.fetchFn(url, init); + return init?.method === 'POST' ? Response.json({ ...(await response.json()), issuer }, response) : response; + }; + const metadata = asMetadata(AS_ONE); + const clientInformation = { client_id: 'cid' }; + const provider: OAuthClientProvider = { + ...createBlobProvider(false), + clientInformation: () => clientInformation, + prepareTokenRequest: () => new URLSearchParams({ grant_type: 'client_credentials' }) + }; + + const results = [ + await registerClient(AS_ONE, { metadata, clientMetadata: { redirect_uris: [] }, fetchFn }), + await exchangeAuthorization(AS_ONE, { + metadata, + clientInformation, + authorizationCode: 'code', + codeVerifier: 'v', + redirectUri: 'http://localhost:3000/callback', + fetchFn + }), + await refreshAuthorization(AS_ONE, { metadata, clientInformation, refreshToken: 'rt', fetchFn }), + await fetchToken(provider, AS_ONE, { metadata, fetchFn }) + ]; + for (const result of results) { + expect(result).not.toHaveProperty('issuer'); + } + }); + + it('a stamp and the authorization server are compared as parsed URLs', async () => { + const requestsSent = async (issuer: string, authorizationServerUrl: string) => { + const srv = createMigratingFetch(); + const provider: OAuthClientProvider = { + ...createBlobProvider(false), + clientInformation: () => ({ client_id: 'cid', client_secret: 'bound-secret', issuer }) + }; + const options = { metadata: asMetadata(AS_ONE), authorizationCode: 'code', fetchFn: srv.fetchFn }; + await fetchToken(provider, authorizationServerUrl, options).catch(() => {}); + return [issuer, authorizationServerUrl, srv.tokenCalls.length]; + }; + + for (const [issuer, url] of [ + ['https://AS-ONE.example.com', AS_ONE], + ['HTTPS://as-one.example.com:443', AS_ONE], + [AS_ONE, 'https://AS-ONE.example.com/'], + [AS_ONE, 'https://as-one.example.com:443'], + ['https://as-one.example.com/a/./b', 'https://as-one.example.com/a/b/'], + ['as-one.example.com', 'as-one.example.com/'] + ]) { + expect(await requestsSent(issuer, url)).toEqual([issuer, url, 1]); + } + + // Another scheme, host, port or path is another authorization server. + for (const [issuer, url] of [ + [AS_ONE, 'http://as-one.example.com'], + [AS_ONE, 'https://as-one.example.com.'], + [AS_ONE, 'https://as-one.example.com.example.org'], + [AS_ONE, 'https://as-one.example.com@example.org'], + [AS_ONE, 'https://a@as-one.example.com'], + [AS_ONE, 'https://as-one.example.com/#f'], + [AS_ONE, 'https://as-one.example.com:8443'], + [AS_ONE, 'https://as-one.example.com/tenant'], + ['https://as-one.example.com/Tenant', 'https://as-one.example.com/tenant'], + ['https://as-one.example.com/tenant', 'https://as-one.example.com/tenant//'], + ['https://as-one.example.com/?tenant=a', 'https://as-one.example.com/?tenant=b'], + ['as-one.example.com', AS_ONE] + ]) { + expect(await requestsSent(issuer, url)).toEqual([issuer, url, 0]); + } + }); + + const notStrings = [{ issuer: 42 }, { issuer: false }, { issuer: { href: AS_ONE } }, { issuer: [AS_ONE] }]; + it.each(notStrings)('a stored issuer of $issuer counts as no stamp', async ({ issuer }) => { + const srv = createMigratingFetch(); + const provider = createBlobProvider(false); + provider.stored.info = { client_id: 'cid', client_secret: 's', issuer } as unknown as OAuthClientInformationMixed; + provider.stored.tokens = { access_token: 'at', token_type: 'Bearer', refresh_token: 'rt', issuer } as unknown as OAuthTokens; + + await fetchToken(provider, AS_ONE, { metadata: asMetadata(AS_ONE), authorizationCode: 'code', fetchFn: srv.fetchFn }); + expect(srv.tokenCalls.map(c => c.authorization)).toEqual([`Basic ${btoa('cid:s')}`]); + + // auth() binds it after its first successful use, as it does for a value with no stamp. + expect(await auth(provider, { serverUrl: SERVER_URL, fetchFn: srv.fetchFn })).toBe('AUTHORIZED'); + expect(provider.stored.info?.issuer).toBe(AS_ONE); + expect(provider.stored.tokens?.issuer).toBe(AS_ONE); + }); + + it('fetchToken uses client information that the provider fills in while preparing the request', async () => { + const srv = createMigratingFetch(); + let filled: OAuthClientInformationMixed | undefined; + const prepareTokenRequest = vi.fn(() => { + filled = { client_id: 'cid', client_secret: 'lazy-secret', issuer: AS_ONE }; + return new URLSearchParams({ grant_type: 'client_credentials' }); + }); + const provider: OAuthClientProvider = { ...createBlobProvider(false), clientInformation: () => filled, prepareTokenRequest }; + + await fetchToken(provider, AS_ONE, { metadata: asMetadata(AS_ONE), fetchFn: srv.fetchFn }); + expect(srv.tokenCalls.map(c => [c.origin, c.authorization])).toEqual([[AS_ONE, `Basic ${btoa('cid:lazy-secret')}`]]); + + // The value read after preparing goes through the same check. + filled = undefined; + await expect(fetchToken(provider, AS_TWO, { metadata: asMetadata(AS_TWO), fetchFn: srv.fetchFn })).rejects.toThrow( + `OAuth client information is bound to authorization server ${AS_ONE}` + ); + expect(srv.tokenCalls).toHaveLength(1); + + // A value that is there before preparing is checked before the request is prepared. + prepareTokenRequest.mockClear(); + await expect(fetchToken(provider, AS_TWO, { metadata: asMetadata(AS_TWO), fetchFn: srv.fetchFn })).rejects.toThrow( + `OAuth client information is bound to authorization server ${AS_ONE}` + ); + expect(prepareTokenRequest).not.toHaveBeenCalled(); + expect(srv.tokenCalls).toHaveLength(1); }); }); }); diff --git a/test/client/sse.test.ts b/test/client/sse.test.ts index 92ca5a8833..0b1a1364a6 100644 --- a/test/client/sse.test.ts +++ b/test/client/sse.test.ts @@ -776,7 +776,8 @@ describe('SSEClientTransport', () => { expect(mockAuthProvider.saveTokens).toHaveBeenCalledWith({ access_token: 'new-token', token_type: 'Bearer', - refresh_token: 'new-refresh-token' + refresh_token: 'new-refresh-token', + issuer: `${authBaseUrl}` }); expect(connectionAttempts).toBe(1); expect(lastServerRequest.headers.authorization).toBe('Bearer new-token'); @@ -928,7 +929,8 @@ describe('SSEClientTransport', () => { expect(mockAuthProvider.saveTokens).toHaveBeenCalledWith({ access_token: 'new-token', token_type: 'Bearer', - refresh_token: 'new-refresh-token' + refresh_token: 'new-refresh-token', + issuer: `${authBaseUrl}` }); expect(postAttempts).toBe(1); expect(lastServerRequest.headers.authorization).toBe('Bearer new-token'); @@ -1515,7 +1517,8 @@ describe('SSEClientTransport', () => { access_token: 'new-access-token', token_type: 'Bearer', expires_in: 3600, - refresh_token: 'new-refresh-token' + refresh_token: 'new-refresh-token', + issuer: authBaseUrl.href }); // Global fetch should never have been called diff --git a/test/client/streamableHttp.test.ts b/test/client/streamableHttp.test.ts index 52c8f10748..3c7a2ff964 100644 --- a/test/client/streamableHttp.test.ts +++ b/test/client/streamableHttp.test.ts @@ -1346,7 +1346,8 @@ describe('StreamableHTTPClientTransport', () => { access_token: 'new-access-token', token_type: 'Bearer', expires_in: 3600, - refresh_token: 'new-refresh-token' + refresh_token: 'new-refresh-token', + issuer: 'http://localhost:1234' }); // Global fetch should never have been called @@ -1619,7 +1620,8 @@ describe('StreamableHTTPClientTransport', () => { access_token: 'new-access-token', token_type: 'Bearer', expires_in: 3600, - refresh_token: 'refresh-token' // Refresh token is preserved + refresh_token: 'refresh-token', // Refresh token is preserved + issuer: 'http://localhost:1234/' }); }); }); diff --git a/test/e2e/scenarios/client-auth.test.ts b/test/e2e/scenarios/client-auth.test.ts index 5e9fb6fdb3..0bf6378523 100644 --- a/test/e2e/scenarios/client-auth.test.ts +++ b/test/e2e/scenarios/client-auth.test.ts @@ -612,7 +612,7 @@ verifies('client-auth:client-credentials', async (_args: TestArgs) => { return baseFetch(url, init); }; - const provider = new ClientCredentialsProvider({ clientId: CLIENT_ID, clientSecret: CLIENT_SECRET }); + const provider = new ClientCredentialsProvider({ expectedIssuer: ISSUER, clientId: CLIENT_ID, clientSecret: CLIENT_SECRET }); const client = new Client({ name: 'c', version: '0' }); const transport = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); @@ -844,7 +844,12 @@ verifies('client-auth:private-key-jwt', async (_args: TestArgs) => { const privateKeyPem = keyPair.privateKey.export({ type: 'pkcs8', format: 'pem' }).toString(); const publicKeyPem = keyPair.publicKey.export({ type: 'spki', format: 'pem' }).toString(); - const provider = new PrivateKeyJwtProvider({ clientId: CLIENT_ID, privateKey: privateKeyPem, algorithm: 'RS256' }); + const provider = new PrivateKeyJwtProvider({ + expectedIssuer: ISSUER, + clientId: CLIENT_ID, + privateKey: privateKeyPem, + algorithm: 'RS256' + }); const client = new Client({ name: 'c', version: '0' }); const transport = new StreamableHTTPClientTransport(new URL(MCP_URL), { authProvider: provider, fetch: combinedFetch }); @@ -1282,6 +1287,7 @@ verifies('client-auth:private-key-jwt:static-assertion', async (_args: TestArgs) const combinedFetch = createCombinedFetch({ as, mcpHost, validToken: ISSUED }); const provider = new StaticPrivateKeyJwtProvider({ + expectedIssuer: ISSUER, clientId: CLIENT_ID, jwtBearerAssertion: preBuiltJwt }); diff --git a/test/server/auth/providers/proxyProvider.test.ts b/test/server/auth/providers/proxyProvider.test.ts index 40fb55d572..30975fcd8b 100644 --- a/test/server/auth/providers/proxyProvider.test.ts +++ b/test/server/auth/providers/proxyProvider.test.ts @@ -225,6 +225,23 @@ describe('Proxy OAuth Server Provider', () => { ); expect(tokens).toEqual(mockTokenResponse); }); + + it.each(['https://upstream.example.com', 42])('does not forward an issuer of %j from the upstream token response', async issuer => { + (global.fetch as Mock).mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ ...mockTokenResponse, issuer }) + }) + ); + + const exchanged = await provider.exchangeAuthorizationCode(validClient, 'test-code', 'test-verifier'); + const refreshed = await provider.exchangeRefreshToken(validClient, 'test-refresh-token'); + + expect(exchanged).toEqual(mockTokenResponse); + expect(exchanged).not.toHaveProperty('issuer'); + expect(refreshed).toEqual(mockTokenResponse); + expect(refreshed).not.toHaveProperty('issuer'); + }); }); describe('client registration', () => { @@ -256,6 +273,28 @@ describe('Proxy OAuth Server Provider', () => { expect(result).toEqual(newClient); }); + it.each(['https://upstream.example.com', 42])( + 'does not forward an issuer of %j from the upstream registration response', + async issuer => { + const newClient: OAuthClientInformationFull = { + client_id: 'new-client', + redirect_uris: ['https://new-client.com/callback'] + }; + + (global.fetch as Mock).mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve({ ...newClient, issuer }) + }) + ); + + const result = await provider.clientsStore.registerClient!(newClient); + + expect(result).toEqual(newClient); + expect(result).not.toHaveProperty('issuer'); + } + ); + it('handles registration failure', async () => { mockFailedResponse(); const newClient: OAuthClientInformationFull = {