fix(oauth-proxy): run the standard callback checks on proxied callbacks (#11930)

.changeset/oauth-proxy-callback-checks.md

@@ -0,0 +1,5 @@
+---
+"better-auth": patch
+---
+
+OAuth callbacks completed through the OAuth proxy now run the same checks as the standard OAuth callback, including issuer and ID token nonce validation.

packages/better-auth/src/api/routes/callback.ts

@@ -1,15 +1,9 @@
 import { createAuthEndpoint } from "@better-auth/core/api";
-import type { AccountKey } from "@better-auth/core/db";
-import type { OAuth2Tokens } from "@better-auth/core/oauth2";
-import { safeJSONParse } from "@better-auth/core/utils/json";
 import { appendQueryParams } from "@better-auth/core/utils/url";
 import * as z from "zod";
 import { getAwaitableValue } from "../../context/helpers";
 import { setSessionCookie } from "../../cookies";
-import {
-	resolveOAuthAccountKey,
-	toOAuthProfileRecord,
-} from "../../oauth2/account-key";
+import { exchangeOAuthCallback } from "../../oauth2/callback";
 import {
 	missingEmailLogMessage,
 	OAUTH_CALLBACK_ERROR_CODES,
@@ -171,88 +165,18 @@ export const callbackOAuth = createAuthEndpoint(
 			throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.PROVIDER_NOT_FOUND);
 		}
 
-		// RFC 9207: validate authorization server issuer identifier.
-		// Only validated when the provider sends the iss parameter;
-		// older OAuth servers that don't support RFC 9207 omit it.
-		if (iss && provider.issuer && iss !== provider.issuer) {
-			c.context.logger.error("OAuth issuer mismatch", {
-				expected: provider.issuer,
-				received: iss,
-			});
-			throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.ISSUER_MISMATCH);
-		}
-
-		// Fail closed: a provider that requires id_token nonce binding must carry
-		// an expected nonce recovered from state. If it is absent (a flow minted
-		// before binding was enabled, or a dropped state field), the redirect
-		// cannot enforce the binding, so refuse rather than trust an unbound
-		// id_token.
-		if (provider.requiresIdTokenNonce && !idTokenNonce) {
-			c.context.logger.error(
-				"OAuth id_token nonce binding required but no expected nonce was found in state",
-				{ providerId: provider.id },
-			);
-			throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.NONCE_BINDING_MISSING);
-		}
-
-		let tokens: OAuth2Tokens | null;
-		try {
-			tokens = await provider.validateAuthorizationCode({
-				code: code,
-				codeVerifier,
-				deviceId: device_id,
-				redirectURI: `${c.context.baseURL}${getOAuthCallbackPath(provider)}`,
-			});
-		} catch (e) {
-			c.context.logger.error("", e);
-			throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.INVALID_CODE);
-		}
-		if (!tokens) {
-			throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.INVALID_CODE);
-		}
-		const parsedUserData = userData
-			? safeJSONParse<{
-					name?: {
-						firstName?: string;
-						lastName?: string;
-					};
-					email?: string;
-				}>(userData)
-			: null;
-
-		const providerResult = await provider.getUserInfo({
-			...tokens,
-			...(idTokenNonce ? { expectedIdTokenNonce: idTokenNonce } : {}),
-			/**
-			 * The user object from the provider
-			 * This is only available for some providers like Apple
-			 */
-			user: parsedUserData ?? undefined,
+		const exchanged = await exchangeOAuthCallback(c, provider, {
+			code,
+			codeVerifier,
+			iss,
+			deviceId: device_id,
+			user: userData,
+			idTokenNonce,
 		});
-		if (!providerResult?.user) {
-			c.context.logger.error("Unable to get user info");
-			return redirectOnError(
-				OAUTH_CALLBACK_ERROR_CODES.UNABLE_TO_GET_USER_INFO,
-			);
-		}
-		const userInfo = providerResult.user;
-		const providerProfile = toOAuthProfileRecord(providerResult.data);
-		let accountKey: AccountKey;
-		try {
-			accountKey = await resolveOAuthAccountKey(
-				provider,
-				tokens,
-				providerResult.data,
-			);
-		} catch (error) {
-			c.context.logger.error("Unable to derive provider account identity", {
-				providerId: provider.id,
-				error,
-			});
-			return redirectOnError(
-				OAUTH_CALLBACK_ERROR_CODES.UNABLE_TO_GET_USER_INFO,
-			);
+		if (exchanged.error) {
+			throw redirectOnError(exchanged.error);
 		}
+		const { tokens, userInfo, providerProfile, accountKey } = exchanged;
 		const { accountId } = accountKey;
 
 		if (!callbackURL) {

packages/better-auth/src/oauth2/callback.ts

@@ -0,0 +1,107 @@
+import type { GenericEndpointContext } from "@better-auth/core";
+import type { OAuth2Tokens, OAuthProvider } from "@better-auth/core/oauth2";
+import { safeJSONParse } from "@better-auth/core/utils/json";
+import { resolveOAuthAccountKey, toOAuthProfileRecord } from "./account-key";
+import { OAUTH_CALLBACK_ERROR_CODES } from "./errors";
+import { getOAuthCallbackPath } from "./utils";
+
+interface OAuthCallbackInput {
+	code: string;
+	codeVerifier: string;
+	iss?: string | undefined;
+	deviceId?: string | undefined;
+	user?: string | undefined;
+	idTokenNonce?: string | undefined;
+}
+
+/**
+ * Turns a provider callback into a verified identity. It checks that the
+ * callback answers this authorization request, exchanges the code, and
+ * resolves the user and account key.
+ *
+ * Every route that completes a provider callback goes through here, so the
+ * routes run the same checks.
+ */
+export async function exchangeOAuthCallback(
+	ctx: GenericEndpointContext,
+	provider: OAuthProvider,
+	callback: OAuthCallbackInput,
+) {
+	// RFC 9207: validate authorization server issuer identifier.
+	// Only validated when the provider sends the iss parameter;
+	// older OAuth servers that don't support RFC 9207 omit it.
+	if (callback.iss && provider.issuer && callback.iss !== provider.issuer) {
+		ctx.context.logger.error("OAuth issuer mismatch", {
+			expected: provider.issuer,
+			received: callback.iss,
+		});
+		return { error: OAUTH_CALLBACK_ERROR_CODES.ISSUER_MISMATCH };
+	}
+
+	// Fail closed: a provider that requires id_token nonce binding must carry
+	// an expected nonce recovered from state. If it is absent (a flow minted
+	// before binding was enabled, or a dropped state field), the redirect
+	// cannot enforce the binding, so refuse rather than trust an unbound
+	// id_token.
+	if (provider.requiresIdTokenNonce && !callback.idTokenNonce) {
+		ctx.context.logger.error(
+			"OAuth id_token nonce binding required but no expected nonce was found in state",
+			{ providerId: provider.id },
+		);
+		return { error: OAUTH_CALLBACK_ERROR_CODES.NONCE_BINDING_MISSING };
+	}
+
+	// A custom provider can throw before returning a promise, so await inside
+	// `try` to catch both.
+	let tokens: OAuth2Tokens | null;
+	try {
+		tokens = await provider.validateAuthorizationCode({
+			code: callback.code,
+			codeVerifier: callback.codeVerifier,
+			deviceId: callback.deviceId,
+			redirectURI: `${ctx.context.baseURL}${getOAuthCallbackPath(provider)}`,
+		});
+	} catch (error) {
+		ctx.context.logger.error("Failed to validate authorization code", error);
+		return { error: OAUTH_CALLBACK_ERROR_CODES.INVALID_CODE };
+	}
+	if (!tokens) {
+		return { error: OAUTH_CALLBACK_ERROR_CODES.INVALID_CODE };
+	}
+
+	const result = await provider.getUserInfo({
+		...tokens,
+		expectedIdTokenNonce: callback.idTokenNonce,
+		/**
+		 * The user object from the provider
+		 * This is only available for some providers like Apple
+		 */
+		user: safeJSONParse(callback.user) ?? undefined,
+	});
+	if (!result?.user) {
+		ctx.context.logger.error("Unable to get user info");
+		return { error: OAUTH_CALLBACK_ERROR_CODES.UNABLE_TO_GET_USER_INFO };
+	}
+
+	const accountKey = await resolveOAuthAccountKey(
+		provider,
+		tokens,
+		result.data,
+	).catch((error) => {
+		ctx.context.logger.error("Unable to derive provider account identity", {
+			providerId: provider.id,
+			error,
+		});
+		return null;
+	});
+	if (!accountKey) {
+		return { error: OAUTH_CALLBACK_ERROR_CODES.UNABLE_TO_GET_USER_INFO };
+	}
+
+	return {
+		tokens,
+		userInfo: result.user,
+		providerProfile: toOAuthProfileRecord(result.data),
+		accountKey,
+	};
+}

packages/better-auth/src/plugins/oauth-proxy/index.ts

@@ -7,10 +7,7 @@ import {
 	createAuthEndpoint,
 	createAuthMiddleware,
 } from "@better-auth/core/api";
-import type { AccountKey } from "@better-auth/core/db";
 import { accountSchema, userSchema } from "@better-auth/core/db";
-import type { OAuth2Tokens } from "@better-auth/core/oauth2";
-import { safeJSONParse } from "@better-auth/core/utils/json";
 import { defu } from "defu";
 import * as z from "zod";
 import { originCheck } from "../../api";
@@ -21,16 +18,12 @@ import { parseSetCookieHeader } from "../../cookies/cookie-utils";
 import { symmetricDecrypt, symmetricEncrypt } from "../../crypto";
 import type { EncryptionPurpose } from "../../crypto/purpose";
 import { derivePurposeKey } from "../../crypto/purpose";
-import {
-	resolveOAuthAccountKey,
-	toOAuthProfileRecord,
-} from "../../oauth2/account-key";
+import { exchangeOAuthCallback } from "../../oauth2/callback";
 import { redirectOnError } from "../../oauth2/errors";
 import {
 	handleOAuthUserInfo,
 	linkOAuthAccount,
 } from "../../oauth2/link-account";
-import { getOAuthCallbackPath } from "../../oauth2/utils";
 import type { StateData } from "../../state";
 import {
 	getAuthStateVerificationIdentifier,
@@ -165,6 +158,8 @@ const oauthCallbackQuerySchema = z.object({
 	code: z.string().optional(),
 	error: z.string().optional(),
 	user: z.string().optional(),
+	iss: z.string().optional(),
+	device_id: z.string().optional(),
 });
 
 export const oAuthProxy = <O extends OAuthProxyOptions>(opts?: O) => {
@@ -455,7 +450,13 @@ export const oAuthProxy = <O extends OAuthProxyOptions>(opts?: O) => {
 							);
 							return;
 						}
-						const { code, error, user: userData } = query.data;
+						const {
+							code,
+							error,
+							user: userData,
+							iss,
+							device_id: deviceId,
+						} = query.data;
 
 						// Decrypt state to get codeVerifier and callbackURL
 						let stateData: StateData;
@@ -509,74 +510,23 @@ export const oAuthProxy = <O extends OAuthProxyOptions>(opts?: O) => {
 							throw redirectOnError(ctx, errorURL, "oauth_provider_not_found");
 						}
 
-						// Exchange code for tokens
-						let tokens: OAuth2Tokens | null;
-						try {
-							tokens = await provider.validateAuthorizationCode({
-								code,
-								codeVerifier: stateData.codeVerifier,
-								redirectURI: `${ctx.context.baseURL}${getOAuthCallbackPath(provider)}`,
-							});
-						} catch (e) {
-							ctx.context.logger.error(
-								"Failed to validate authorization code",
-								e,
-							);
-							throw redirectOnError(ctx, errorURL, "invalid_code");
-						}
-
-						if (!tokens) {
-							throw redirectOnError(ctx, errorURL, "invalid_code");
-						}
-
-						const parsedUserData = userData
-							? safeJSONParse<{
-									name?: {
-										firstName?: string;
-										lastName?: string;
-									};
-									email?: string;
-								}>(userData)
-							: null;
-
-						// Get user info from provider
-						const userInfoResult = await provider.getUserInfo({
-							...tokens,
-							/**
-							 * The user object from the provider
-							 * This is only available for some providers like Apple
-							 */
-							user: parsedUserData ?? undefined,
+						const exchanged = await exchangeOAuthCallback(ctx, provider, {
+							code,
+							codeVerifier: stateData.codeVerifier,
+							iss,
+							deviceId,
+							user: userData,
+							idTokenNonce: stateData.idTokenNonce,
 						});
-						const userInfo = userInfoResult?.user;
-
-						if (!userInfo) {
-							ctx.context.logger.error("Unable to get user info from provider");
-							throw redirectOnError(ctx, errorURL, "unable_to_get_user_info");
+						if (exchanged.error) {
+							throw redirectOnError(ctx, errorURL, exchanged.error);
 						}
+						const { tokens, userInfo, providerProfile, accountKey } = exchanged;
 
 						if (!userInfo.email) {
 							ctx.context.logger.error("Provider did not return email");
 							throw redirectOnError(ctx, errorURL, "email_not_found");
 						}
-						let accountKey: AccountKey;
-						try {
-							accountKey = await resolveOAuthAccountKey(
-								provider,
-								tokens,
-								userInfoResult.data,
-							);
-						} catch (error) {
-							ctx.context.logger.error(
-								"Unable to derive provider account identity",
-								{
-									providerId: provider.id,
-									error,
-								},
-							);
-							throw redirectOnError(ctx, errorURL, "unable_to_get_user_info");
-						}
-						const providerProfile = toOAuthProfileRecord(userInfoResult.data);
 
 						const proxyCallbackURL = new URL(stateData.callbackURL);
 						const finalCallbackURL =

packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts

@@ -2437,3 +2437,142 @@ describe("oauth-proxy current URL trust", () => {
 		}
 	});
 });
+
+describe("oauth-proxy callback binding", () => {
+	const providerId = "test-idp";
+
+	function createProvider(
+		options: Partial<
+			Pick<
+				OAuthProvider,
+				"issuer" | "requiresIdTokenNonce" | "validateAuthorizationCode"
+			>
+		> = {},
+	) {
+		return {
+			id: providerId,
+			name: "Test IdP",
+			issuer: options.issuer,
+			requiresIdTokenNonce: options.requiresIdTokenNonce,
+			accountSubject: () => "test-subject",
+			createAuthorizationURL: vi.fn<OAuthProvider["createAuthorizationURL"]>(
+				({ state }) =>
+					new URL(`https://idp.example.com/authorize?state=${state}`),
+			),
+			validateAuthorizationCode:
+				options.validateAuthorizationCode ??
+				(async () => ({ accessToken: "test-access-token" })),
+			getUserInfo: vi.fn<OAuthProvider["getUserInfo"]>(async () => ({
+				user: {
+					email: "user@idp.example.com",
+					name: "Test User",
+					emailVerified: true,
+				},
+				data: { sub: "test-subject" },
+			})),
+		} satisfies OAuthProvider;
+	}
+
+	function withProvider(provider: OAuthProvider) {
+		return {
+			id: "test-idp-provider",
+			init: (ctx: { socialProviders: OAuthProvider[] }) => ({
+				context: { socialProviders: [provider, ...ctx.socialProviders] },
+			}),
+		};
+	}
+
+	/**
+	 * Starts sign-in on a preview instance and completes the callback on
+	 * production, returning where production redirects.
+	 */
+	async function completeProxiedCallback({
+		preview = createProvider(),
+		production = createProvider(),
+		query = "",
+	}: {
+		preview?: OAuthProvider;
+		production?: OAuthProvider;
+		query?: string;
+	} = {}) {
+		const productionInstance = await getTestInstance(
+			{
+				baseURL: "http://localhost:3000",
+				plugins: [oAuthProxy(), withProvider(production)],
+			},
+			{ disableTestUser: true },
+		);
+		const previewInstance = await getTestInstance(
+			{
+				baseURL: "http://preview.example.com",
+				plugins: [
+					oAuthProxy({ productionURL: "http://localhost:3000" }),
+					withProvider(preview),
+				],
+			},
+			{ disableTestUser: true },
+		);
+		const { url } = await previewInstance.client.signIn.social(
+			{ provider: providerId, callbackURL: "/dashboard" },
+			{ throw: true },
+		);
+		const state = new URL(url!).searchParams.get("state");
+
+		const redirect: { location: string | null } = { location: null };
+		await productionInstance.client.$fetch(
+			`/callback/${providerId}?code=test&state=${state}${query}`,
+			{
+				onError(context) {
+					redirect.location = context.response.headers.get("location");
+				},
+			},
+		);
+		if (!redirect.location) {
+			throw new Error("OAuth proxy callback did not redirect");
+		}
+		return new URL(redirect.location);
+	}
+
+	it("rejects a callback whose iss does not match the provider issuer", async () => {
+		const location = await completeProxiedCallback({
+			production: createProvider({ issuer: "https://idp.example.com" }),
+			query: "&iss=https://attacker.example",
+		});
+
+		expect(location.searchParams.get("error")).toBe("issuer_mismatch");
+	});
+
+	it("rejects a callback when the provider requires a nonce the state lacks", async () => {
+		const location = await completeProxiedCallback({
+			production: createProvider({ requiresIdTokenNonce: true }),
+		});
+
+		expect(location.searchParams.get("error")).toBe("nonce_binding_missing");
+	});
+
+	it("passes the nonce issued at sign-in to the provider's getUserInfo", async () => {
+		const preview = createProvider({ requiresIdTokenNonce: true });
+		const production = createProvider({ requiresIdTokenNonce: true });
+
+		await completeProxiedCallback({ preview, production });
+
+		const issuedNonce =
+			preview.createAuthorizationURL.mock.calls[0]?.[0].idTokenNonce;
+		expect(issuedNonce).toEqual(expect.any(String));
+		expect(production.getUserInfo).toHaveBeenCalledWith(
+			expect.objectContaining({ expectedIdTokenNonce: issuedNonce }),
+		);
+	});
+
+	it("redirects with invalid_code when the code exchange throws synchronously", async () => {
+		const location = await completeProxiedCallback({
+			production: createProvider({
+				validateAuthorizationCode: () => {
+					throw new Error("token endpoint misconfigured");
+				},
+			}),
+		});
+
+		expect(location.searchParams.get("error")).toBe("invalid_code");
+	});
+});