From 2301ec9d1d33d69bf2f1cb040c9d42b0b7a8ba0b Mon Sep 17 00:00:00 2001 From: Glen Maddern Date: Tue, 1 Jul 2025 21:06:28 +1000 Subject: [PATCH 1/3] Returning undefined from `discoverOAuthMetadata` for CORS errors This behaviour was already happening for the root URL, but the new `fetchWithCorsRetry` logic differed. The issue was if the server returns a 404 for `/.well-known/oauth-authorization-server/xyz` that didn't have the `access-control-allow-origin`, a TypeError was being thrown. There was logic there already to handle a TypeError for a _preflight_ request (cause by custom headers), but not the fallback. I refactored so all combinations return `undefined`. --- src/client/auth.ts | 47 ++++++++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index 376905743..71101a428 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -292,25 +292,24 @@ export async function discoverOAuthProtectedResourceMetadata( return OAuthProtectedResourceMetadataSchema.parse(await response.json()); } -/** - * Looks up RFC 8414 OAuth 2.0 Authorization Server Metadata. - * - * If the server returns a 404 for the well-known endpoint, this function will - * return `undefined`. Any other errors will be thrown as exceptions. - */ /** * Helper function to handle fetch with CORS retry logic */ async function fetchWithCorsRetry( url: URL, - headers: Record, -): Promise { + headers?: Record, +): Promise { try { return await fetch(url, { headers }); } catch (error) { - // CORS errors come back as TypeError, retry without headers if (error instanceof TypeError) { - return await fetch(url); + if (headers) { + // CORS errors come back as TypeError, retry without headers + return fetchWithCorsRetry(url) + } else { + // We're getting CORS errors on retry too, return undefined + return undefined + } } throw error; } @@ -334,7 +333,7 @@ function buildWellKnownPath(pathname: string): string { async function tryMetadataDiscovery( url: URL, protocolVersion: string, -): Promise { +): Promise { const headers = { "MCP-Protocol-Version": protocolVersion }; @@ -344,10 +343,16 @@ async function tryMetadataDiscovery( /** * Determines if fallback to root discovery should be attempted */ -function shouldAttemptFallback(response: Response, pathname: string): boolean { - return response.status === 404 && pathname !== '/'; +function shouldAttemptFallback(response: Response | undefined, pathname: string): boolean { + return !response || response.status === 404 && pathname !== '/'; } +/** + * Looks up RFC 8414 OAuth 2.0 Authorization Server Metadata. + * + * If the server returns a 404 for the well-known endpoint, this function will + * return `undefined`. Any other errors will be thrown as exceptions. + */ export async function discoverOAuthMetadata( authorizationServerUrl: string | URL, opts?: { protocolVersion?: string }, @@ -362,18 +367,10 @@ export async function discoverOAuthMetadata( // If path-aware discovery fails with 404, try fallback to root discovery if (shouldAttemptFallback(response, issuer.pathname)) { - try { - const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer); - response = await tryMetadataDiscovery(rootUrl, protocolVersion); - - if (response.status === 404) { - return undefined; - } - } catch { - // If fallback fails, return undefined - return undefined; - } - } else if (response.status === 404) { + const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer); + response = await tryMetadataDiscovery(rootUrl, protocolVersion); + } + if (!response || response.status === 404) { return undefined; } From 15bfa04055157db3590cdbfdfe9cb872bc4c34b0 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Tue, 1 Jul 2025 16:14:16 +0100 Subject: [PATCH 2/3] Add test for CORS error handling that should return undefined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test covers the scenario where both the initial request with headers and the retry without headers fail with CORS TypeErrors. The desired behavior is to return undefined instead of throwing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 8e77c0a5b..c6ee3e306 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -403,6 +403,20 @@ describe("OAuth Authorization", () => { expect(mockFetch).toHaveBeenCalledTimes(2); }); + it("returns undefined when both CORS requests fail in fetchWithCorsRetry", async () => { + // This simulates the exact scenario from PR description: + // fetchWithCorsRetry tries with headers (fails with CORS), then retries without headers (also fails with CORS) + // We want this to return undefined, not throw TypeError + mockFetch.mockImplementation(() => { + // Both the initial request with headers and retry without headers fail with CORS TypeError + return Promise.reject(new TypeError("Failed to fetch")); + }); + + // This should return undefined (the desired behavior after the fix) + const metadata = await discoverOAuthMetadata("https://auth.example.com/path"); + expect(metadata).toBeUndefined(); + }); + it("returns undefined when discovery endpoint returns 404", async () => { mockFetch.mockResolvedValueOnce({ ok: false, From 8916e8325538d7530b7fcc263e19af06207dff26 Mon Sep 17 00:00:00 2001 From: Paul Carleton Date: Tue, 1 Jul 2025 16:22:48 +0100 Subject: [PATCH 3/3] fix test comment --- src/client/auth.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index c6ee3e306..8155e1342 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -404,9 +404,8 @@ describe("OAuth Authorization", () => { }); it("returns undefined when both CORS requests fail in fetchWithCorsRetry", async () => { - // This simulates the exact scenario from PR description: // fetchWithCorsRetry tries with headers (fails with CORS), then retries without headers (also fails with CORS) - // We want this to return undefined, not throw TypeError + // simulating a 404 w/o headers set. We want this to return undefined, not throw TypeError mockFetch.mockImplementation(() => { // Both the initial request with headers and retry without headers fail with CORS TypeError return Promise.reject(new TypeError("Failed to fetch"));