fix(oidc-provider, mcp): drop "none" alg, default plain PKCE off, reject missing PKCE method (#9575)

Author: gustavovalverde

.changeset/oidc-pkce-and-alg-none-hardening.md

@@ -0,0 +1,13 @@
+---
+"better-auth": patch
+---
+
+fix(oidc-provider, mcp): drop `"none"` from advertised signing algorithms, default `allowPlainCodeChallengeMethod` to `false`, and reject missing PKCE method
+
+The legacy `oidc-provider` and `mcp` plugins now follow OAuth 2.1 (RFC 9700) on three protocol gates:
+
+- `id_token_signing_alg_values_supported` (oidc-provider, mcp) and `resource_signing_alg_values_supported` (mcp) no longer include `"none"`. Relying parties that negotiate from this list will no longer be steered toward unsigned tokens.
+- `allowPlainCodeChallengeMethod` defaults to `false`. Callers who need `plain` PKCE must opt in explicitly.
+- Under the secure default the authorize endpoint no longer silently rewrites a missing `code_challenge_method` to `"plain"` before the allowlist check. A request that provides `code_challenge` without `code_challenge_method` is now rejected with `invalid_request`; the inverse case (`code_challenge_method` without `code_challenge`) is also rejected so no inconsistent PKCE state is persisted on the authorization code record.
+
+Non-breaking for callers who never relied on `"none"` advertisement or the plain default. Callers who explicitly set `allowPlainCodeChallengeMethod: true` keep `plain` on the allowlist **and** retain the legacy "missing method defaults to plain" behavior for backward compatibility, so existing integrations that opted into plain PKCE continue to work. The next-minor on `next` will drop both the `plain` allowlist entry and this fallback; until then, the option is the single explicit knob for legacy behavior. Migrate to `@better-auth/oauth-provider` for the canonical, spec-aligned implementation.

packages/better-auth/src/plugins/mcp/authorize.ts

@@ -149,25 +149,49 @@ export async function authorizeMCPOAuth(
 		);
 	}
 
-	if (!query.code_challenge_method) {
-		query.code_challenge_method = "plain";
-	}
-
-	if (
-		![
-			"s256",
-			options.allowPlainCodeChallengeMethod ? "plain" : "s256",
-		].includes(query.code_challenge_method?.toLowerCase() || "")
-	) {
+	if (query.code_challenge_method && !query.code_challenge) {
 		throw ctx.redirect(
 			redirectErrorURL(
 				query.redirect_uri,
 				"invalid_request",
-				"invalid code_challenge method",
+				"code_challenge_method requires code_challenge",
 			),
 		);
 	}
 
+	if (query.code_challenge) {
+		const allowedCodeChallengeMethods = options.allowPlainCodeChallengeMethod
+			? ["s256", "plain"]
+			: ["s256"];
+		let codeChallengeMethod: AuthorizationQuery["code_challenge_method"] =
+			query.code_challenge_method?.toLowerCase() as AuthorizationQuery["code_challenge_method"];
+		// Backward-compat: callers who explicitly opt into plain PKCE retain the
+		// legacy "default missing method to `plain`" behavior. The secure default
+		// (`allowPlainCodeChallengeMethod: false`) still rejects a missing method.
+		// FIXME(legacy-plain-pkce-removal): remove this fallback on next; require
+		// callers to send `code_challenge_method` explicitly.
+		if (!codeChallengeMethod && options.allowPlainCodeChallengeMethod) {
+			codeChallengeMethod = "plain";
+		}
+		if (
+			!codeChallengeMethod ||
+			!allowedCodeChallengeMethods.includes(codeChallengeMethod)
+		) {
+			throw ctx.redirect(
+				redirectErrorURL(
+					query.redirect_uri,
+					"invalid_request",
+					"invalid code_challenge method",
+				),
+			);
+		}
+		// Persist the normalized value back so the verification record stores the
+		// lowercased and (optionally) fallback-resolved method. The token endpoint
+		// compares against `"plain"` exactly, so casing variations or a missing
+		// method on the opt-in path would otherwise break PKCE verification.
+		query.code_challenge_method = codeChallengeMethod;
+	}
+
 	const code = generateRandomString(32, "a-z", "A-Z", "0-9");
 	const codeExpiresInMs = opts.codeExpiresIn * 1000;
 	const expiresAt = new Date(Date.now() + codeExpiresInMs);

packages/better-auth/src/plugins/mcp/index.ts

@@ -83,7 +83,7 @@ export const getMCPProviderMetadata = (
 			"urn:mace:incommon:iap:bronze",
 		],
 		subject_types_supported: ["public"],
-		id_token_signing_alg_values_supported: ["RS256", "none"],
+		id_token_signing_alg_values_supported: ["RS256"],
 		token_endpoint_auth_methods_supported: [
 			"client_secret_basic",
 			"client_secret_post",
@@ -124,7 +124,7 @@ export const getMCPProtectedResourceMetadata = (
 			"offline_access",
 		],
 		bearer_methods_supported: ["header"],
-		resource_signing_alg_values_supported: ["RS256", "none"],
+		resource_signing_alg_values_supported: ["RS256"],
 	};
 };
 
