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 <noreply@anthropic.com> * fix test comment --------- Co-authored-by: Glen Maddern <glen@glenmaddern.com> Co-authored-by: Paul Carleton <paulc@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Paul Carleton <paulcarletonjr@gmail.com>
This commit is contained in:
parent
fb4d07f0ee
commit
6f5e53b44b
2 changed files with 35 additions and 25 deletions
|
|
@ -403,6 +403,19 @@ describe("OAuth Authorization", () => {
|
|||
expect(mockFetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("returns undefined when both CORS requests fail in fetchWithCorsRetry", async () => {
|
||||
// fetchWithCorsRetry tries with headers (fails with CORS), then retries without headers (also fails with CORS)
|
||||
// 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"));
|
||||
});
|
||||
|
||||
// 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,
|
||||
|
|
|
|||
|
|
@ -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<string, string>,
|
||||
): Promise<Response> {
|
||||
headers?: Record<string, string>,
|
||||
): Promise<Response | undefined> {
|
||||
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<Response> {
|
||||
): Promise<Response | undefined> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue