From 53736a0c8490ac9b463c95df7859d8ba0a4dbabe Mon Sep 17 00:00:00 2001 From: Maximilian Kaske <56969857+mxkaske@users.noreply.github.com> Date: Mon, 7 Sep 2026 10:12:55 +0200 Subject: [PATCH] fix(oauth): fall back to stored or pinned client metadata when the CIMD fetch fails (#2653) * fix(oauth): fall back to stored or pinned client metadata when the CIMD fetch fails claude.ai's Cloudflare bot protection challenges Fly egress IPs, so the authorize flow failed with "HTTP 403" before any consent screen. When the live fetch fails, reuse the last stored document, or a pinned one for known clients, instead of rejecting the client. A successful fetch still always wins, and revoked clients stay blocked. Also log non-2xx responses with the Cloudflare diagnostic headers so a challenge is visible in the Fly logs next time. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016uySLnH1sC5gRHdRrHKKTZ * test(oauth): cover the stored-row fallback at the route level The unreachable-document case reused a client id the previous test had already stored, so the new fallback served it. Use an unknown id for the 400 case and assert the stored client survives an outage. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016uySLnH1sC5gRHdRrHKKTZ * fix(oauth): only fall back to stored metadata when the document is unreachable Address review: a 404 or an invalid body stays a hard failure so a de-registered client cannot live on from stale data; only network errors, timeouts, bot challenges, 429 and 5xx use the stored or pinned copy. Re-read the client row before falling back so a revocation during the fetch still wins. Align the fetched-document log with console.warn and seed the route-level fallback test inside the test. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016uySLnH1sC5gRHdRrHKKTZ --------- Co-authored-by: Claude Fable 5.1 --- apps/server/src/routes/oauth/oauth.test.ts | 33 ++++++- .../services/src/oauth/__tests__/cimd.test.ts | 91 +++++++++++++++++++ packages/services/src/oauth/cimd.ts | 88 +++++++++++++++--- packages/services/src/oauth/index.ts | 1 + 4 files changed, 198 insertions(+), 15 deletions(-) diff --git a/apps/server/src/routes/oauth/oauth.test.ts b/apps/server/src/routes/oauth/oauth.test.ts index 89a45a76..45ccbc84 100644 --- a/apps/server/src/routes/oauth/oauth.test.ts +++ b/apps/server/src/routes/oauth/oauth.test.ts @@ -7,6 +7,7 @@ import { } from "@openstatus/db/src/schema"; import { createTestWorkspace } from "@openstatus/db/src/test/factories"; import { + ClientMetadataUnavailableError, OAuthError, decideSession, parseClientMetadataDocument, @@ -533,7 +534,8 @@ describe("URL client ids (CIMD)", () => { const CIMD_ID = "https://partner.example/.well-known/oauth-client"; const CIMD_REDIRECT = "https://partner.example/oauth/callback"; - function cimdApp(document: Record | null) { + /** `null` answers 404 (gone); `"unavailable"` a bot challenge (403). */ + function cimdApp(document: Record | null | "unavailable") { const { createOAuthRoutes } = routes; const local = new Hono(); local.route( @@ -541,6 +543,11 @@ describe("URL client ids (CIMD)", () => { createOAuthRoutes({ ...config, fetchClientMetadata: async (clientId) => { + if (document === "unavailable") { + throw new ClientMetadataUnavailableError( + "Client metadata document responded with HTTP 403", + ); + } if (!document) { throw new OAuthError( "invalid_client", @@ -627,8 +634,10 @@ describe("URL client ids (CIMD)", () => { ).toBe(200); }); - test("an unreachable or mismatched document is a 400 invalid_client", async () => { - const missing = await cimdAuthorize(cimdApp(null)); + test("an unreachable or mismatched document for an unknown client is a 400 invalid_client", async () => { + // A client id never stored, so no fallback document exists. + const unknown = { client_id: "https://partner.example/.well-known/other" }; + const missing = await cimdAuthorize(cimdApp(null), unknown); expect(missing.status).toBe(400); expect((await missing.json()).error).toBe("invalid_client"); @@ -637,11 +646,29 @@ describe("URL client ids (CIMD)", () => { client_id: "https://other.example/c", redirect_uris: [CIMD_REDIRECT], }), + unknown, ); expect(mismatched.status).toBe(400); expect((await mismatched.json()).error).toBe("invalid_client"); }); + test("an unreachable document for a stored client falls back to the stored row", async () => { + // Seed the row here so the test does not depend on sibling order. + const seeded = await cimdAuthorize( + cimdApp({ client_id: CIMD_ID, redirect_uris: [CIMD_REDIRECT] }), + ); + expect(seeded.status).toBe(302); + + const res = await cimdAuthorize(cimdApp("unavailable")); + expect(res.status).toBe(302); + expect( + new URL(res.headers.get("location") ?? "").searchParams.get("session"), + ).not.toBeNull(); + + const gone = await cimdAuthorize(cimdApp(null)); + expect(gone.status).toBe(400); + }); + test("a URL client id on a private host is refused before any fetch", async () => { let fetched = false; const local = new Hono(); diff --git a/packages/services/src/oauth/__tests__/cimd.test.ts b/packages/services/src/oauth/__tests__/cimd.test.ts index 66e089c9..216ab7f3 100644 --- a/packages/services/src/oauth/__tests__/cimd.test.ts +++ b/packages/services/src/oauth/__tests__/cimd.test.ts @@ -10,7 +10,9 @@ import { import type { Workspace } from "../../types"; import { type ClientMetadataDocument, + ClientMetadataUnavailableError, isUrlClientId, + KNOWN_CLIENT_DOCUMENTS, parseClientMetadataDocument, } from "../cimd"; import { pkceChallenge } from "../crypto"; @@ -307,6 +309,95 @@ describe("authorize with a URL client id", () => { }); }); + test("a fetch failure falls back to the stored document", async () => { + await withTestTransaction(async (tx) => { + await authorizeAs(CLIENT_ID, stub(doc({ client_name: "Stored" })), tx); + const { id } = await authorizeAs( + CLIENT_ID, + stub( + new ClientMetadataUnavailableError( + "Client metadata document responded with HTTP 403", + ), + ), + tx, + ); + expect((await getSession({ input: { id }, db: tx })).clientId).toBe( + CLIENT_ID, + ); + const row = await tx + .select() + .from(oauthClient) + .where(eq(oauthClient.clientId, CLIENT_ID)) + .get(); + expect(row?.name).toBe("Stored"); + expect(row?.redirectUris).toEqual([REDIRECT]); + }); + }); + + test("a fetch failure falls back to a pinned document when nothing is stored", async () => { + const [clientId, pinned] = Object.entries(KNOWN_CLIENT_DOCUMENTS)[0]; + await withTestTransaction(async (tx) => { + const { id } = await authorizeAs( + clientId, + stub( + new ClientMetadataUnavailableError( + "Client metadata document responded with HTTP 403", + ), + ), + tx, + { redirect_uri: pinned.redirect_uris[0] }, + ); + expect((await getSession({ input: { id }, db: tx })).clientId).toBe( + clientId, + ); + const row = await tx + .select() + .from(oauthClient) + .where(eq(oauthClient.clientId, clientId)) + .get(); + expect(row?.name).toBe(pinned.client_name); + expect(row?.redirectUris).toEqual(pinned.redirect_uris); + }); + }); + + test("a 404 for a stored client is still a hard failure", async () => { + await withTestTransaction(async (tx) => { + await authorizeAs(CLIENT_ID, stub(doc()), tx); + const err = await authorizeAs( + CLIENT_ID, + stub( + new OAuthError( + "invalid_client", + "Client metadata document responded with HTTP 404", + ), + ), + tx, + ).catch((e) => e); + expect(err.oauthCode).toBe("invalid_client"); + }); + }); + + test("a client revoked while the fetch is in flight does not fall back", async () => { + await withTestTransaction(async (tx) => { + await authorizeAs(CLIENT_ID, stub(doc()), tx); + const err = await authorizeAs( + CLIENT_ID, + async () => { + await tx + .update(oauthClient) + .set({ revokedAt: new Date() }) + .where(eq(oauthClient.clientId, CLIENT_ID)); + throw new ClientMetadataUnavailableError( + "Client metadata document could not be fetched", + ); + }, + tx, + ).catch((e) => e); + expect(err.oauthCode).toBe("invalid_client"); + expect(err.message).toContain("revoked"); + }); + }); + test("an operator-revoked URL client stays blocked even with a valid document", async () => { await withTestTransaction(async (tx) => { await authorizeAs(CLIENT_ID, stub(doc()), tx); diff --git a/packages/services/src/oauth/cimd.ts b/packages/services/src/oauth/cimd.ts index c402fed4..a0966401 100644 --- a/packages/services/src/oauth/cimd.ts +++ b/packages/services/src/oauth/cimd.ts @@ -110,6 +110,16 @@ export function parseClientMetadataDocument( return parsed.data; } +/** + * The document could not be reached (network failure, timeout, bot challenge, + * rate limit, 5xx). Unlike a 404 or an invalid body, a stored copy may stand in. + */ +export class ClientMetadataUnavailableError extends OAuthError { + constructor(message: string) { + super("invalid_client", message); + } +} + export type ClientMetadataFetcher = ( clientId: string, ) => Promise; @@ -161,10 +171,21 @@ export async function fetchClientMetadataDocument( signal: controller.signal, }); if (!res.ok) { - throw new OAuthError( - "invalid_client", - `Client metadata document responded with HTTP ${res.status}`, + // cf-mitigated/cf-ray tell a bot challenge apart from a real 4xx. + const diag = ["cf-mitigated", "cf-ray", "server", "content-type"] + .map((h) => `${h}=${res.headers.get(h) ?? "-"}`) + .join(" "); + console.warn( + `[oauth/cimd] ${clientId} responded with HTTP ${res.status} (${diag})`, ); + const message = `Client metadata document responded with HTTP ${res.status}`; + const unavailable = + res.status === 403 || + res.status === 429 || + res.status >= 500 || + res.headers.has("cf-mitigated"); + if (unavailable) throw new ClientMetadataUnavailableError(message); + throw new OAuthError("invalid_client", message); } const text = await readBounded(res, CIMD_MAX_BYTES); let body: unknown; @@ -186,8 +207,7 @@ export async function fetchClientMetadataDocument( err instanceof Error ? err.message : String(err) }`, ); - throw new OAuthError( - "invalid_client", + throw new ClientMetadataUnavailableError( "Client metadata document could not be fetched", ); } finally { @@ -195,26 +215,70 @@ export async function fetchClientMetadataDocument( } } +/** + * Documents pinned for clients whose hosts sit behind bot protection that + * challenges datacenter egress IPs. Used only when the live fetch fails and + * nothing is stored yet; a successful fetch always wins. + */ +export const KNOWN_CLIENT_DOCUMENTS: Record = { + "https://claude.ai/oauth/mcp-oauth-client-metadata": { + client_id: "https://claude.ai/oauth/mcp-oauth-client-metadata", + client_name: "Claude", + redirect_uris: ["https://claude.ai/api/mcp/auth_callback"], + }, +}; + /** * Fetch the document and upsert the client row keyed by its URL. Runs once per * authorize request, so no separate cache; a row an operator revoked stays - * revoked no matter what the document says. + * revoked no matter what the document says. When the document is unreachable, + * the last stored document (or a pinned one) stands in, since it passed the + * same domain-ownership check when it was stored. A 404 or an invalid body is + * still a hard failure so a de-registered client does not live on. */ export async function resolveUrlClient( db: DB, clientId: string, fetcher: ClientMetadataFetcher = fetchClientMetadataDocument, ): Promise { - const existing = await db - .select({ revokedAt: oauthClient.revokedAt }) - .from(oauthClient) - .where(eq(oauthClient.clientId, clientId)) - .get(); + const loadStored = () => + db + .select() + .from(oauthClient) + .where(eq(oauthClient.clientId, clientId)) + .get(); + const existing = await loadStored(); if (existing?.revokedAt) { throw new OAuthError("invalid_client", "Unknown or revoked client"); } - const doc = await fetcher(clientId); + let doc: ClientMetadataDocument; + try { + doc = await fetcher(clientId); + console.warn( + `[oauth/cimd] fetched document for ${clientId} (${doc.redirect_uris.length} redirect_uris, ${existing ? "update" : "insert"})`, + ); + } catch (err) { + if (!(err instanceof ClientMetadataUnavailableError)) throw err; + // Re-read: the operator may have revoked the client while the fetch ran. + const stored = await loadStored(); + if (stored?.revokedAt) { + throw new OAuthError("invalid_client", "Unknown or revoked client"); + } + if (stored) { + console.warn( + `[oauth/cimd] using stored document for ${clientId}: ${err.message}`, + ); + return selectOAuthClientSchema.parse(stored); + } + const pinned = KNOWN_CLIENT_DOCUMENTS[clientId]; + if (!pinned) throw err; + console.warn( + `[oauth/cimd] using pinned document for ${clientId}: ${err.message}`, + ); + doc = pinned; + } + const values = { name: doc.client_name ?? new URL(clientId).hostname, redirectUris: Array.from(new Set(doc.redirect_uris)), diff --git a/packages/services/src/oauth/index.ts b/packages/services/src/oauth/index.ts index cd55eacd..6e125768 100644 --- a/packages/services/src/oauth/index.ts +++ b/packages/services/src/oauth/index.ts @@ -17,6 +17,7 @@ export { pkceChallenge } from "./crypto"; export { type ClientMetadataDocument, type ClientMetadataFetcher, + ClientMetadataUnavailableError, isUrlClientId, parseClientMetadataDocument, } from "./cimd"; -- 2.51.2