Skip to content

Commit 6f5e53b

Browse files
geelenpcarletonclaude
authored
Returning undefined from discoverOAuthMetadata for CORS errors (#717)
* 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`. * Add test for CORS error handling that should return undefined 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 <[email protected]> * fix test comment --------- Co-authored-by: Glen Maddern <[email protected]> Co-authored-by: Paul Carleton <[email protected]> Co-authored-by: Claude <[email protected]> Co-authored-by: Paul Carleton <[email protected]>
1 parent fb4d07f commit 6f5e53b

File tree

2 files changed

+35
-25
lines changed

2 files changed

+35
-25
lines changed

src/client/auth.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,19 @@ describe("OAuth Authorization", () => {
403403
expect(mockFetch).toHaveBeenCalledTimes(2);
404404
});
405405

406+
it("returns undefined when both CORS requests fail in fetchWithCorsRetry", async () => {
407+
// fetchWithCorsRetry tries with headers (fails with CORS), then retries without headers (also fails with CORS)
408+
// simulating a 404 w/o headers set. We want this to return undefined, not throw TypeError
409+
mockFetch.mockImplementation(() => {
410+
// Both the initial request with headers and retry without headers fail with CORS TypeError
411+
return Promise.reject(new TypeError("Failed to fetch"));
412+
});
413+
414+
// This should return undefined (the desired behavior after the fix)
415+
const metadata = await discoverOAuthMetadata("https://auth.example.com/path");
416+
expect(metadata).toBeUndefined();
417+
});
418+
406419
it("returns undefined when discovery endpoint returns 404", async () => {
407420
mockFetch.mockResolvedValueOnce({
408421
ok: false,

src/client/auth.ts

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -292,25 +292,24 @@ export async function discoverOAuthProtectedResourceMetadata(
292292
return OAuthProtectedResourceMetadataSchema.parse(await response.json());
293293
}
294294

295-
/**
296-
* Looks up RFC 8414 OAuth 2.0 Authorization Server Metadata.
297-
*
298-
* If the server returns a 404 for the well-known endpoint, this function will
299-
* return `undefined`. Any other errors will be thrown as exceptions.
300-
*/
301295
/**
302296
* Helper function to handle fetch with CORS retry logic
303297
*/
304298
async function fetchWithCorsRetry(
305299
url: URL,
306-
headers: Record<string, string>,
307-
): Promise<Response> {
300+
headers?: Record<string, string>,
301+
): Promise<Response | undefined> {
308302
try {
309303
return await fetch(url, { headers });
310304
} catch (error) {
311-
// CORS errors come back as TypeError, retry without headers
312305
if (error instanceof TypeError) {
313-
return await fetch(url);
306+
if (headers) {
307+
// CORS errors come back as TypeError, retry without headers
308+
return fetchWithCorsRetry(url)
309+
} else {
310+
// We're getting CORS errors on retry too, return undefined
311+
return undefined
312+
}
314313
}
315314
throw error;
316315
}
@@ -334,7 +333,7 @@ function buildWellKnownPath(pathname: string): string {
334333
async function tryMetadataDiscovery(
335334
url: URL,
336335
protocolVersion: string,
337-
): Promise<Response> {
336+
): Promise<Response | undefined> {
338337
const headers = {
339338
"MCP-Protocol-Version": protocolVersion
340339
};
@@ -344,10 +343,16 @@ async function tryMetadataDiscovery(
344343
/**
345344
* Determines if fallback to root discovery should be attempted
346345
*/
347-
function shouldAttemptFallback(response: Response, pathname: string): boolean {
348-
return response.status === 404 && pathname !== '/';
346+
function shouldAttemptFallback(response: Response | undefined, pathname: string): boolean {
347+
return !response || response.status === 404 && pathname !== '/';
349348
}
350349

350+
/**
351+
* Looks up RFC 8414 OAuth 2.0 Authorization Server Metadata.
352+
*
353+
* If the server returns a 404 for the well-known endpoint, this function will
354+
* return `undefined`. Any other errors will be thrown as exceptions.
355+
*/
351356
export async function discoverOAuthMetadata(
352357
authorizationServerUrl: string | URL,
353358
opts?: { protocolVersion?: string },
@@ -362,18 +367,10 @@ export async function discoverOAuthMetadata(
362367

363368
// If path-aware discovery fails with 404, try fallback to root discovery
364369
if (shouldAttemptFallback(response, issuer.pathname)) {
365-
try {
366-
const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
367-
response = await tryMetadataDiscovery(rootUrl, protocolVersion);
368-
369-
if (response.status === 404) {
370-
return undefined;
371-
}
372-
} catch {
373-
// If fallback fails, return undefined
374-
return undefined;
375-
}
376-
} else if (response.status === 404) {
370+
const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
371+
response = await tryMetadataDiscovery(rootUrl, protocolVersion);
372+
}
373+
if (!response || response.status === 404) {
377374
return undefined;
378375
}
379376

0 commit comments

Comments
 (0)