fix(core): refuse redirects on server-side OAuth requests (#10241)

.changeset/oauth-fetch-refuse-redirects.md

@@ -0,0 +1,7 @@
+---
+"@better-auth/core": patch
+---
+
+Refuse HTTP redirects on server-side OAuth requests
+
+Better Auth refuses HTTP redirects on the server-side OAuth requests it makes: the token exchange, token refresh, client-credentials, token introspection, and JWKS requests. A provider endpoint cannot redirect one of these requests to an unintended internal address. Conformant OAuth providers answer these endpoints with a direct response and never redirect, so standard integrations are unaffected.

docs/content/docs/reference/security.mdx

@@ -298,6 +298,10 @@ You can also dynamically set the list of trusted origins by providing a function
   **Important**: This function will be invoked per incoming request, so be careful if you decide to dynamically fetch your list of trusted domains.
 </Callout>
 
+## Outbound OAuth Requests
+
+During sign-in, Better Auth makes server-side requests to OAuth and OIDC provider endpoints: the token exchange, token refresh, token introspection, and key-set (JWKS) requests. These requests do not follow HTTP redirects. A conformant provider answers these endpoints with a direct response and never redirects, so standard integrations are unaffected. Refusing redirects keeps a provider endpoint from steering a server-side request to an unintended internal address.
+
 ## Email Enumeration Protection
 
 Better Auth prevents email enumeration on the sign-up and change-email endpoints. When `requireEmailVerification` is enabled or `autoSignIn` is set to `false`, the sign-up endpoint returns the same `200` response whether the email is already registered or not, following [OWASP authentication best practices](https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html#authentication-and-error-messages). Timing attacks are mitigated by simulating password hashing on duplicate sign-up attempts.

packages/core/src/oauth2/client-credentials-token.ts

@@ -1,7 +1,7 @@
 import { base64Url } from "@better-auth/utils/base64";
-import { betterFetch } from "@better-fetch/fetch";
 import type { AwaitableFunction } from "../types";
 import type { OAuth2Tokens, ProviderOptions } from "./oauth-provider";
+import { fetchRefusingRedirects } from "./reject-redirects";
 
 export async function clientCredentialsTokenRequest({
 	options,
@@ -96,7 +96,7 @@ export async function clientCredentialsToken({
 		resource,
 	});
 
-	const { data, error } = await betterFetch<{
+	const { data, error } = await fetchRefusingRedirects<{
 		access_token: string;
 		expires_in?: number | undefined;
 		token_type?: string | undefined;

packages/core/src/oauth2/refresh-access-token.ts

@@ -1,7 +1,7 @@
 import { base64 } from "@better-auth/utils/base64";
-import { betterFetch } from "@better-fetch/fetch";
 import type { AwaitableFunction } from "../types";
 import type { OAuth2Tokens, ProviderOptions } from "./oauth-provider";
+import { fetchRefusingRedirects } from "./reject-redirects";
 
 export async function refreshAccessTokenRequest({
 	refreshToken,
@@ -115,7 +115,7 @@ export async function refreshAccessToken({
 		extraParams,
 	});
 
-	const { data, error } = await betterFetch<{
+	const { data, error } = await fetchRefusingRedirects<{
 		access_token: string;
 		refresh_token?: string | undefined;
 		expires_in?: number | undefined;

packages/core/src/oauth2/reject-redirects.runtime.test.ts

@@ -0,0 +1,43 @@
+import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
+import { refreshAccessToken } from "./refresh-access-token";
+
+/**
+ * Exercises the real `betterFetch` (only `globalThis.fetch` is mocked) to prove
+ * the control end to end: a redirecting token endpoint is rejected and the
+ * redirect is never followed to the internal host.
+ */
+describe("server-side OAuth fetch refuses redirects via real betterFetch", () => {
+	const originalFetch = globalThis.fetch;
+	const mockedFetch = vi.fn() as unknown as typeof fetch &
+		ReturnType<typeof vi.fn>;
+
+	beforeEach(() => {
+		mockedFetch.mockReset();
+		globalThis.fetch = mockedFetch;
+	});
+
+	afterAll(() => {
+		globalThis.fetch = originalFetch;
+	});
+
+	it("rejects a 302 from the token endpoint and never follows it", async () => {
+		mockedFetch.mockResolvedValueOnce(
+			new Response("", {
+				status: 302,
+				headers: { location: "http://169.254.169.254/" },
+			}),
+		);
+
+		await expect(
+			refreshAccessToken({
+				refreshToken: "refresh-token",
+				options: { clientId: "client", clientSecret: "secret" },
+				tokenEndpoint: "https://idp.example/token",
+			}),
+		).rejects.toThrow(/refuse redirects to prevent SSRF/);
+
+		expect(mockedFetch).toHaveBeenCalledTimes(1);
+		const init = mockedFetch.mock.calls[0]?.[1] as RequestInit | undefined;
+		expect(init?.redirect).toBe("manual");
+	});
+});

packages/core/src/oauth2/reject-redirects.server.test.ts

@@ -0,0 +1,132 @@
+import type { Server } from "node:http";
+import { createServer } from "node:http";
+import type { AddressInfo } from "node:net";
+import { exportJWK, generateKeyPair, SignJWT } from "jose";
+import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest";
+import { clientCredentialsToken } from "./client-credentials-token";
+import { refreshAccessToken } from "./refresh-access-token";
+import {
+	validateAuthorizationCode,
+	validateToken,
+} from "./validate-authorization-code";
+
+/**
+ * Drives the real network stack (undici / betterFetch / jose) against a real
+ * loopback server that 302-redirects to an "internal" endpoint. Proves the
+ * control behaviorally: the request is rejected AND the redirect target is never
+ * connected to. A mocked fetch cannot prove the second part, because there is no
+ * real second request to observe.
+ */
+describe("server-side OAuth fetches never follow a redirect to an internal host", () => {
+	let server: Server;
+	let baseUrl: string;
+	let internalHit = false;
+	let signedToken: string;
+
+	beforeAll(async () => {
+		const { publicKey, privateKey } = await generateKeyPair("RS256", {
+			extractable: true,
+		});
+		const publicJWK = await exportJWK(publicKey);
+		publicJWK.kid = "test-key";
+		publicJWK.alg = "RS256";
+		signedToken = await new SignJWT({ sub: "user" })
+			.setProtectedHeader({ alg: "RS256", kid: "test-key" })
+			.setIssuedAt()
+			.setExpirationTime("1h")
+			.sign(privateKey);
+
+		server = createServer((req, res) => {
+			const path = (req.url ?? "/").split("?")[0];
+			if (path === "/redirecting-token" || path === "/redirecting-jwks") {
+				const target =
+					path === "/redirecting-jwks" ? "/internal-jwks" : "/internal-token";
+				res.writeHead(302, { location: `${baseUrl}${target}` });
+				res.end();
+				return;
+			}
+			if (path === "/internal-token") {
+				internalHit = true;
+				res.writeHead(200, { "content-type": "application/json" });
+				res.end(
+					JSON.stringify({
+						access_token: "leaked-internal-token",
+						token_type: "Bearer",
+					}),
+				);
+				return;
+			}
+			if (path === "/internal-jwks") {
+				internalHit = true;
+				res.writeHead(200, { "content-type": "application/json" });
+				res.end(JSON.stringify({ keys: [publicJWK] }));
+				return;
+			}
+			res.writeHead(404);
+			res.end();
+		});
+
+		await new Promise<void>((resolve) =>
+			server.listen(0, "127.0.0.1", resolve),
+		);
+		const { port } = server.address() as AddressInfo;
+		baseUrl = `http://127.0.0.1:${port}`;
+	});
+
+	afterAll(async () => {
+		await new Promise<void>((resolve) => server.close(() => resolve()));
+	});
+
+	beforeEach(() => {
+		internalHit = false;
+	});
+
+	it("sanity: a client that follows redirects does reach the internal endpoint", async () => {
+		const response = await fetch(`${baseUrl}/redirecting-token`, {
+			redirect: "follow",
+		});
+		await response.text();
+		expect(internalHit).toBe(true);
+	});
+
+	it("validateAuthorizationCode rejects the redirect and never connects to the internal host", async () => {
+		await expect(
+			validateAuthorizationCode({
+				code: "auth-code",
+				redirectURI: `${baseUrl}/callback`,
+				options: { clientId: "client", clientSecret: "secret" },
+				tokenEndpoint: `${baseUrl}/redirecting-token`,
+			}),
+		).rejects.toThrow(/refuse redirects to prevent SSRF/);
+		expect(internalHit).toBe(false);
+	});
+
+	it("refreshAccessToken rejects the redirect and never connects to the internal host", async () => {
+		await expect(
+			refreshAccessToken({
+				refreshToken: "refresh-token",
+				options: { clientId: "client", clientSecret: "secret" },
+				tokenEndpoint: `${baseUrl}/redirecting-token`,
+			}),
+		).rejects.toThrow(/refuse redirects to prevent SSRF/);
+		expect(internalHit).toBe(false);
+	});
+
+	it("clientCredentialsToken rejects the redirect and never connects to the internal host", async () => {
+		await expect(
+			clientCredentialsToken({
+				options: { clientId: "client", clientSecret: "secret" },
+				tokenEndpoint: `${baseUrl}/redirecting-token`,
+				scope: "openid",
+			}),
+		).rejects.toThrow(/refuse redirects to prevent SSRF/);
+		expect(internalHit).toBe(false);
+	});
+
+	it("validateToken (JWKS) rejects the redirect and never connects to the internal host", async () => {
+		await expect(
+			validateToken(signedToken, `${baseUrl}/redirecting-jwks`),
+		).rejects.toThrow(/refuse redirects to prevent SSRF/);
+		expect(internalHit).toBe(false);
+	});
+});

packages/core/src/oauth2/reject-redirects.test.ts

@@ -0,0 +1,39 @@
+import { describe, expect, it } from "vitest";
+import { BetterAuthError } from "../error";
+import { assertResponseNotRedirect } from "./reject-redirects";
+
+const fakeResponse = (init: { type?: string; status: number }): Response =>
+	({ type: "basic", ...init }) as unknown as Response;
+
+describe("assertResponseNotRedirect", () => {
+	it("throws on a 3xx status (undici-style manual redirect)", () => {
+		for (const status of [301, 302, 303, 307, 308]) {
+			expect(() =>
+				assertResponseNotRedirect(
+					"https://idp.example/token",
+					fakeResponse({ status }),
+				),
+			).toThrow(BetterAuthError);
+		}
+	});
+
+	it("throws on an opaque redirect (spec-runtime manual redirect)", () => {
+		expect(() =>
+			assertResponseNotRedirect(
+				"https://idp.example/token",
+				fakeResponse({ type: "opaqueredirect", status: 0 }),
+			),
+		).toThrow(BetterAuthError);
+	});
+
+	it("does not throw on a non-redirect response", () => {
+		for (const status of [200, 304, 400, 401]) {
+			expect(() =>
+				assertResponseNotRedirect(
+					"https://idp.example/token",
+					fakeResponse({ status }),
+				),
+			).not.toThrow();
+		}
+	});
+});

packages/core/src/oauth2/reject-redirects.ts

@@ -0,0 +1,70 @@
+// cspell:ignore workerd
+import type { BetterFetchOption } from "@better-fetch/fetch";
+import { betterFetch } from "@better-fetch/fetch";
+import { BetterAuthError } from "../error";
+
+const HTTP_REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]);
+
+/**
+ * Whether a response from a `redirect: "manual"` fetch is a redirect.
+ *
+ * Node/undici exposes the real 3xx status. Spec-compliant runtimes (Cloudflare
+ * Workers, Deno, browsers) return an opaque-redirect filtered response with
+ * status 0 and type `"opaqueredirect"`, so the status alone is not enough.
+ */
+function isRedirectResponse(response: Response): boolean {
+	return (
+		response.type === "opaqueredirect" ||
+		HTTP_REDIRECT_STATUSES.has(response.status)
+	);
+}
+
+function redirectRefused(endpoint: string): BetterAuthError {
+	return new BetterAuthError(
+		`The OAuth endpoint "${endpoint}" returned an HTTP redirect. Server-side OAuth fetches refuse redirects to prevent SSRF; configure the final endpoint URL.`,
+	);
+}
+
+/**
+ * Fetch option that refuses HTTP redirects portably.
+ *
+ * Cloudflare Workers (workerd) rejects `redirect: "error"`, so manual mode is
+ * used and the resolved response is checked with {@link assertResponseNotRedirect}
+ * (or, for betterFetch, with {@link fetchRefusingRedirects}).
+ */
+export const NO_FOLLOW_REDIRECT = { redirect: "manual" } as const;
+
+/**
+ * Throw when a native-`fetch` response (e.g. jose's JWKS loader) resolved to a
+ * redirect, so an attacker-influenced endpoint cannot bounce a server-side
+ * request to an internal address.
+ */
+export function assertResponseNotRedirect(
+	endpoint: string,
+	response: Response,
+): void {
+	if (isRedirectResponse(response)) throw redirectRefused(endpoint);
+}
+
+/**
+ * betterFetch that refuses HTTP redirects on a server-side OAuth fetch.
+ *
+ * Returns the betterFetch result and throws if the endpoint redirected, on both
+ * undici (real 3xx status) and spec-compliant runtimes (opaque redirect, where
+ * the error status is 0). The redirect is never followed on any runtime.
+ */
+export async function fetchRefusingRedirects<T>(
+	url: string,
+	options?: BetterFetchOption,
+) {
+	let redirected = false;
+	const result = await betterFetch<T>(url, {
+		...options,
+		...NO_FOLLOW_REDIRECT,
+		onError(context) {
+			if (isRedirectResponse(context.response)) redirected = true;
+		},
+	});
+	if (redirected) throw redirectRefused(url);
+	return result;
+}

packages/core/src/oauth2/validate-authorization-code.ts

@@ -1,9 +1,13 @@
 import { base64 } from "@better-auth/utils/base64";
-import { betterFetch } from "@better-fetch/fetch";
-import { createRemoteJWKSet, jwtVerify } from "jose";
+import { createRemoteJWKSet, customFetch, jwtVerify } from "jose";
 import type { AwaitableFunction } from "../types";
 import type { ProviderOptions } from "./index";
 import { getOAuth2Tokens } from "./index";
+import {
+	assertResponseNotRedirect,
+	fetchRefusingRedirects,
+	NO_FOLLOW_REDIRECT,
+} from "./reject-redirects";
 
 export async function authorizationCodeRequest({
 	code,
@@ -151,7 +155,7 @@ export async function validateAuthorizationCode({
 		resource,
 	});
 
-	const { data, error } = await betterFetch<object>(tokenEndpoint, {
+	const { data, error } = await fetchRefusingRedirects<object>(tokenEndpoint, {
 		method: "POST",
 		body: body,
 		headers: requestHeaders,
@@ -171,7 +175,13 @@ export async function validateToken(
 		issuer?: string | string[];
 	},
 ) {
-	const jwks = createRemoteJWKSet(new URL(jwksEndpoint));
+	const jwks = createRemoteJWKSet(new URL(jwksEndpoint), {
+		[customFetch]: async (url, init) => {
+			const response = await fetch(url, { ...init, ...NO_FOLLOW_REDIRECT });
+			assertResponseNotRedirect(String(url), response);
+			return response;
+		},
+	});
 	const verified = await jwtVerify(token, jwks, {
 		audience: options?.audience,
 		issuer: options?.issuer,

packages/core/src/oauth2/validate-token.test.ts

@@ -154,6 +154,32 @@ describe("validateToken", () => {
 		).rejects.toBeDefined();
 	});
 
+	/**
+	 * The JWKS endpoint is reached via OIDC discovery, so its URL is influenced by
+	 * data an authenticated user can register; following a redirect could bounce
+	 * the server-side request to an internal address.
+	 */
+	it("refuses a redirecting JWKS endpoint and fetches with redirects disabled", async () => {
+		const { privateKey, kid } = await createTestJWKS("RS256");
+		const token = await createSignedToken(privateKey, "RS256", kid);
+		mockedFetch.mockClear();
+		mockedFetch.mockResolvedValueOnce(
+			new Response("", {
+				status: 302,
+				headers: { location: "http://169.254.169.254/" },
+			}),
+		);
+
+		await expect(
+			validateToken(token, "https://example.com/.well-known/jwks"),
+		).rejects.toThrow(/refuse redirects to prevent SSRF/);
+
+		expect(mockedFetch).toHaveBeenCalledWith(
+			expect.anything(),
+			expect.objectContaining({ redirect: "manual" }),
+		);
+	});
+
 	it("should verify token with matching audience", async () => {
 		const { publicJWK, privateKey, kid } = await createTestJWKS("RS256");
 		const token = await createSignedToken(privateKey, "RS256", kid);

packages/core/src/oauth2/verify.ts

@@ -1,4 +1,3 @@
-import { betterFetch } from "@better-fetch/fetch";
 import { APIError } from "better-call";
 import type {
 	JSONWebKeySet,
@@ -15,6 +14,7 @@ import {
 	UnsecuredJWT,
 } from "jose";
 import { logger } from "../env";
+import { fetchRefusingRedirects } from "./reject-redirects";
 
 const joseInfrastructureErrorCodes = new Set([
 	joseErrors.JWKSTimeout.code,
@@ -107,7 +107,7 @@ async function fetchJwks(
 ): Promise<JSONWebKeySet> {
 	const jwks =
 		typeof jwksFetch === "string"
-			? await betterFetch<JSONWebKeySet>(jwksFetch, {
+			? await fetchRefusingRedirects<JSONWebKeySet>(jwksFetch, {
 					headers: {
 						Accept: "application/json",
 					},
@@ -332,23 +332,24 @@ export async function verifyAccessToken(
 
 	// Remote verify
 	if (opts?.remoteVerify) {
-		const { data: introspect, error: introspectError } = await betterFetch<
-			JWTPayload & {
-				active: boolean;
-			}
-		>(opts.remoteVerify.introspectUrl, {
-			method: "POST",
-			headers: {
-				Accept: "application/json",
-				"Content-Type": "application/x-www-form-urlencoded",
-			},
-			body: new URLSearchParams({
-				client_id: opts.remoteVerify.clientId,
-				client_secret: opts.remoteVerify.clientSecret,
-				token,
-				token_type_hint: "access_token",
-			}).toString(),
-		});
+		const { data: introspect, error: introspectError } =
+			await fetchRefusingRedirects<
+				JWTPayload & {
+					active: boolean;
+				}
+			>(opts.remoteVerify.introspectUrl, {
+				method: "POST",
+				headers: {
+					Accept: "application/json",
+					"Content-Type": "application/x-www-form-urlencoded",
+				},
+				body: new URLSearchParams({
+					client_id: opts.remoteVerify.clientId,
+					client_secret: opts.remoteVerify.clientSecret,
+					token,
+					token_type_hint: "access_token",
+				}).toString(),
+			});
 		if (introspectError)
 			logger.error(
 				`Introspection failed: ${introspectError.message ?? introspectError.statusText}`,