@@ -175,7 +175,7 @@ export const mcp = (options: MCPOptions) => {
 		defaultScope: "openid",
 		accessTokenExpiresIn: 3600,
 		refreshTokenExpiresIn: 604800,
-		allowPlainCodeChallengeMethod: true,
+		allowPlainCodeChallengeMethod: false,
 		...options.oidcConfig,
 		loginPage: options.loginPage,
 		scopes: [
@@ -664,18 +664,20 @@ export const mcp = (options: MCPOptions) => {
 						});
 					}
 
-					const challenge =
-						value.codeChallengeMethod === "plain"
-							? code_verifier
-							: await createHash("SHA-256", "base64urlnopad").digest(
-									code_verifier,
-								);
+					if (value.codeChallenge) {
+						const challenge =
+							value.codeChallengeMethod === "plain"
+								? code_verifier
+								: await createHash("SHA-256", "base64urlnopad").digest(
+										code_verifier,
+									);
 
-					if (challenge !== value.codeChallenge) {
-						throw new APIError("UNAUTHORIZED", {
-							error_description: "code verification failed",
-							error: "invalid_request",
-						});
+						if (challenge !== value.codeChallenge) {
+							throw new APIError("UNAUTHORIZED", {
+								error_description: "code verification failed",
+								error: "invalid_request",
+							});
+						}
 					}
 
 					const requestedScopes = value.scope;

packages/better-auth/src/plugins/mcp/mcp.test.ts

@@ -441,7 +441,7 @@ describe("mcp", async () => {
 			response_modes_supported: ["query"],
 			grant_types_supported: ["authorization_code", "refresh_token"],
 			subject_types_supported: ["public"],
-			id_token_signing_alg_values_supported: ["RS256", "none"],
+			id_token_signing_alg_values_supported: ["RS256"],
 			token_endpoint_auth_methods_supported: [
 				"client_secret_basic",
 				"client_secret_post",
@@ -475,7 +475,7 @@ describe("mcp", async () => {
 			jwks_uri: `${baseURL}/api/auth/mcp/jwks`,
 			scopes_supported: ["openid", "profile", "email", "offline_access"],
 			bearer_methods_supported: ["header"],
-			resource_signing_alg_values_supported: ["RS256", "none"],
+			resource_signing_alg_values_supported: ["RS256"],
 		});
 	});
 
@@ -1177,3 +1177,39 @@ describe("mcp refresh_token grant client authentication", () => {
 		expect(body?.access_token).toBeUndefined();
 	});
 });
+
+/**
+ * @see https://github.com/better-auth/better-auth/security/advisories/GHSA-9h47-pqcx-hjr4
+ */
+describe("mcp discovery metadata (security)", async () => {
+	const { auth } = await getTestInstance({
+		baseURL: "http://localhost:3000",
+		plugins: [mcp({ loginPage: "/login" })],
+	});
+
+	it("/.well-known/oauth-authorization-server must not advertise alg=none", async () => {
+		const res = await auth.handler(
+			new Request(
+				"http://localhost:3000/api/auth/.well-known/oauth-authorization-server",
+				{ method: "GET" },
+			),
+		);
+		const body = (await res.json()) as {
+			id_token_signing_alg_values_supported: string[];
+		};
+		expect(body.id_token_signing_alg_values_supported).not.toContain("none");
+	});
+
+	it("/.well-known/oauth-protected-resource must not advertise alg=none", async () => {
+		const res = await auth.handler(
+			new Request(
+				"http://localhost:3000/api/auth/.well-known/oauth-protected-resource",
+				{ method: "GET" },
+			),
+		);
+		const body = (await res.json()) as {
+			resource_signing_alg_values_supported: string[];
+		};
+		expect(body.resource_signing_alg_values_supported).not.toContain("none");
+	});
+});

packages/better-auth/src/plugins/oidc-provider/authorize.ts

@@ -197,25 +197,51 @@ export async function authorize(
 		);
 	}
 
