diff --git a/.changeset/oauth-fetch-refuse-redirects.md b/.changeset/oauth-fetch-refuse-redirects.md new file mode 100644 index 0000000000..e82a31e737 --- /dev/null +++ b/.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. diff --git a/docs/content/docs/reference/security.mdx b/docs/content/docs/reference/security.mdx index 39320b31cc..24db1076a7 100644 --- a/docs/content/docs/reference/security.mdx +++ b/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. +## 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. diff --git a/packages/core/src/oauth2/client-credentials-token.ts b/packages/core/src/oauth2/client-credentials-token.ts index 97c57daa35..496ea88f50 100644 --- a/packages/core/src/oauth2/client-credentials-token.ts +++ b/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; diff --git a/packages/core/src/oauth2/refresh-access-token.ts b/packages/core/src/oauth2/refresh-access-token.ts index de3bf2b27b..d7a8c739d3 100644 --- a/packages/core/src/oauth2/refresh-access-token.ts +++ b/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; diff --git a/packages/core/src/oauth2/reject-redirects.runtime.test.ts b/packages/core/src/oauth2/reject-redirects.runtime.test.ts new file mode 100644 index 0000000000..4659ef795d --- /dev/null +++ b/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; + + 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"); + }); +}); diff --git a/packages/core/src/oauth2/reject-redirects.server.test.ts b/packages/core/src/oauth2/reject-redirects.server.test.ts new file mode 100644 index 0000000000..b693fc4ed8 --- /dev/null +++ b/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((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((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); + }); +}); diff --git a/packages/core/src/oauth2/reject-redirects.test.ts b/packages/core/src/oauth2/reject-redirects.test.ts new file mode 100644 index 0000000000..a47bee30f4 --- /dev/null +++ b/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(); + } + }); +}); diff --git a/packages/core/src/oauth2/reject-redirects.ts b/packages/core/src/oauth2/reject-redirects.ts new file mode 100644 index 0000000000..2f938ec1f0 --- /dev/null +++ b/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( + url: string, + options?: BetterFetchOption, +) { + let redirected = false; + const result = await betterFetch(url, { + ...options, + ...NO_FOLLOW_REDIRECT, + onError(context) { + if (isRedirectResponse(context.response)) redirected = true; + }, + }); + if (redirected) throw redirectRefused(url); + return result; +} diff --git a/packages/core/src/oauth2/validate-authorization-code.ts b/packages/core/src/oauth2/validate-authorization-code.ts index f4c8bec524..7bce42bb53 100644 --- a/packages/core/src/oauth2/validate-authorization-code.ts +++ b/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(tokenEndpoint, { + const { data, error } = await fetchRefusingRedirects(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, diff --git a/packages/core/src/oauth2/validate-token.test.ts b/packages/core/src/oauth2/validate-token.test.ts index 4c64843fbe..888d6efe0a 100644 --- a/packages/core/src/oauth2/validate-token.test.ts +++ b/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); diff --git a/packages/core/src/oauth2/verify.ts b/packages/core/src/oauth2/verify.ts index 606e051b81..afaca88031 100644 --- a/packages/core/src/oauth2/verify.ts +++ b/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 { const jwks = typeof jwksFetch === "string" - ? await betterFetch(jwksFetch, { + ? await fetchRefusingRedirects(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}`,