fix(mcp): stop breaking public OAuth clients at token refresh (#420)
* 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<string, unknown> 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.
This commit is contained in:
parent
30948a2a1d
commit
8a728563d4
@ -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);
|
||||
|
||||
@ -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<string, unknown>, 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);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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<string, unknown>;
|
||||
|
||||
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,
|
||||
});
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user