-	if (!query.code_challenge_method) {
-		query.code_challenge_method = "plain";
-	}
-
-	if (
-		![
-			"s256",
-			options.allowPlainCodeChallengeMethod ? "plain" : "s256",
-		].includes(query.code_challenge_method?.toLowerCase() || "")
-	) {
+	if (query.code_challenge_method && !query.code_challenge) {
 		return handleRedirect(
 			formatErrorURL(
 				query.redirect_uri,
 				"invalid_request",
-				"invalid code_challenge method",
+				"code_challenge_method requires code_challenge",
 			),
 		);
 	}
 
+	if (query.code_challenge) {
+		const allowedCodeChallengeMethods = options.allowPlainCodeChallengeMethod
+			? ["s256", "plain"]
+			: ["s256"];
+		let codeChallengeMethod: AuthorizationQuery["code_challenge_method"] =
+			query.code_challenge_method?.toLowerCase() as AuthorizationQuery["code_challenge_method"];
+		// Backward-compat: callers who explicitly opt into plain PKCE retain the
+		// legacy "default missing method to `plain`" behavior. The secure default
+		// (`allowPlainCodeChallengeMethod: false`) still rejects a missing method
+		// as `invalid_request`. The whole branch should be removed once the next
+		// minor drops the `plain` PKCE allowlist entry (see FIXME below).
+		// FIXME(legacy-plain-pkce-removal): remove this fallback on next; require
+		// callers to send `code_challenge_method` explicitly.
+		if (!codeChallengeMethod && options.allowPlainCodeChallengeMethod) {
+			codeChallengeMethod = "plain";
+		}
+		if (
+			!codeChallengeMethod ||
+			!allowedCodeChallengeMethods.includes(codeChallengeMethod)
+		) {
+			return handleRedirect(
+				formatErrorURL(
+					query.redirect_uri,
+					"invalid_request",
+					"invalid code_challenge method",
+				),
+			);
+		}
+		// Persist the normalized value back so the verification record stores the
+		// lowercased and (optionally) fallback-resolved method. The token endpoint
+		// compares against `"plain"` exactly, so casing variations or a missing
+		// method on the opt-in path would otherwise break PKCE verification.
+		query.code_challenge_method = codeChallengeMethod;
+	}
+
 	const code = generateRandomString(32, "a-z", "A-Z", "0-9");
 	const codeExpiresInMs = opts.codeExpiresIn! * 1000;
 	const expiresAt = new Date(Date.now() + codeExpiresInMs);

packages/better-auth/src/plugins/oidc-provider/index.ts

@@ -95,9 +95,7 @@ export const getMetadata = (
 			? jwtPlugin.options.jwt.issuer
 			: (ctx.context.options.baseURL as string);
 	const baseURL = ctx.context.baseURL;
-	const supportedAlgs = options?.useJWTPlugin
-		? ["RS256", "EdDSA", "none"]
-		: ["HS256", "none"];
+	const supportedAlgs = options?.useJWTPlugin ? ["RS256", "EdDSA"] : ["HS256"];
 	return {
 		issuer,
 		authorization_endpoint: `${baseURL}/oauth2/authorize`,
@@ -309,7 +307,7 @@ export const oidcProvider = (options: OIDCOptions) => {
 		defaultScope: "openid",
 		accessTokenExpiresIn: DEFAULT_ACCESS_TOKEN_EXPIRES_IN,
 		refreshTokenExpiresIn: DEFAULT_REFRESH_TOKEN_EXPIRES_IN,
-		allowPlainCodeChallengeMethod: true,
+		allowPlainCodeChallengeMethod: false,
 		storeClientSecret: "plain" as const,
 		...options,
 		scopes: [
@@ -983,18 +981,20 @@ export const oidcProvider = (options: OIDCOptions) => {
 							});
 						}
 					}
-					const challenge =
-						value.codeChallengeMethod === "plain"
-							? code_verifier
-							: await createHash("SHA-256", "base64urlnopad").digest(
-									code_verifier,
-								);
-
-					if (challenge !== value.codeChallenge) {
-						throw new APIError("UNAUTHORIZED", {
-							error_description: "code verification failed",
-							error: "invalid_request",
-						});
+					if (value.codeChallenge) {
+						const challenge =
+							value.codeChallengeMethod === "plain"
+								? code_verifier
+								: await createHash("SHA-256", "base64urlnopad").digest(
+										code_verifier,
+									);
+
+						if (challenge !== value.codeChallenge) {
+							throw new APIError("UNAUTHORIZED", {
+								error_description: "code verification failed",
+								error: "invalid_request",
+							});
+						}
 					}
 
 					const requestedScopes = value.scope;