From 3d98845335858c39032f548611092adba46c2499 Mon Sep 17 00:00:00 2001 From: Matan Tsach Date: Thu, 5 Mar 2026 16:29:34 +0200 Subject: [PATCH 1/2] fix: continue OAuth metadata discovery on 5xx responses `discoverAuthorizationServerMetadata()` threw immediately on 5xx responses instead of trying the next discovery URL. This breaks MCP servers behind reverse proxies that return 502/503 for well-known paths they don't route. Change both `discoverAuthorizationServerMetadata()` and `shouldAttemptFallback()` to treat any non-OK response as "try next" instead of only 4xx. Fixes #1631 --- packages/client/src/client/auth.ts | 12 +--- packages/client/test/client/auth.test.ts | 77 +++++++++++++++++++++--- 2 files changed, 72 insertions(+), 17 deletions(-) diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 7f7f44019..dc1269f81 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -818,7 +818,7 @@ async function tryMetadataDiscovery(url: URL, protocolVersion: string, fetchFn: * Determines if fallback to root discovery should be attempted */ function shouldAttemptFallback(response: Response | undefined, pathname: string): boolean { - return !response || (response.status >= 400 && response.status < 500 && pathname !== '/'); + return !response || (!response.ok && pathname !== '/'); } /** @@ -845,7 +845,7 @@ async function discoverMetadataWithFallback( let response = await tryMetadataDiscovery(url, protocolVersion, fetchFn); - // If path-aware discovery fails with 404 and we're not already at root, try fallback to root discovery + // If path-aware discovery fails (any non-OK status) and we're not already at root, try fallback to root discovery if (!opts?.metadataUrl && shouldAttemptFallback(response, issuer.pathname)) { const rootUrl = new URL(`/.well-known/${wellKnownType}`, issuer); response = await tryMetadataDiscovery(rootUrl, protocolVersion, fetchFn); @@ -1013,13 +1013,7 @@ export async function discoverAuthorizationServerMetadata( if (!response.ok) { await response.text?.().catch(() => {}); - // Continue looking for any 4xx response code. - if (response.status >= 400 && response.status < 500) { - continue; // Try next URL - } - throw new Error( - `HTTP ${response.status} trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}` - ); + continue; // Try next URL for any non-OK response (4xx, 5xx) } // Parse and validate based on type diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts index 9d8f5cf6b..174e91a55 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -299,17 +299,23 @@ describe('OAuth Authorization', () => { expect(calls.length).toBe(2); }); - it('throws error on 500 status and does not fallback', async () => { - // First call (path-aware) returns 500 + it('falls back to root on 500 status for path URL', async () => { + // First call (path-aware) returns 500 (reverse proxy) mockFetch.mockResolvedValueOnce({ ok: false, status: 500 }); - await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com/path/name')).rejects.toThrow(); + // Root fallback also returns 500 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 500 + }); + + await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com/path/name')).rejects.toThrow('HTTP 500'); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(1); // Should not attempt fallback + expect(calls.length).toBe(2); // Should attempt root fallback }); it('does not fallback when the original URL is already at root path', async () => { @@ -660,12 +666,42 @@ describe('OAuth Authorization', () => { expect(metadata).toBeUndefined(); }); - it('throws on non-404 errors', async () => { + it('throws on non-404 errors for root URL', async () => { mockFetch.mockResolvedValueOnce(new Response(null, { status: 500 })); await expect(discoverOAuthMetadata('https://auth.example.com')).rejects.toThrow('HTTP 500'); }); + it('falls back to root URL on 5xx for path-aware discovery', async () => { + // Path-aware URL returns 502 (reverse proxy has no route for well-known path) + mockFetch.mockResolvedValueOnce(new Response(null, { status: 502 })); + + // Root fallback URL succeeds + mockFetch.mockResolvedValueOnce(Response.json(validMetadata, { status: 200 })); + + const metadata = await discoverOAuthMetadata('https://auth.example.com/tenant1', { + authorizationServerUrl: 'https://auth.example.com/tenant1' + }); + + expect(metadata).toEqual(validMetadata); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('throws when root fallback also returns 5xx for path-aware discovery', async () => { + // Path-aware URL returns 502 + mockFetch.mockResolvedValueOnce(new Response(null, { status: 502 })); + + // Root fallback also returns 503 + mockFetch.mockResolvedValueOnce(new Response(null, { status: 503 })); + + await expect( + discoverOAuthMetadata('https://auth.example.com/tenant1', { + authorizationServerUrl: 'https://auth.example.com/tenant1' + }) + ).rejects.toThrow('HTTP 503'); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + it('validates metadata schema', async () => { mockFetch.mockResolvedValueOnce( Response.json( @@ -818,13 +854,38 @@ describe('OAuth Authorization', () => { expect(metadata).toEqual(validOpenIdMetadata); }); - it('throws on non-4xx errors', async () => { + it('continues on 5xx errors and tries next URL', async () => { + // First URL (OAuth) returns 502 (e.g. reverse proxy with no route) mockFetch.mockResolvedValueOnce({ ok: false, - status: 500 + status: 502, + text: async () => '' + }); + + // Second URL (OIDC) succeeds + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validOpenIdMetadata + }); + + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com'); + + expect(metadata).toEqual(validOpenIdMetadata); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('returns undefined when all URLs fail with 5xx', async () => { + // All URLs return 5xx + mockFetch.mockResolvedValue({ + ok: false, + status: 502, + text: async () => '' }); - await expect(discoverAuthorizationServerMetadata('https://mcp.example.com')).rejects.toThrow('HTTP 500'); + const metadata = await discoverAuthorizationServerMetadata('https://auth.example.com/tenant1'); + + expect(metadata).toBeUndefined(); }); it('handles CORS errors with retry', async () => { From b5b03607f08bd70c1ff32eb07c9bb3213eb3c2a6 Mon Sep 17 00:00:00 2001 From: Matan Tsach Date: Thu, 5 Mar 2026 17:13:47 +0200 Subject: [PATCH 2/2] Add changeset for OAuth 5xx discovery fix Co-Authored-By: Claude Opus 4.6 --- .changeset/fix-oauth-5xx-discovery.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-oauth-5xx-discovery.md diff --git a/.changeset/fix-oauth-5xx-discovery.md b/.changeset/fix-oauth-5xx-discovery.md new file mode 100644 index 000000000..7194a1ad4 --- /dev/null +++ b/.changeset/fix-oauth-5xx-discovery.md @@ -0,0 +1,5 @@ +--- +'@modelcontextprotocol/client': patch +--- + +Continue OAuth metadata discovery on 5xx responses instead of throwing, matching the existing behavior for 4xx. This fixes MCP servers behind reverse proxies that return 502/503 for path-aware metadata URLs.