From a8335db24d824d248d1c13ff4006a0852cf7a0d7 Mon Sep 17 00:00:00 2001 From: Charles McMillan Date: Fri, 4 Sep 2026 14:27:02 -0400 Subject: [PATCH 1/3] fix(client): deduplicate concurrent OAuth refreshes --- .../deduplicate-concurrent-oauth-refresh.md | 8 + packages/client/src/client/auth.ts | 21 ++ packages/client/test/client/auth.test.ts | 201 ++++++++++++++++++ 3 files changed, 230 insertions(+) create mode 100644 .changeset/deduplicate-concurrent-oauth-refresh.md diff --git a/.changeset/deduplicate-concurrent-oauth-refresh.md b/.changeset/deduplicate-concurrent-oauth-refresh.md new file mode 100644 index 0000000000..5b8a046ecd --- /dev/null +++ b/.changeset/deduplicate-concurrent-oauth-refresh.md @@ -0,0 +1,8 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Deduplicate concurrent OAuth refresh flows for the same provider. Parallel 401 handlers now +share one in-flight refresh instead of redeeming the same rotating refresh token multiple times. +Authorization-code exchanges and forced reauthorization bypass deduplication because they carry +distinct, one-time authorization state. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 4c37339297..cfc14d69c6 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1036,6 +1036,8 @@ function warnCredentialInvalidation(provider: OAuthClientProvider, error: OAuthE console.warn(`[mcp-sdk] OAuth ${JSON.stringify(error.code)} — ${action}. Cause: ${JSON.stringify(error.message)}`); } +const pendingAuthRequests = new WeakMap>(); + /** * Orchestrates the full auth flow with a server. * @@ -1043,6 +1045,25 @@ function warnCredentialInvalidation(provider: OAuthClientProvider, error: OAuthE * instead of linking together the other lower-level functions in this module. */ export async function auth(provider: OAuthClientProvider, options: AuthOptions): Promise { + if (options.authorizationCode !== undefined || options.forceReauthorization === true) { + return authWithErrorHandling(provider, options); + } + + const pending = pendingAuthRequests.get(provider); + if (pending) return pending; + + const request = authWithErrorHandling(provider, options); + pendingAuthRequests.set(provider, request); + try { + return await request; + } finally { + if (pendingAuthRequests.get(provider) === request) { + pendingAuthRequests.delete(provider); + } + } +} + +async function authWithErrorHandling(provider: OAuthClientProvider, options: AuthOptions): Promise { try { return await authInternal(provider, options); } catch (error) { diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 3ac9c7ddff..5858082b3c 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -3049,6 +3049,207 @@ describe('OAuth Authorization', () => { expect(body.get('refresh_token')).toBe('refresh123'); }); + describe('concurrent auth deduplication', () => { + const configureRefresh = (tokenResponse: (body: URLSearchParams) => Promise | Response): void => { + mockFetch.mockImplementation((url, init) => { + const urlString = url.toString(); + if (urlString.includes('/.well-known/oauth-protected-resource')) { + return Promise.resolve( + Response.json({ + resource: 'https://api.example.com/mcp-server', + authorization_servers: ['https://auth.example.com'] + }) + ); + } + if (urlString.includes('/.well-known/oauth-authorization-server')) { + return Promise.resolve( + Response.json({ + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/authorize', + token_endpoint: 'https://auth.example.com/token', + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] + }) + ); + } + if (urlString.includes('/token')) { + return Promise.resolve(tokenResponse(init?.body as URLSearchParams)); + } + return Promise.resolve(Response.json({}, { status: 404 })); + }); + (mockProvider.clientInformation as Mock).mockResolvedValue({ client_id: 'test-client', client_secret: 'test-secret' }); + (mockProvider.tokens as Mock).mockResolvedValue({ + access_token: 'expired-access', + refresh_token: 'current-refresh', + issuer: 'https://auth.example.com' + }); + (mockProvider.saveTokens as Mock).mockResolvedValue(undefined); + (mockProvider.codeVerifier as Mock).mockResolvedValue('test-verifier'); + }; + + it('deduplicates concurrent refreshes for the same provider', async () => { + let tokenRequests = 0; + let release!: () => void; + const gate = new Promise(resolve => { + release = resolve; + }); + configureRefresh(async () => { + tokenRequests++; + await gate; + return Response.json({ + access_token: 'new-access', + refresh_token: 'new-refresh', + token_type: 'Bearer', + expires_in: 3600 + }); + }); + + const requests = [ + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }) + ]; + release(); + + await expect(Promise.all(requests)).resolves.toEqual(['AUTHORIZED', 'AUTHORIZED', 'AUTHORIZED']); + expect(tokenRequests).toBe(1); + expect(mockProvider.saveTokens).toHaveBeenCalledTimes(1); + }); + + it('allows a new refresh after the previous one completes', async () => { + let tokenRequests = 0; + configureRefresh(() => { + tokenRequests++; + return Response.json({ + access_token: `access-${tokenRequests}`, + refresh_token: `refresh-${tokenRequests}`, + token_type: 'Bearer', + expires_in: 3600 + }); + }); + + await auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }); + await auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }); + + expect(tokenRequests).toBe(2); + }); + + it('does not coalesce an authorization-code exchange with an in-flight refresh', async () => { + let releaseRefresh!: () => void; + let refreshStarted!: () => void; + const refreshGate = new Promise(resolve => { + releaseRefresh = resolve; + }); + const started = new Promise(resolve => { + refreshStarted = resolve; + }); + let codeRequests = 0; + configureRefresh(async body => { + if (body.get('grant_type') === 'refresh_token') { + refreshStarted(); + await refreshGate; + } else { + codeRequests++; + } + return Response.json({ access_token: 'new-access', refresh_token: 'new-refresh', token_type: 'Bearer' }); + }); + + const refresh = auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }); + await started; + const codeExchange = auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server', + authorizationCode: 'auth-code' + }); + + await expect(codeExchange).resolves.toBe('AUTHORIZED'); + expect(codeRequests).toBe(1); + releaseRefresh(); + await expect(refresh).resolves.toBe('AUTHORIZED'); + }); + + it('does not coalesce forced reauthorization with an in-flight refresh', async () => { + let releaseRefresh!: () => void; + let refreshStarted!: () => void; + const refreshGate = new Promise(resolve => { + releaseRefresh = resolve; + }); + const started = new Promise(resolve => { + refreshStarted = resolve; + }); + configureRefresh(async body => { + if (body.get('grant_type') === 'refresh_token') { + refreshStarted(); + await refreshGate; + } + return Response.json({ access_token: 'new-access', refresh_token: 'new-refresh', token_type: 'Bearer' }); + }); + + const refresh = auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }); + await started; + await expect( + auth(mockProvider, { + serverUrl: 'https://api.example.com/mcp-server', + forceReauthorization: true + }) + ).resolves.toBe('REDIRECT'); + expect(mockProvider.redirectToAuthorization).toHaveBeenCalledTimes(1); + releaseRefresh(); + await expect(refresh).resolves.toBe('AUTHORIZED'); + }); + + it('does not deduplicate different providers', async () => { + let tokenRequests = 0; + configureRefresh(() => { + tokenRequests++; + return Response.json({ access_token: 'new-access', refresh_token: 'new-refresh', token_type: 'Bearer' }); + }); + const otherProvider: OAuthClientProvider = { + ...mockProvider, + clientInformation: vi.fn().mockResolvedValue({ client_id: 'other-client', client_secret: 'other-secret' }), + tokens: vi.fn().mockResolvedValue({ + access_token: 'other-expired-access', + refresh_token: 'other-current-refresh', + issuer: 'https://auth.example.com' + }), + saveTokens: vi.fn().mockResolvedValue(undefined) + }; + + await expect( + Promise.all([ + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), + auth(otherProvider, { serverUrl: 'https://api.example.com/mcp-server' }) + ]) + ).resolves.toEqual(['AUTHORIZED', 'AUTHORIZED']); + expect(tokenRequests).toBe(2); + }); + + it('propagates one refresh failure to every concurrent caller', async () => { + const persistError = new Error('credential store unavailable'); + let tokenRequests = 0; + configureRefresh(() => { + tokenRequests++; + return Response.json({ access_token: 'new-access', refresh_token: 'new-refresh', token_type: 'Bearer' }); + }); + (mockProvider.saveTokens as Mock).mockRejectedValueOnce(persistError); + + const results = await Promise.allSettled([ + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), + auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }) + ]); + + expect(results).toEqual([ + { status: 'rejected', reason: persistError }, + { status: 'rejected', reason: persistError }, + { status: 'rejected', reason: persistError } + ]); + expect(tokenRequests).toBe(1); + + await expect(auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' })).resolves.toBe('AUTHORIZED'); + expect(tokenRequests).toBe(2); + }); + }); + // The #2034 tests below differ only in how the token endpoint answers, so the // discovery fixture is shared. `tokenResponse` is invoked per POST to /token. const mockDiscoveryWithTokenEndpoint = (tokenResponse: () => unknown): void => { From fc8927bcecb0f42450d0e75975139dadf0463742 Mon Sep 17 00:00:00 2001 From: Charles McMillan Date: Sun, 6 Sep 2026 16:14:50 -0400 Subject: [PATCH 2/3] fix(client): preserve OAuth transaction ownership and request options Wrap complete auth recovery in an optional provider transaction. Serialize incompatible ordinary calls without dropping their options, while code exchange and forced reauthorization continue to bypass the queue. Normalize null error descriptions only at the client parser boundary. --- .../deduplicate-concurrent-oauth-refresh.md | 16 +- packages/client/src/client/auth.ts | 58 ++- .../test/client/auth.transactions.test.ts | 331 ++++++++++++++++++ 3 files changed, 394 insertions(+), 11 deletions(-) create mode 100644 packages/client/test/client/auth.transactions.test.ts diff --git a/.changeset/deduplicate-concurrent-oauth-refresh.md b/.changeset/deduplicate-concurrent-oauth-refresh.md index 5b8a046ecd..c7271752e0 100644 --- a/.changeset/deduplicate-concurrent-oauth-refresh.md +++ b/.changeset/deduplicate-concurrent-oauth-refresh.md @@ -2,7 +2,15 @@ '@modelcontextprotocol/client': patch --- -Deduplicate concurrent OAuth refresh flows for the same provider. Parallel 401 handlers now -share one in-flight refresh instead of redeeming the same rotating refresh token multiple times. -Authorization-code exchanges and forced reauthorization bypass deduplication because they carry -distinct, one-time authorization state. +Deduplicate concurrent OAuth flows only when the provider and request options match. Ordinary +calls with different resources, scopes, discovery validation settings, metadata URLs, or fetch +functions run serially and read credentials after the previous operation settles, including failures. +Authorization-code exchanges and forced reauthorization still bypass this queue. + +Add the optional `OAuthClientProvider.withAuthTransaction` hook so hosts can protect a complete +auth operation, including credential invalidation and recovery retries, with their own lock. +Code exchanges and forced reauthorization also invoke the hook independently. + +Treat `error_description: null` as an omitted description when parsing an OAuth error response. +This allows `invalid_grant` recovery without relaxing the public OAuth error schema or accepting +malformed values in other fields. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index cfc14d69c6..f0b1a0a0ac 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -241,6 +241,21 @@ export function adaptOAuthProvider( * No changes are needed to existing implementations. */ export interface OAuthClientProvider { + /** + * Optionally runs a complete {@linkcode auth} operation inside a host-managed + * credential transaction (for example, a cross-process lock). The operation + * includes credential reads, writes, invalidation, and recovery retries. + * Authorization-code exchanges and forced reauthorization also use this hook, + * but are never deduplicated by the SDK. + * + * Implementations must invoke and await `operation`, returning its result and + * releasing any lock in `finally`. Do not release the lock when an outer wait + * is cancelled while the underlying operation is still running. The operation + * returns `REDIRECT` after initiating authorization; it does not hold a lock + * across the human/browser authorization wait. + */ + withAuthTransaction?(operation: () => Promise): Promise; + /** * The URL to redirect the user agent to after authorization. * Return `undefined` for non-interactive flows that don't require user interaction @@ -950,7 +965,19 @@ export async function parseErrorResponse(input: Response | string): Promise>(); +const pendingAuthRequests = new WeakMap }>(); + +function equivalentAuthOptions(a: AuthOptions, b: AuthOptions): boolean { + return ( + String(a.serverUrl) === String(b.serverUrl) && + a.resourceMetadataUrl?.toString() === b.resourceMetadataUrl?.toString() && + a.scope === b.scope && + (a.skipIssuerMetadataValidation ?? false) === (b.skipIssuerMetadataValidation ?? false) && + a.fetchFn === b.fetchFn + ); +} /** * Orchestrates the full auth flow with a server. @@ -1045,19 +1082,26 @@ const pendingAuthRequests = new WeakMap * instead of linking together the other lower-level functions in this module. */ export async function auth(provider: OAuthClientProvider, options: AuthOptions): Promise { + const execute = (): Promise => { + const operation = (): Promise => authWithErrorHandling(provider, options); + return provider.withAuthTransaction ? provider.withAuthTransaction(operation) : operation(); + }; if (options.authorizationCode !== undefined || options.forceReauthorization === true) { - return authWithErrorHandling(provider, options); + return execute(); } const pending = pendingAuthRequests.get(provider); - if (pending) return pending; + if (pending && equivalentAuthOptions(pending.options, options)) return pending.request; - const request = authWithErrorHandling(provider, options); - pendingAuthRequests.set(provider, request); + // Incompatible ordinary calls must use their own options and read credentials + // after the previous operation settles, even if it failed. Track the queue tail + // so equivalent callers can also share an operation that has not started yet. + const request = pending ? pending.request.then(execute, execute) : Promise.resolve().then(execute); + pendingAuthRequests.set(provider, { options, request }); try { return await request; } finally { - if (pendingAuthRequests.get(provider) === request) { + if (pendingAuthRequests.get(provider)?.request === request) { pendingAuthRequests.delete(provider); } } diff --git a/packages/client/test/client/auth.transactions.test.ts b/packages/client/test/client/auth.transactions.test.ts new file mode 100644 index 0000000000..5060f08007 --- /dev/null +++ b/packages/client/test/client/auth.transactions.test.ts @@ -0,0 +1,331 @@ +import type { StoredOAuthTokens } from '@modelcontextprotocol/core-internal'; +import { OAuthErrorCode, OAuthErrorResponseSchema } from '@modelcontextprotocol/core-internal'; +import { describe, expect, it, vi } from 'vitest'; + +import type { AuthOptions, AuthResult, OAuthClientProvider } from '../../src/client/auth'; +import { auth, IssuerMismatchError, parseErrorResponse } from '../../src/client/auth'; + +const issuer = 'https://auth.example.com'; +const serverUrl = 'https://api.example.com/mcp'; +const metadata = { + issuer, + authorization_endpoint: `${issuer}/authorize`, + token_endpoint: `${issuer}/token`, + response_types_supported: ['code'], + code_challenge_methods_supported: ['S256'] +}; + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void; + const promise = new Promise(r => { + resolve = r; + }); + return { promise, resolve }; +} + +function fixture() { + let tokens: StoredOAuthTokens | undefined = { access_token: 'expired', refresh_token: 'refresh-0', token_type: 'Bearer', issuer }; + const posts: URLSearchParams[] = []; + const tokenResponse = vi.fn( + async (): Promise => + Response.json({ access_token: 'fresh', refresh_token: `refresh-${posts.length}`, token_type: 'Bearer' }) + ); + const fetchFn = vi.fn(async (input: string | URL | Request, init?: RequestInit): Promise => { + const url = new URL(String(input)); + if (url.pathname.includes('oauth-protected-resource')) { + return Response.json({ + resource: `${url.origin}${url.pathname.replace('/.well-known/oauth-protected-resource', '')}`, + authorization_servers: [issuer] + }); + } + if (url.pathname.includes('oauth-authorization-server')) return Response.json(metadata); + if (url.href === `${issuer}/token`) { + expect(init?.method).toBe('POST'); + posts.push(new URLSearchParams(init?.body as URLSearchParams)); + return tokenResponse(); + } + throw new Error(`Unexpected request: ${url}`); + }); + const provider: OAuthClientProvider = { + redirectUrl: 'https://client.example.com/callback', + clientMetadata: { redirect_uris: ['https://client.example.com/callback'] }, + clientInformation: () => ({ client_id: 'client', issuer }), + tokens: () => tokens, + saveTokens: vi.fn(value => { + tokens = value; + }), + invalidateCredentials: vi.fn(scope => { + if (scope === 'tokens') tokens = undefined; + }), + redirectToAuthorization: vi.fn(), + saveCodeVerifier: vi.fn(), + codeVerifier: () => 'verifier' + }; + const options: AuthOptions = { serverUrl, fetchFn }; + return { provider, options, posts, tokenResponse, fetchFn }; +} + +describe('OAuth error response boundary', () => { + it.each(['string', 'Response'])('accepts a null description from %s without relaxing the public schema', async kind => { + const body = { error: 'invalid_grant', error_description: null }; + expect(OAuthErrorResponseSchema.safeParse(body).success).toBe(false); + const error = await parseErrorResponse(kind === 'string' ? JSON.stringify(body) : Response.json(body, { status: 400 })); + expect(error.code).toBe(OAuthErrorCode.InvalidGrant); + }); + + it.each([ + null, + [], + [{ error: 'invalid_grant', error_description: null }], + { error: null, error_description: null }, + { error: 'invalid_grant', error_description: 42 }, + { error: 'invalid_grant', error_description: null, error_uri: 42 } + ])('keeps malformed payloads as ServerError: %j', async body => { + expect((await parseErrorResponse(JSON.stringify(body))).code).toBe(OAuthErrorCode.ServerError); + }); +}); + +describe('auth transactions', () => { + it('recovers concurrent invalid_grant-null callers within one complete transaction', async () => { + const { provider, options, posts, tokenResponse } = fixture(); + const events: string[] = []; + let active = false; + provider.withAuthTransaction = async operation => { + events.push('enter'); + active = true; + try { + return await operation(); + } finally { + active = false; + events.push('exit'); + } + }; + const invalidate = provider.invalidateCredentials!; + provider.invalidateCredentials = vi.fn(async scope => { + expect(active).toBe(true); + events.push(`invalidate:${scope}`); + await invalidate(scope); + }); + provider.redirectToAuthorization = vi.fn(() => { + expect(active).toBe(true); + events.push('redirect'); + }); + tokenResponse.mockImplementation(async () => { + expect(active).toBe(true); + events.push('refresh'); + return Response.json({ error: 'invalid_grant', error_description: null }, { status: 400 }); + }); + const warning = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + await expect(Promise.all([auth(provider, options), auth(provider, options), auth(provider, options)])).resolves.toEqual([ + 'REDIRECT', + 'REDIRECT', + 'REDIRECT' + ]); + } finally { + warning.mockRestore(); + } + expect(posts).toHaveLength(1); + expect(posts[0]!.get('grant_type')).toBe('refresh_token'); + expect(provider.invalidateCredentials).toHaveBeenCalledExactlyOnceWith('tokens'); + expect(provider.redirectToAuthorization).toHaveBeenCalledTimes(1); + expect(events).toEqual(['enter', 'refresh', 'invalidate:tokens', 'redirect', 'exit']); + }); + + it.each([false, true])('refreshes with transaction hook present=%s', async present => { + const { provider, options, posts } = fixture(); + let active = false; + const hook = vi.fn(async (operation: () => Promise) => { + active = true; + try { + return await operation(); + } finally { + active = false; + } + }); + if (present) provider.withAuthTransaction = hook; + const save = provider.saveTokens; + provider.saveTokens = value => { + expect(active).toBe(present); + return save(value); + }; + await expect(Promise.all([auth(provider, options), auth(provider, options)])).resolves.toEqual(['AUTHORIZED', 'AUTHORIZED']); + expect(posts).toHaveLength(1); + expect(hook).toHaveBeenCalledTimes(present ? 1 : 0); + expect(active).toBe(false); + }); + + it('releases the transaction after callback failure and permits the next call', async () => { + const { provider, options } = fixture(); + const failure = new Error('save failed'); + const save = provider.saveTokens; + provider.saveTokens = vi.fn().mockRejectedValueOnce(failure).mockImplementation(save); + const events: string[] = []; + provider.withAuthTransaction = async operation => { + events.push('enter'); + try { + return await operation(); + } finally { + events.push('exit'); + } + }; + await expect(auth(provider, options)).rejects.toBe(failure); + await expect(auth(provider, options)).resolves.toBe('AUTHORIZED'); + expect(events).toEqual(['enter', 'exit', 'enter', 'exit']); + }); + + it.each([{ authorizationCode: 'code' }, { forceReauthorization: true }])( + 'bypass runs its own transaction without waiting for ordinary auth: %j', + async bypass => { + const { provider, options, tokenResponse, posts } = fixture(); + // Supply the callback's persisted issuer binding, without affecting discovery on ordinary calls. + provider.discoveryState = () => ({ + authorizationServerUrl: issuer, + authorizationServerMetadata: metadata, + resourceMetadata: { resource: serverUrl, authorization_servers: [issuer] } + }); + const started = deferred(); + const release = deferred(); + tokenResponse.mockImplementationOnce(async () => { + started.resolve(); + await release.promise; + return Response.json({ access_token: 'fresh', token_type: 'Bearer' }); + }); + const hook = vi.fn(async (operation: () => Promise) => operation()); + provider.withAuthTransaction = hook; + const pending = auth(provider, options); + await started.promise; + try { + await expect(auth(provider, { ...options, ...bypass })).resolves.toBe( + 'authorizationCode' in bypass ? 'AUTHORIZED' : 'REDIRECT' + ); + expect(hook).toHaveBeenCalledTimes(2); + if ('authorizationCode' in bypass) expect(posts[1]!.get('code')).toBe('code'); + else expect(provider.redirectToAuthorization).toHaveBeenCalledTimes(1); + } finally { + release.resolve(); + await pending; + } + } + ); +}); + +describe('ordinary auth option compatibility', () => { + it('shares equivalent URL strings and effective false validation options', async () => { + const { provider, options, posts } = fixture(); + await Promise.all([ + auth(provider, options), + auth(provider, { ...options, serverUrl: new URL(serverUrl), skipIssuerMetadataValidation: false }) + ]); + expect(posts).toHaveLength(1); + }); + + it.each(['resource', 'scope', 'metadata URL', 'fetch identity'])( + 'serializes different %s options and reads freshly stored tokens', + async difference => { + const { provider, options, posts, fetchFn, tokenResponse } = fixture(); + const started = deferred(); + const release = deferred(); + tokenResponse.mockImplementationOnce(async () => { + started.resolve(); + await release.promise; + return Response.json({ access_token: 'fresh', refresh_token: 'rotated', token_type: 'Bearer' }); + }); + const other: AuthOptions = { ...options }; + if (difference === 'resource') other.serverUrl = 'https://api.example.com/other'; + if (difference === 'scope') other.scope = 'extra'; + if (difference === 'metadata URL') + other.resourceMetadataUrl = new URL('https://api.example.com/.well-known/oauth-protected-resource/mcp?version=2'); + const otherFetch = vi.fn((input: string | URL | Request, init?: RequestInit) => fetchFn(input, init)); + if (difference === 'fetch identity') other.fetchFn = otherFetch; + const first = auth(provider, options); + await started.promise; + const second = auth(provider, other); + const sharedSecond = auth(provider, { ...other }); + expect(posts).toHaveLength(1); + release.resolve(); + await expect(Promise.all([first, second, sharedSecond])).resolves.toEqual(['AUTHORIZED', 'AUTHORIZED', 'AUTHORIZED']); + expect(posts).toHaveLength(2); + expect(posts[1]!.get('refresh_token')).toBe('rotated'); + expect(posts[1]!.get('resource')).toBe(difference === 'resource' ? 'https://api.example.com/other' : serverUrl); + if (difference === 'metadata URL') { + expect(fetchFn.mock.calls.some(([input]) => String(input) === other.resourceMetadataUrl!.href)).toBe(true); + } + if (difference === 'fetch identity') { + expect(otherFetch.mock.calls.some(([input]) => String(input) === `${issuer}/token`)).toBe(true); + } + } + ); + + it('does not let an opt-out caller suppress issuer validation for a queued strict caller', async () => { + const { provider, options, fetchFn, posts } = fixture(); + const originalFetch = fetchFn.getMockImplementation()!; + fetchFn.mockImplementation(async (input, init) => + String(input).includes('oauth-authorization-server') + ? Response.json({ ...metadata, issuer: 'https://different.example.com' }) + : originalFetch(input, init) + ); + provider.clientInformation = () => ({ client_id: 'client', issuer: 'https://different.example.com' }); + provider.tokens = () => ({ + access_token: 'expired', + refresh_token: 'refresh', + token_type: 'Bearer', + issuer: 'https://different.example.com' + }); + const results = await Promise.allSettled([ + auth(provider, { ...options, skipIssuerMetadataValidation: true }), + auth(provider, options) + ]); + expect(results[0]).toEqual({ status: 'fulfilled', value: 'AUTHORIZED' }); + expect(results[1]).toMatchObject({ status: 'rejected', reason: expect.any(IssuerMismatchError) }); + expect(posts).toHaveLength(1); + }); + + it('preserves each queued scope when initiating authorization', async () => { + const { provider, options } = fixture(); + provider.tokens = () => undefined; + await expect( + Promise.all([auth(provider, { ...options, scope: 'read' }), auth(provider, { ...options, scope: 'write' })]) + ).resolves.toEqual(['REDIRECT', 'REDIRECT']); + const redirects = vi.mocked(provider.redirectToAuthorization).mock.calls; + expect(redirects.map(([url]) => url.searchParams.get('scope'))).toEqual(['read', 'write']); + }); + + it('keeps a queued operation shareable after the earlier entry cleans up', async () => { + const { provider, options, posts, tokenResponse } = fixture(); + const secondStarted = deferred(); + const releaseSecond = deferred(); + tokenResponse.mockImplementationOnce(async () => + Response.json({ access_token: 'fresh', refresh_token: 'rotated', token_type: 'Bearer' }) + ); + tokenResponse.mockImplementationOnce(async () => { + secondStarted.resolve(); + await releaseSecond.promise; + return Response.json({ access_token: 'fresh', token_type: 'Bearer' }); + }); + const first = auth(provider, options); + const other = { ...options, scope: 'other' }; + const second = auth(provider, other); + await first; + await secondStarted.promise; + const third = auth(provider, other); + releaseSecond.resolve(); + await expect(Promise.all([second, third])).resolves.toEqual(['AUTHORIZED', 'AUTHORIZED']); + expect(posts).toHaveLength(2); + }); + + it('executes queued options after rejection and clears the final rejected entry', async () => { + const { provider, options, posts } = fixture(); + const failure = new Error('storage failed'); + const save = provider.saveTokens; + provider.saveTokens = vi.fn().mockRejectedValueOnce(failure).mockRejectedValueOnce(failure).mockImplementation(save); + const results = await Promise.allSettled([auth(provider, options), auth(provider, { ...options, scope: 'other' })]); + expect(results).toEqual([ + { status: 'rejected', reason: failure }, + { status: 'rejected', reason: failure } + ]); + expect(posts).toHaveLength(2); + await expect(auth(provider, { ...options, scope: 'other' })).resolves.toBe('AUTHORIZED'); + expect(posts).toHaveLength(3); + }); +}); From 3b205e7dd2f997b6a87e479e36421f7eaa2058e0 Mon Sep 17 00:00:00 2001 From: Charles McMillan Date: Sun, 6 Sep 2026 16:37:28 -0400 Subject: [PATCH 3/3] test(client): join an already-running OAuth refresh --- packages/client/test/client/auth.test.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 5858082b3c..7efeb638ac 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -3090,11 +3090,16 @@ describe('OAuth Authorization', () => { it('deduplicates concurrent refreshes for the same provider', async () => { let tokenRequests = 0; let release!: () => void; + let markStarted!: () => void; + const started = new Promise(resolve => { + markStarted = resolve; + }); const gate = new Promise(resolve => { release = resolve; }); configureRefresh(async () => { tokenRequests++; + markStarted(); await gate; return Response.json({ access_token: 'new-access', @@ -3104,8 +3109,10 @@ describe('OAuth Authorization', () => { }); }); + const first = auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }); + await started; const requests = [ - auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), + first, auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }), auth(mockProvider, { serverUrl: 'https://api.example.com/mcp-server' }) ];