From 3d98845335858c39032f548611092adba46c2499 Mon Sep 17 00:00:00 2001 From: Matan Tsach Date: Thu, 5 Mar 2026 16:29:34 +0200 Subject: [PATCH 1/3] 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/3] 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. From c9a60125064977cdce8d2332a26021cce417d7f7 Mon Sep 17 00:00:00 2001 From: Matan Tsach Date: Mon, 30 Mar 2026 19:59:12 +0300 Subject: [PATCH 3/3] fix: scope discovery fallback to 502 only, not all 5xx MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per maintainer feedback — blanket 5xx fallback could hammer overloaded servers. 502 Bad Gateway is the most common reverse proxy routing error and covers the reported use case (Azure APIM). Co-Authored-By: Claude Opus 4.6 (1M context) --- .changeset/fix-oauth-5xx-discovery.md | 2 +- packages/client/src/client/auth.ts | 13 +++-- packages/client/test/client/auth.test.ts | 62 +++++++++++++++++++----- 3 files changed, 60 insertions(+), 17 deletions(-) diff --git a/.changeset/fix-oauth-5xx-discovery.md b/.changeset/fix-oauth-5xx-discovery.md index 7194a1ad4..0b2e05ae7 100644 --- a/.changeset/fix-oauth-5xx-discovery.md +++ b/.changeset/fix-oauth-5xx-discovery.md @@ -2,4 +2,4 @@ '@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. +Continue OAuth metadata discovery on 502 (Bad Gateway) responses, matching the existing behavior for 4xx. This fixes MCP servers behind reverse proxies that return 502 for path-aware metadata URLs. Other 5xx errors still throw to avoid retrying against overloaded servers. diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts index 17dbd3328..c7b2056cf 100644 --- a/packages/client/src/client/auth.ts +++ b/packages/client/src/client/auth.ts @@ -1002,7 +1002,9 @@ 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.ok && pathname !== '/'); + if (!response) return true; // CORS error — always try fallback + if (pathname === '/') return false; // Already at root + return (response.status >= 400 && response.status < 500) || response.status === 502; } /** @@ -1029,7 +1031,7 @@ async function discoverMetadataWithFallback( let response = await tryMetadataDiscovery(url, protocolVersion, fetchFn); - // If path-aware discovery fails (any non-OK status) and we're not already at root, try fallback to root discovery + // If path-aware discovery fails (4xx or 502 Bad Gateway) 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); @@ -1197,7 +1199,12 @@ export async function discoverAuthorizationServerMetadata( if (!response.ok) { await response.text?.().catch(() => {}); - continue; // Try next URL for any non-OK response (4xx, 5xx) + if ((response.status >= 400 && response.status < 500) || response.status === 502) { + continue; // Try next URL for 4xx or 502 (Bad Gateway) + } + throw new Error( + `HTTP ${response.status} trying to load ${type === 'oauth' ? 'OAuth' : 'OpenID provider'} metadata from ${endpointUrl}` + ); } // 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 267390ab0..e274cd817 100644 --- a/packages/client/test/client/auth.test.ts +++ b/packages/client/test/client/auth.test.ts @@ -337,23 +337,36 @@ describe('OAuth Authorization', () => { expect(calls.length).toBe(2); }); - it('falls back to root on 500 status for path URL', async () => { - // First call (path-aware) returns 500 (reverse proxy) + it('throws on 500 status without fallback', async () => { + // First call (path-aware) returns 500 (overloaded server) mockFetch.mockResolvedValueOnce({ ok: false, status: 500 }); - // Root fallback also returns 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 + }); + + it('falls back to root on 502 status for path URL', async () => { + // First call (path-aware) returns 502 (reverse proxy routing error) mockFetch.mockResolvedValueOnce({ ok: false, - status: 500 + status: 502 }); - await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com/path/name')).rejects.toThrow('HTTP 500'); + // Root fallback also returns 502 + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 502 + }); + + await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com/path/name')).rejects.toThrow('HTTP 502'); const calls = mockFetch.mock.calls; - expect(calls.length).toBe(2); // Should attempt root fallback + expect(calls.length).toBe(2); // Should attempt root fallback for 502 }); it('does not fallback when the original URL is already at root path', async () => { @@ -715,7 +728,7 @@ describe('OAuth Authorization', () => { await expect(discoverOAuthMetadata('https://auth.example.com')).rejects.toThrow('HTTP 500'); }); - it('falls back to root URL on 5xx for path-aware discovery', async () => { + it('falls back to root URL on 502 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 })); @@ -730,8 +743,20 @@ describe('OAuth Authorization', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); - it('throws when root fallback also returns 5xx for path-aware discovery', async () => { - // Path-aware URL returns 502 + it('does not fall back on non-502 5xx for path-aware discovery', async () => { + // Path-aware URL returns 500 (overloaded server — should not retry) + mockFetch.mockResolvedValueOnce(new Response(null, { status: 500 })); + + await expect( + discoverOAuthMetadata('https://auth.example.com/tenant1', { + authorizationServerUrl: 'https://auth.example.com/tenant1' + }) + ).rejects.toThrow('HTTP 500'); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('throws when root fallback also returns error for path-aware discovery', async () => { + // Path-aware URL returns 502 (gateway error — triggers fallback) mockFetch.mockResolvedValueOnce(new Response(null, { status: 502 })); // Root fallback also returns 503 @@ -897,8 +922,8 @@ describe('OAuth Authorization', () => { expect(metadata).toEqual(validOpenIdMetadata); }); - it('continues on 5xx errors and tries next URL', async () => { - // First URL (OAuth) returns 502 (e.g. reverse proxy with no route) + it('continues on 502 and tries next URL', async () => { + // First URL (OAuth) returns 502 (reverse proxy with no route) mockFetch.mockResolvedValueOnce({ ok: false, status: 502, @@ -918,8 +943,19 @@ describe('OAuth Authorization', () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); - it('returns undefined when all URLs fail with 5xx', async () => { - // All URLs return 5xx + it('throws on non-502 5xx errors', async () => { + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 500, + text: async () => '' + }); + + await expect(discoverAuthorizationServerMetadata('https://auth.example.com')).rejects.toThrow('HTTP 500'); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('returns undefined when all URLs fail with 502', async () => { + // All URLs return 502 mockFetch.mockResolvedValue({ ok: false, status: 502,