From e848b155f1869716d60ffe0b6a24ccd55b8bf69e Mon Sep 17 00:00:00 2001 From: Matt Carey Date: Tue, 9 Jun 2026 21:42:35 +0100 Subject: [PATCH] fix(client): invalidate credentials and re-register when the authorization server changes (SEP-2352) --- .changeset/sep-2352-as-binding.md | 5 + packages/client/src/client/auth.ts | 59 +++++++ packages/client/test/client/auth.test.ts | 209 +++++++++++++++++++++++ 3 files changed, 273 insertions(+) create mode 100644 .changeset/sep-2352-as-binding.md diff --git a/.changeset/sep-2352-as-binding.md b/.changeset/sep-2352-as-binding.md new file mode 100644 index 0000000000..bd5c90a06c --- /dev/null +++ b/.changeset/sep-2352-as-binding.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Implement SEP-2352 authorization server binding: when OAuth discovery shows the authorization server has changed since client credentials were recorded, `auth()` now invalidates the stale client registration and tokens (`invalidateCredentials('client')` / `('tokens')`) and re-registers with the new authorization server. CIMD (HTTPS URL) client IDs are exempt, as they are portable across authorization servers. Provider implementations should persist client credentials keyed by the authorization server's `issuer` identifier. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 5f55fb7a08..18773af3e8 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -167,6 +167,12 @@ 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. + * + * Per SEP-2352 (authorization server binding), implementations that persist + * client credentials SHOULD key them by the authorization server's `issuer` + * identifier, and SHOULD NOT return credentials that were issued by a + * different authorization server. CIMD (HTTPS URL) client IDs are exempt: + * they are portable across authorization servers. */ clientInformation(): OAuthClientInformationMixed | undefined | Promise; @@ -177,6 +183,11 @@ export interface OAuthClientProvider { * * This method is not required to be implemented if client information is * statically known (e.g., pre-registered). + * + * Per SEP-2352 (authorization server binding), implementations SHOULD persist + * client credentials keyed by the authorization server's `issuer` identifier, + * so credentials registered with one authorization server are never reused + * with another. */ saveClientInformation?(clientInformation: OAuthClientInformationMixed): void | Promise; @@ -681,6 +692,40 @@ async function authInternal( }); } + // SEP-2352: Authorization server binding. Client credentials are bound to the + // authorization server that issued them; when discovery shows the authorization + // server has changed (e.g., via updated protected resource metadata), stale client + // credentials and tokens MUST NOT be reused and the client MUST re-register. + // + // Canonical comparison key: the validated authorization server metadata `issuer` + // (the identifier SEP-2352 specifies), falling back to the authorization server URL + // when metadata is unavailable. Under RFC 8414 the issuer and the URL used for + // discovery coincide, so a match on either is treated as the same authorization + // server to avoid false-positive invalidation. + const previousAuthServerIdentities = [ + cachedState?.authorizationServerMetadata?.issuer, + cachedState?.authorizationServerUrl, + await provider.authorizationServerUrl?.() + ] + .filter((value): value is string => typeof value === 'string' && value.length > 0) + .map(value => normalizeAuthorizationServerIdentity(value)); + const currentAuthServerIdentities = [metadata?.issuer, String(authorizationServerUrl)] + .filter((value): value is string => typeof value === 'string' && value.length > 0) + .map(value => normalizeAuthorizationServerIdentity(value)); + const authorizationServerChanged = + previousAuthServerIdentities.length > 0 && + !currentAuthServerIdentities.some(identity => previousAuthServerIdentities.includes(identity)); + + if (authorizationServerChanged) { + const staleClientInformation = await Promise.resolve(provider.clientInformation()); + // CIMD (URL-based) client IDs are portable across authorization servers + // (SEP-991/SEP-2352) — no invalidation or re-registration is needed. + if (staleClientInformation && !isHttpsUrl(staleClientInformation.client_id)) { + await provider.invalidateCredentials?.('client'); + await provider.invalidateCredentials?.('tokens'); + } + } + // Save authorization server URL for providers that need it (e.g., CrossAppAccessProvider) await provider.saveAuthorizationServerUrl?.(String(authorizationServerUrl)); @@ -840,6 +885,20 @@ export function isHttpsUrl(value?: string): boolean { } } +/** + * SEP-2352: Normalizes an authorization server identity (issuer identifier or + * authorization server URL) for comparison, so that textual variations of the + * same URL (e.g. a missing trailing slash on an origin-only issuer) do not + * register as an authorization server change. + */ +function normalizeAuthorizationServerIdentity(value: string): string { + try { + return new URL(value).href; + } catch { + return value; + } +} + export async function selectResourceURL( serverUrl: string | URL, provider: OAuthClientProvider, diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 04d7f4a3fb..4603404fea 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -4131,3 +4131,212 @@ describe('OAuth Authorization', () => { }); }); }); + +describe('SEP-2352: authorization server binding', () => { + const oldAuthServerUrl = 'https://old-auth.example.com'; + + const newResourceMetadata = { + resource: 'https://resource.example.com', + authorization_servers: ['https://new-auth.example.com'] + }; + + const newAuthMetadata = { + issuer: 'https://new-auth.example.com', + authorization_endpoint: 'https://new-auth.example.com/authorize', + token_endpoint: 'https://new-auth.example.com/token', + registration_endpoint: 'https://new-auth.example.com/register', + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] + }; + + const sameResourceMetadata = { + resource: 'https://resource.example.com', + authorization_servers: [oldAuthServerUrl] + }; + + const sameAuthMetadata = { + issuer: oldAuthServerUrl, + authorization_endpoint: `${oldAuthServerUrl}/authorize`, + token_endpoint: `${oldAuthServerUrl}/token`, + registration_endpoint: `${oldAuthServerUrl}/register`, + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] + }; + + /** + * Creates a provider that previously completed an OAuth flow against + * `oldAuthServerUrl` (recorded via `authorizationServerUrl()`), holds stored + * client credentials, and honors `invalidateCredentials` by dropping them. + */ + function createBoundProvider(initialClientInformation: { client_id: string; client_secret?: string }): { + provider: OAuthClientProvider; + invalidateCredentials: Mock; + saveClientInformation: Mock; + redirectToAuthorization: Mock; + } { + let clientInformation: { client_id: string; client_secret?: string } | undefined = initialClientInformation; + + const invalidateCredentials = vi.fn(async (scope: 'all' | 'client' | 'tokens' | 'verifier' | 'discovery') => { + if (scope === 'all' || scope === 'client') { + clientInformation = undefined; + } + }); + const saveClientInformation = vi.fn(async (info: { client_id: string; client_secret?: string }) => { + clientInformation = info; + }); + const redirectToAuthorization = vi.fn(); + + const provider: OAuthClientProvider = { + get redirectUrl() { + return 'http://localhost:3000/callback'; + }, + get clientMetadata() { + return { + redirect_uris: ['http://localhost:3000/callback'], + client_name: 'Test Client' + }; + }, + clientInformation: vi.fn(async () => clientInformation), + saveClientInformation, + tokens: vi.fn().mockResolvedValue(undefined), + saveTokens: vi.fn(), + redirectToAuthorization, + saveCodeVerifier: vi.fn(), + codeVerifier: vi.fn().mockResolvedValue('test_verifier'), + authorizationServerUrl: vi.fn().mockResolvedValue(oldAuthServerUrl), + invalidateCredentials + }; + + return { provider, invalidateCredentials, saveClientInformation, redirectToAuthorization }; + } + + function mockDiscoveryAndRegistration(options: { + resourceMetadata: { resource: string; authorization_servers: string[] }; + authMetadata: { issuer: string }; + registeredClient?: { client_id: string; client_secret?: string }; + }): void { + mockFetch.mockImplementation((url, init) => { + const urlString = url.toString(); + + if (urlString.includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => options.resourceMetadata + }); + } + + if (urlString.includes('/.well-known/oauth-authorization-server')) { + return Promise.resolve({ + ok: true, + status: 200, + json: async () => options.authMetadata + }); + } + + if (urlString.includes('/register') && init?.method === 'POST') { + if (!options.registeredClient) { + return Promise.reject(new Error(`Unexpected registration request: ${urlString}`)); + } + return Promise.resolve({ + ok: true, + status: 201, + json: async () => ({ + ...JSON.parse(init.body as string), + ...options.registeredClient + }) + }); + } + + return Promise.reject(new Error(`Unexpected fetch: ${urlString}`)); + }); + } + + beforeEach(() => { + mockFetch.mockReset(); + vi.clearAllMocks(); + }); + + it('invalidates client credentials and tokens, then re-registers, when the authorization server changes', async () => { + const { provider, invalidateCredentials, saveClientInformation, redirectToAuthorization } = createBoundProvider({ + client_id: 'old-client-id', + client_secret: 'old-client-secret' + }); + + mockDiscoveryAndRegistration({ + resourceMetadata: newResourceMetadata, + authMetadata: newAuthMetadata, + registeredClient: { client_id: 'new-client-id', client_secret: 'new-client-secret' } + }); + + const result = await auth(provider, { serverUrl: 'https://resource.example.com' }); + + expect(result).toBe('REDIRECT'); + + // Stale credentials bound to the old authorization server are invalidated + expect(invalidateCredentials).toHaveBeenCalledWith('client'); + expect(invalidateCredentials).toHaveBeenCalledWith('tokens'); + + // The client re-registers with the new authorization server + const registrationCalls = mockFetch.mock.calls.filter(call => call[0].toString().includes('/register')); + expect(registrationCalls).toHaveLength(1); + expect(registrationCalls[0]![0].toString()).toBe('https://new-auth.example.com/register'); + expect(saveClientInformation).toHaveBeenCalledWith(expect.objectContaining({ client_id: 'new-client-id' })); + + // The authorization redirect uses the newly registered client, not the stale one + const redirectUrl: URL = redirectToAuthorization.mock.calls[0]![0]; + expect(redirectUrl.origin).toBe('https://new-auth.example.com'); + expect(redirectUrl.searchParams.get('client_id')).toBe('new-client-id'); + }); + + it('does not invalidate credentials when the authorization server is unchanged', async () => { + const { provider, invalidateCredentials, redirectToAuthorization } = createBoundProvider({ + client_id: 'old-client-id', + client_secret: 'old-client-secret' + }); + + mockDiscoveryAndRegistration({ + resourceMetadata: sameResourceMetadata, + authMetadata: sameAuthMetadata + }); + + const result = await auth(provider, { serverUrl: 'https://resource.example.com' }); + + expect(result).toBe('REDIRECT'); + expect(invalidateCredentials).not.toHaveBeenCalled(); + + // No re-registration; the existing client credentials are reused + const registrationCalls = mockFetch.mock.calls.filter(call => call[0].toString().includes('/register')); + expect(registrationCalls).toHaveLength(0); + + const redirectUrl: URL = redirectToAuthorization.mock.calls[0]![0]; + expect(redirectUrl.searchParams.get('client_id')).toBe('old-client-id'); + }); + + it('does not invalidate CIMD (HTTPS URL) client IDs when the authorization server changes', async () => { + const cimdClientId = 'https://client.example.com/oauth/client-metadata.json'; + const { provider, invalidateCredentials, redirectToAuthorization } = createBoundProvider({ + client_id: cimdClientId + }); + + mockDiscoveryAndRegistration({ + resourceMetadata: newResourceMetadata, + authMetadata: newAuthMetadata + }); + + const result = await auth(provider, { serverUrl: 'https://resource.example.com' }); + + expect(result).toBe('REDIRECT'); + + // CIMD client IDs are portable across authorization servers — no invalidation + expect(invalidateCredentials).not.toHaveBeenCalled(); + + // No re-registration; the portable client ID is reused with the new server + const registrationCalls = mockFetch.mock.calls.filter(call => call[0].toString().includes('/register')); + expect(registrationCalls).toHaveLength(0); + + const redirectUrl: URL = redirectToAuthorization.mock.calls[0]![0]; + expect(redirectUrl.origin).toBe('https://new-auth.example.com'); + expect(redirectUrl.searchParams.get('client_id')).toBe(cimdClientId); + }); +});