From db10ffa250d4ebd072b3250024683a9978c617c0 Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Fri, 8 May 2026 18:07:05 -0400 Subject: [PATCH] Fix Claude Desktop MCP OAuth registration and resume flows (#173) --- src/client/features/auth/AuthPage.tsx | 12 ++- src/lib/auth-redirect.test.ts | 63 ++++++++++++ src/lib/auth-redirect.ts | 54 ++++++++++ src/routes/_auth.sign-in.tsx | 5 +- src/routes/_auth.tsx | 4 +- src/routes/api/auth/$.ts | 77 +++++++++++++-- src/routes/api/auth/-$.test.ts | 98 +++++++++++++++++++ src/server/mcp/handler.ts | 19 ++-- src/server/mcp/protected-resource-metadata.ts | 7 +- 9 files changed, 318 insertions(+), 21 deletions(-) create mode 100644 src/lib/auth-redirect.test.ts diff --git a/src/client/features/auth/AuthPage.tsx b/src/client/features/auth/AuthPage.tsx index 1eb80e9..d59d6dd 100644 --- a/src/client/features/auth/AuthPage.tsx +++ b/src/client/features/auth/AuthPage.tsx @@ -1,5 +1,8 @@ import { z } from "zod"; -import { normalizeAuthRedirect } from "@/lib/auth-redirect"; +import { + getCurrentAuthRedirect, + getOAuthSignedQuery, +} from "@/lib/auth-redirect"; import { isHostedClientAuthMode } from "@/lib/auth-mode"; import { getFieldError as getSharedFieldError, @@ -11,11 +14,16 @@ export const authRedirectSearchSchema = z.object({ }); export function useAuthPageState(redirect: string | undefined) { - const redirectTo = normalizeAuthRedirect(redirect); + const redirectTo = getCurrentAuthRedirect(redirect); + const oauthQuery = + typeof window !== "undefined" + ? getOAuthSignedQuery(window.location.search) + : null; const isHostedMode = isHostedClientAuthMode(); return { redirectTo, + oauthQuery, isHostedMode, }; } diff --git a/src/lib/auth-redirect.test.ts b/src/lib/auth-redirect.test.ts new file mode 100644 index 0000000..9831270 --- /dev/null +++ b/src/lib/auth-redirect.test.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from "vitest"; +import { + getAuthRedirectFromSearch, + getOAuthAuthorizeRedirectFromSearch, + getOAuthSignedQuery, + normalizeAuthRedirect, +} from "./auth-redirect"; + +const oauthSearch = new URLSearchParams({ + response_type: "code", + client_id: "claude-client", + redirect_uri: "https://claude.ai/api/mcp/auth_callback", + scope: "offline_access mcp", + state: "state-123", + code_challenge: "challenge-123", + code_challenge_method: "S256", + resource: "https://app.openseo.so/mcp", + exp: "1778271800", + sig: "signed-value", +}).toString(); + +describe("auth redirect helpers", () => { + it("defaults unsafe or missing redirects to the app root", () => { + expect(normalizeAuthRedirect(undefined)).toBe("/"); + expect(normalizeAuthRedirect("https://evil.example/app")).toBe("/"); + expect(normalizeAuthRedirect("//evil.example/app")).toBe("/"); + }); + + it("preserves safe internal redirects", () => { + expect(getAuthRedirectFromSearch("", "/app")).toBe("/app"); + }); + + it("extracts Better Auth signed OAuth query parameters through sig", () => { + const signedQuery = getOAuthSignedQuery( + `${oauthSearch}&ignored_after_sig=true`, + ); + + expect(signedQuery).toBe(oauthSearch); + }); + + it("builds an internal OAuth authorize continuation redirect", () => { + const redirect = getOAuthAuthorizeRedirectFromSearch(oauthSearch); + + expect(redirect).toBe(`/api/auth/oauth2/authorize?${oauthSearch}`); + expect(redirect).toContain("client_id=claude-client"); + expect(redirect).toContain( + "redirect_uri=https%3A%2F%2Fclaude.ai%2Fapi%2Fmcp%2Fauth_callback", + ); + expect(redirect).toContain("scope=offline_access+mcp"); + expect(redirect).toContain("state=state-123"); + expect(redirect).toContain("code_challenge=challenge-123"); + expect(redirect).toContain("code_challenge_method=S256"); + expect(redirect).toContain("resource=https%3A%2F%2Fapp.openseo.so%2Fmcp"); + expect(redirect).toContain("exp=1778271800"); + expect(redirect).toContain("sig=signed-value"); + }); + + it("prefers OAuth continuation over a generic redirect", () => { + expect(getAuthRedirectFromSearch(oauthSearch, "/app")).toBe( + `/api/auth/oauth2/authorize?${oauthSearch}`, + ); + }); +}); diff --git a/src/lib/auth-redirect.ts b/src/lib/auth-redirect.ts index 4d047fb..b5b5c8b 100644 --- a/src/lib/auth-redirect.ts +++ b/src/lib/auth-redirect.ts @@ -1,3 +1,7 @@ +const OAUTH_AUTHORIZE_PATH = "/api/auth/oauth2/authorize"; +const OAUTH_SIGNED_QUERY_END = "sig"; +const OAUTH_AUTHORIZE_MARKERS = ["response_type", "client_id", "redirect_uri"]; + export function normalizeAuthRedirect(value: string | null | undefined) { if (!value || !value.startsWith("/") || value.startsWith("//")) { return "/"; @@ -6,6 +10,56 @@ export function normalizeAuthRedirect(value: string | null | undefined) { return value; } +export function getOAuthSignedQuery(search: string | null | undefined) { + if (!search) return null; + + const params = new URLSearchParams(search); + if ( + !params.has(OAUTH_SIGNED_QUERY_END) || + !OAUTH_AUTHORIZE_MARKERS.every((marker) => params.has(marker)) + ) { + return null; + } + + // Better Auth signs the authorize params it appends to `loginPage`. + // Preserve only the signed segment through `sig`; any later params belong to + // the app page URL and must not be folded into the OAuth continuation. + const signedParams = new URLSearchParams(); + for (const [key, value] of params.entries()) { + signedParams.append(key, value); + if (key === OAUTH_SIGNED_QUERY_END) break; + } + + return signedParams.toString(); +} + +export function getOAuthAuthorizeRedirectFromSearch( + search: string | null | undefined, +) { + const signedQuery = getOAuthSignedQuery(search); + return signedQuery ? `${OAUTH_AUTHORIZE_PATH}?${signedQuery}` : null; +} + +export function getAuthRedirectFromSearch( + search: string | null | undefined, + redirect: string | null | undefined, +) { + return ( + getOAuthAuthorizeRedirectFromSearch(search) ?? + normalizeAuthRedirect(redirect) + ); +} + +export function getCurrentAuthRedirect( + redirect: string | null | undefined, + location: Pick | null | undefined = typeof window !== + "undefined" + ? window.location + : null, +) { + return getAuthRedirectFromSearch(location?.search, redirect); +} + export function getCurrentAuthRedirectFromHref(href: string) { const url = new URL(href, "https://openseo.local"); return normalizeAuthRedirect(`${url.pathname}${url.search}${url.hash}`); diff --git a/src/routes/_auth.sign-in.tsx b/src/routes/_auth.sign-in.tsx index b46d68f..94e42ba 100644 --- a/src/routes/_auth.sign-in.tsx +++ b/src/routes/_auth.sign-in.tsx @@ -26,7 +26,9 @@ export const Route = createFileRoute("/_auth/sign-in")({ function SignInPage() { const search = Route.useSearch(); - const { redirectTo, isHostedMode } = useAuthPageState(search.redirect); + const { redirectTo, oauthQuery, isHostedMode } = useAuthPageState( + search.redirect, + ); const [verificationEmail, setVerificationEmail] = useState( null, ); @@ -52,6 +54,7 @@ function SignInPage() { email, password: value.password, callbackURL: redirectTo, + ...(oauthQuery ? { oauth_query: oauthQuery } : {}), }); if (!result.error) { diff --git a/src/routes/_auth.tsx b/src/routes/_auth.tsx index fcf12e7..63ee27c 100644 --- a/src/routes/_auth.tsx +++ b/src/routes/_auth.tsx @@ -6,7 +6,7 @@ import { } from "@/client/features/auth/AuthPage"; import { useSession } from "@/lib/auth-client"; import { isHostedClientAuthMode } from "@/lib/auth-mode"; -import { normalizeAuthRedirect } from "@/lib/auth-redirect"; +import { getCurrentAuthRedirect } from "@/lib/auth-redirect"; export const Route = createFileRoute("/_auth")({ validateSearch: authRedirectSearchSchema, @@ -18,7 +18,7 @@ function AuthPageLayout() { const navigate = useNavigate(); const { data: session, isPending } = useSession(); const isHostedMode = isHostedClientAuthMode(); - const redirectTo = normalizeAuthRedirect(search.redirect); + const redirectTo = getCurrentAuthRedirect(search.redirect); useEffect(() => { if (!session?.user?.id) { diff --git a/src/routes/api/auth/$.ts b/src/routes/api/auth/$.ts index 0cdebeb..c49dd94 100644 --- a/src/routes/api/auth/$.ts +++ b/src/routes/api/auth/$.ts @@ -5,6 +5,29 @@ import { isHostedAuthMode } from "@/lib/auth-mode"; import { getMcpResource } from "@/lib/oauth-resource"; const TOKEN_PATH = "/api/auth/oauth2/token"; +const REGISTER_PATH = "/api/auth/oauth2/register"; +const PUBLIC_CLIENT_AUTH_METHOD = "none"; + +function isJsonObject(value: unknown): value is Record { + return value != null && typeof value === "object" && !Array.isArray(value); +} + +function hasRequestAuthContext(request: Request) { + return Boolean( + request.headers.get("authorization") || request.headers.get("cookie"), + ); +} + +function requestWithReplacedBody(request: Request, body: BodyInit) { + const headers = new Headers(request.headers); + headers.delete("content-length"); + + return new Request(request.url, { + method: request.method, + headers, + body, + }); +} // Inject RFC 8707 `resource` into /oauth2/token requests when the client // omitted it. Some MCP clients (notably codex as of 2026-05) skip the @@ -35,11 +58,48 @@ export async function maybeInjectMcpResource( params.set("resource", getMcpResource(getHostedBaseUrl())); - return new Request(request.url, { - method: request.method, - headers: request.headers, - body: params.toString(), - }); + return requestWithReplacedBody(request, params.toString()); +} + +// Some hosted MCP clients attempt unauthenticated DCR while sending a +// confidential-client auth method. Better Auth only permits unauthenticated DCR +// for public clients, so normalize that case to the compatible public shape. +// Claude Desktop hit this path during connector setup: it had no session or +// registration bearer token, but sent a non-`none` token endpoint auth method. +export async function maybeDefaultMcpClientRegistrationAuthMethod( + request: Request, +): Promise { + if (request.method !== "POST") return request; + + const url = new URL(request.url); + if (url.pathname !== REGISTER_PATH) return request; + + const contentType = request.headers.get("content-type") ?? ""; + if (!contentType.includes("application/json")) return request; + + let body: unknown; + try { + body = await request.clone().json(); + } catch { + return request; + } + + if (!isJsonObject(body)) return request; + + if ( + hasRequestAuthContext(request) || + body.token_endpoint_auth_method === PUBLIC_CLIENT_AUTH_METHOD + ) { + return request; + } + + return requestWithReplacedBody( + request, + JSON.stringify({ + ...body, + token_endpoint_auth_method: PUBLIC_CLIENT_AUTH_METHOD, + }), + ); } async function handleAuthRequest(request: Request) { @@ -56,7 +116,12 @@ async function handleAuthRequest(request: Request) { } const auth = getAuth(); - return auth.handler(await maybeInjectMcpResource(request)); + const requestWithRegistrationDefaults = + await maybeDefaultMcpClientRegistrationAuthMethod(request); + const requestWithResource = await maybeInjectMcpResource( + requestWithRegistrationDefaults, + ); + return auth.handler(requestWithResource); } export const Route = createFileRoute("/api/auth/$")({ diff --git a/src/routes/api/auth/-$.test.ts b/src/routes/api/auth/-$.test.ts index af9e4e4..f23c322 100644 --- a/src/routes/api/auth/-$.test.ts +++ b/src/routes/api/auth/-$.test.ts @@ -23,6 +23,7 @@ describe("maybeInjectMcpResource", () => { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", + "Content-Length": "13", }, body: new URLSearchParams({ grant_type: "authorization_code", @@ -36,6 +37,7 @@ describe("maybeInjectMcpResource", () => { expect(params.get("resource")).toBe("https://open-seo.test/mcp"); expect(params.get("grant_type")).toBe("authorization_code"); expect(params.get("code")).toBe("code_123"); + expect(result.headers.has("content-length")).toBe(false); }); it("leaves token requests alone when a resource is already present", async () => { @@ -71,6 +73,7 @@ describe("maybeInjectMcpResource", () => { method: "POST", headers: { "Content-Type": "application/json", + "Content-Length": "13", }, body: JSON.stringify({ grant_type: "authorization_code" }), }), @@ -81,3 +84,98 @@ describe("maybeInjectMcpResource", () => { } }); }); + +describe("maybeDefaultMcpClientRegistrationAuthMethod", () => { + it("defaults JSON dynamic client registration to a public client when omitted", async () => { + const { maybeDefaultMcpClientRegistrationAuthMethod } = + await import("@/routes/api/auth/$"); + const request = new Request( + "https://open-seo.test/api/auth/oauth2/register", + { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ + client_name: "Claude", + redirect_uris: ["https://claude.ai/api/mcp/auth_callback"], + }), + }, + ); + + const result = await maybeDefaultMcpClientRegistrationAuthMethod(request); + const body = await result.json(); + + expect(body).toMatchObject({ + client_name: "Claude", + redirect_uris: ["https://claude.ai/api/mcp/auth_callback"], + token_endpoint_auth_method: "none", + }); + expect(result.headers.has("content-length")).toBe(false); + }); + + it("forces unauthenticated dynamic client registration to a public client", async () => { + const { maybeDefaultMcpClientRegistrationAuthMethod } = + await import("@/routes/api/auth/$"); + const request = new Request( + "https://open-seo.test/api/auth/oauth2/register", + { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify({ + token_endpoint_auth_method: "client_secret_basic", + }), + }, + ); + + const result = await maybeDefaultMcpClientRegistrationAuthMethod(request); + const body = await result.json(); + + expect(body).toMatchObject({ + token_endpoint_auth_method: "none", + }); + }); + + it("leaves explicit registration auth methods alone when auth context exists", async () => { + const { maybeDefaultMcpClientRegistrationAuthMethod } = + await import("@/routes/api/auth/$"); + const request = new Request( + "https://open-seo.test/api/auth/oauth2/register", + { + method: "POST", + headers: { + "Content-Type": "application/json", + Cookie: "better-auth.session_token=session_123", + }, + body: JSON.stringify({ + token_endpoint_auth_method: "client_secret_basic", + }), + }, + ); + + await expect( + maybeDefaultMcpClientRegistrationAuthMethod(request), + ).resolves.toBe(request); + }); + + it("leaves malformed JSON registration requests for the auth handler", async () => { + const { maybeDefaultMcpClientRegistrationAuthMethod } = + await import("@/routes/api/auth/$"); + const request = new Request( + "https://open-seo.test/api/auth/oauth2/register", + { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: "{", + }, + ); + + await expect( + maybeDefaultMcpClientRegistrationAuthMethod(request), + ).resolves.toBe(request); + }); +}); diff --git a/src/server/mcp/handler.ts b/src/server/mcp/handler.ts index 460d6d8..d72d958 100644 --- a/src/server/mcp/handler.ts +++ b/src/server/mcp/handler.ts @@ -68,20 +68,22 @@ function unauthorizedResponse(resource: string) { }); } -async function isPublicMcpRequest(request: Request) { - if (request.method === "OPTIONS") return true; - if (request.method !== "POST") return false; +async function getMcpJsonRpcMethod(request: Request) { + if (request.method !== "POST") return null; try { const body: McpJsonRpcRequest = await request.clone().json(); - return ( - typeof body.method === "string" && PUBLIC_MCP_METHODS.has(body.method) - ); + return typeof body.method === "string" ? body.method : null; } catch { - return false; + return null; } } +function isPublicMcpRequest(request: Request, jsonRpcMethod: string | null) { + if (request.method === "OPTIONS") return true; + return jsonRpcMethod != null && PUBLIC_MCP_METHODS.has(jsonRpcMethod); +} + export async function handleMcpRequest( request: Request, env: { AUTH_MODE?: unknown }, @@ -89,6 +91,7 @@ export async function handleMcpRequest( ) { const authMode = typeof env.AUTH_MODE === "string" ? env.AUTH_MODE : undefined; + const jsonRpcMethod = await getMcpJsonRpcMethod(request); if (!isHostedAuthMode(authMode)) { return new Response("Not found", { status: 404 }); @@ -107,7 +110,7 @@ export async function handleMcpRequest( const organizationIdClaim = getMcpOrganizationIdClaim(baseUrl); const server = createOpenSeoMcpServer(); - if (await isPublicMcpRequest(request)) { + if (isPublicMcpRequest(request, jsonRpcMethod)) { return createMcpHandler(server, { route: MCP_ROUTE, enableJsonResponse: true, diff --git a/src/server/mcp/protected-resource-metadata.ts b/src/server/mcp/protected-resource-metadata.ts index 0e52609..3d0496e 100644 --- a/src/server/mcp/protected-resource-metadata.ts +++ b/src/server/mcp/protected-resource-metadata.ts @@ -23,10 +23,13 @@ export async function mcpProtectedResourceMetadataResponse( } const baseUrl = getHostedBaseUrl(); + const resource = getMcpResource(baseUrl); + const authorizationServer = `${baseUrl}/api/auth`; + const metadata = await getOAuthProviderResourceActions().getProtectedResourceMetadata({ - resource: getMcpResource(baseUrl), - authorization_servers: [`${baseUrl}/api/auth`], + resource, + authorization_servers: [authorizationServer], scopes_supported: [...MCP_OAUTH_SCOPES], resource_name: "OpenSEO MCP", });