From 8a728563d48b393cc6bdc68c65b0636dcad19ac7 Mon Sep 17 00:00:00 2001 From: Ben Senescu <44480372+bensenescu@users.noreply.github.com> Date: Wed, 5 Aug 2026 16:10:28 -0400 Subject: [PATCH] fix(mcp): stop breaking public OAuth clients at token refresh (#420) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): stop breaking public OAuth clients at token refresh The DCR shim force-upgraded every public client (token_endpoint_auth_method "none" or omitted) to client_secret_post so Perplexity would accept the registration response. That made the stored client confidential, so the token endpoint demanded client authentication on every grant — and MCP clients that discard the secret (Codex) lost their session at first token expiry with "invalid_client: missing client_secret". Register those clients as true public clients instead (PKCE + the provider's grant-to-client binding secure that flow), and satisfy Perplexity by decorating only the registration response with a placeholder client_secret and client_secret_post. The provider skips secret validation for public clients, so clients that send the placeholder and clients that never store it both keep working, including on refresh. * refactor(mcp): only rebuild DCR requests that actually change * fix(mcp): satisfy type-aware lint in DCR shims oxlint --type-aware rejected the Record assertions used to read untrusted DCR payloads. Parse both with loose Zod schemas instead, per the repo's trust-boundary convention, which also replaces the hand-rolled object guards. --- src/server/mcp/oauth-provider.ts | 30 +++++--- src/server/mcp/oauth-registration.test.ts | 72 +++++++++++++++++-- src/server/mcp/oauth-registration.ts | 88 ++++++++++++++++++----- 3 files changed, 157 insertions(+), 33 deletions(-) diff --git a/src/server/mcp/oauth-provider.ts b/src/server/mcp/oauth-provider.ts index 5026c58..d280875 100644 --- a/src/server/mcp/oauth-provider.ts +++ b/src/server/mcp/oauth-provider.ts @@ -20,7 +20,10 @@ import { MCP_ROUTE, withWorkersOAuthMcpScopes, } from "@/server/mcp/context"; -import { normalizeClientRegistrationRequest } from "@/server/mcp/oauth-registration"; +import { + normalizeClientRegistrationRequest, + withCompatibilityClientSecret, +} from "@/server/mcp/oauth-registration"; import { getPublicOrigin } from "@/server/mcp/public-origin"; import { handleAuthenticatedOpenSeoMcpRequest } from "@/server/mcp/transport"; import { resolveHostedContext } from "@/middleware/ensure-user/hosted"; @@ -114,10 +117,11 @@ function oauthErrorResponse(error: { }) { // 401s here are the standard OAuth discovery handshake, not failures: an // unauthenticated /mcp hit returns `invalid_token` (which triggers the - // client's .well-known discovery), and the DCR client_secret_post shim makes - // a client's first token attempt return `invalid_client` before it retries - // with the secret. Log those at debug so they stop masquerading as errors; - // keep 5xx at error and everything else (bad client metadata, etc.) at warn. + // client's .well-known discovery), and clients registered as confidential + // before the public-client DCR fix still draw `invalid_client` until they + // retry with the secret or re-register. Log those at debug so they stop + // masquerading as errors; keep 5xx at error and everything else (bad client + // metadata, etc.) at warn. const line = `[oauth] ${error.status} ${error.code}: ${error.description}`; if (error.status === 401) { console.debug(line); @@ -445,11 +449,17 @@ export function createOpenSeoOAuthProvider(appFetch: AppFetch) { const url = new URL(request.url); if (url.pathname === OAUTH_REGISTER_PATH) { - // Cloudflare's provider can reject public DCR clients, but Perplexity - // does not appear to retry as confidential and instead expects a - // client_secret. Normalize before handing the request to Cloudflare so - // it still owns client creation, secret hashing, and token storage. - request = await normalizeClientRegistrationRequest(request); + // Register secretless MCP clients as true public clients so refresh + // grants never require client authentication, then dress the DCR + // response as confidential for clients (Perplexity) that reject + // responses without a client_secret. Cloudflare still owns client + // creation, secret hashing, and token storage. + const response = await provider.fetch( + await normalizeClientRegistrationRequest(request), + env, + ctx, + ); + return withCompatibilityClientSecret(response); } return provider.fetch(request, env, ctx); diff --git a/src/server/mcp/oauth-registration.test.ts b/src/server/mcp/oauth-registration.test.ts index 97b5426..3d536f7 100644 --- a/src/server/mcp/oauth-registration.test.ts +++ b/src/server/mcp/oauth-registration.test.ts @@ -1,16 +1,22 @@ import { describe, expect, it } from "vitest"; -import { normalizeClientRegistrationRequest } from "@/server/mcp/oauth-registration"; +import { z } from "zod"; +import { + normalizeClientRegistrationRequest, + withCompatibilityClientSecret, +} from "@/server/mcp/oauth-registration"; + +const registrationBodySchema = z.looseObject({ client_secret: z.string() }); describe("normalizeClientRegistrationRequest", () => { - it("converts public dynamic registration requests to confidential clients", async () => { + it("keeps explicit public registrations public", async () => { const request = new Request( "https://app.openseo.so/api/auth/oauth2/register", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ - redirect_uris: ["https://www.perplexity.ai/api/mcp/oauth/callback"], - client_name: "Perplexity", + redirect_uris: ["http://localhost:1455/auth/callback"], + client_name: "Codex", token_endpoint_auth_method: "none", }), }, @@ -19,11 +25,11 @@ describe("normalizeClientRegistrationRequest", () => { const normalized = await normalizeClientRegistrationRequest(request); await expect(normalized.json()).resolves.toMatchObject({ - token_endpoint_auth_method: "client_secret_post", + token_endpoint_auth_method: "none", }); }); - it("defaults omitted token auth methods to confidential clients", async () => { + it("defaults omitted token auth methods to public clients", async () => { const request = new Request( "https://app.openseo.so/api/auth/oauth2/register", { @@ -39,7 +45,7 @@ describe("normalizeClientRegistrationRequest", () => { const normalized = await normalizeClientRegistrationRequest(request); await expect(normalized.json()).resolves.toMatchObject({ - token_endpoint_auth_method: "client_secret_post", + token_endpoint_auth_method: "none", }); }); @@ -86,3 +92,55 @@ describe("normalizeClientRegistrationRequest", () => { ); }); }); + +function registrationResponse(body: Record, status = 201) { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("withCompatibilityClientSecret", () => { + it("adds a placeholder secret to public client registrations", async () => { + const response = await withCompatibilityClientSecret( + registrationResponse({ + client_id: "abc123", + token_endpoint_auth_method: "none", + client_id_issued_at: 1753228800, + }), + ); + + const body = registrationBodySchema.parse(await response.json()); + + expect(body).toMatchObject({ + client_id: "abc123", + token_endpoint_auth_method: "client_secret_post", + client_secret_expires_at: 0, + client_secret_issued_at: 1753228800, + }); + expect(body.client_secret).toMatch(/^[0-9a-f]{32}$/); + }); + + it("leaves confidential registrations untouched", async () => { + const original = registrationResponse({ + client_id: "abc123", + token_endpoint_auth_method: "client_secret_post", + client_secret: "real-secret", + }); + + const response = await withCompatibilityClientSecret(original); + + expect(response).toBe(original); + }); + + it("leaves registration errors untouched", async () => { + const original = registrationResponse( + { error: "invalid_client_metadata" }, + 400, + ); + + const response = await withCompatibilityClientSecret(original); + + expect(response).toBe(original); + }); +}); diff --git a/src/server/mcp/oauth-registration.ts b/src/server/mcp/oauth-registration.ts index 427dfe8..1e68b5d 100644 --- a/src/server/mcp/oauth-registration.ts +++ b/src/server/mcp/oauth-registration.ts @@ -1,6 +1,20 @@ -const CONFIDENTIAL_CLIENT_AUTH_METHOD = "client_secret_post"; +import { z } from "zod"; + +const PUBLIC_CLIENT_AUTH_METHOD = "none"; +const COMPAT_CLIENT_AUTH_METHOD = "client_secret_post"; const MAX_CLIENT_REGISTRATION_BODY_BYTES = 1024 * 1024; +// Loose so every field the provider cares about survives the round trip; these +// shims only read the auth method and secret. +const clientMetadataSchema = z.looseObject({ + token_endpoint_auth_method: z.string().optional(), +}); + +const clientRegistrationSchema = z.looseObject({ + token_endpoint_auth_method: z.string().optional(), + client_secret: z.string().optional(), +}); + export async function normalizeClientRegistrationRequest(request: Request) { if (request.method !== "POST") { return request; @@ -16,7 +30,7 @@ export async function normalizeClientRegistrationRequest(request: Request) { return request; } - let clientMetadata: unknown; + let rawMetadata: unknown; try { const text = await request.clone().text(); if (text.length > MAX_CLIENT_REGISTRATION_BODY_BYTES) { @@ -25,28 +39,26 @@ export async function normalizeClientRegistrationRequest(request: Request) { return request; } - clientMetadata = JSON.parse(text); + rawMetadata = JSON.parse(text); } catch { return request; } - if (!clientMetadata || typeof clientMetadata !== "object") { + const parsed = clientMetadataSchema.safeParse(rawMetadata); + if (!parsed.success || parsed.data.token_endpoint_auth_method !== undefined) { return request; } + // The provider defaults an omitted auth method to client_secret_basic, + // which requires client authentication on every grant — including refresh. + // MCP clients that discard the secret (Codex did) then lose their session + // at first token expiry with "invalid_client: missing client_secret". + // Register them as public clients instead; PKCE plus the provider's + // grant-to-client binding secure the public-client flow. const metadata = { - ...clientMetadata, - } as Record; - - if ( - metadata.token_endpoint_auth_method === undefined || - metadata.token_endpoint_auth_method === "none" - ) { - // Perplexity registers as a public client but then rejects DCR responses - // without a client_secret. Use client_secret_post because its validator - // accepts that method but rejects client_secret_basic. - metadata.token_endpoint_auth_method = CONFIDENTIAL_CLIENT_AUTH_METHOD; - } + ...parsed.data, + token_endpoint_auth_method: PUBLIC_CLIENT_AUTH_METHOD, + }; const headers = new Headers(request.headers); headers.set("Content-Type", "application/json"); @@ -58,3 +70,47 @@ export async function normalizeClientRegistrationRequest(request: Request) { body: JSON.stringify(metadata), }); } + +export async function withCompatibilityClientSecret(response: Response) { + if (response.status !== 201) { + return response; + } + + let rawBody: unknown; + try { + rawBody = await response.clone().json(); + } catch { + return response; + } + + const parsed = clientRegistrationSchema.safeParse(rawBody); + if ( + !parsed.success || + parsed.data.token_endpoint_auth_method !== PUBLIC_CLIENT_AUTH_METHOD || + parsed.data.client_secret !== undefined + ) { + return response; + } + + // Perplexity registers as a public client but rejects DCR responses without + // a client_secret (its validator accepts client_secret_post but not + // client_secret_basic). Dress the response as confidential while the stored + // client stays public: the token endpoint skips secret validation for public + // clients, so clients that send this placeholder and clients that never + // store it both keep working — including refresh grants. + const compatBody = { + ...parsed.data, + token_endpoint_auth_method: COMPAT_CLIENT_AUTH_METHOD, + client_secret: crypto.randomUUID().replaceAll("-", ""), + client_secret_expires_at: 0, + client_secret_issued_at: parsed.data.client_id_issued_at, + }; + + const headers = new Headers(response.headers); + headers.delete("Content-Length"); + + return new Response(JSON.stringify(compatBody), { + status: response.status, + headers, + }); +}