From 876608da86036d9ca4b5c2a37ed0b01e29264e52 Mon Sep 17 00:00:00 2001 From: Taesu <166604494+bytaesu@users.noreply.github.com> Date: Mon, 19 Oct 2026 21:06:58 +0000 Subject: [PATCH] fix(core): verify id_tokens with one shared policy (#11919) --- .changeset/id-token-required-claims.md | 5 + packages/core/src/oauth2/oauth-provider.ts | 5 +- .../src/oauth2/validate-authorization-code.ts | 8 ++ .../core/src/oauth2/verify-id-token.test.ts | 56 ++++++++ packages/core/src/oauth2/verify-id-token.ts | 51 ++++++-- .../core/src/social-providers/apple.test.ts | 6 +- packages/core/src/social-providers/apple.ts | 1 + packages/core/src/social-providers/cognito.ts | 1 + .../src/social-providers/facebook.test.ts | 123 ++++++++++++++++++ .../core/src/social-providers/facebook.ts | 13 ++ packages/core/src/social-providers/google.ts | 20 +-- 11 files changed, 265 insertions(+), 24 deletions(-) create mode 100644 .changeset/id-token-required-claims.md diff --git a/.changeset/id-token-required-claims.md b/.changeset/id-token-required-claims.md new file mode 100644 index 0000000000..fbf5cf2062 --- /dev/null +++ b/.changeset/id-token-required-claims.md @@ -0,0 +1,5 @@ +--- +"@better-auth/core": patch +--- + +A custom Facebook `getUserInfo` no longer receives client-submitted access tokens issued to another Facebook app, and ID tokens missing `exp` or `iat` are now rejected. diff --git a/packages/core/src/oauth2/oauth-provider.ts b/packages/core/src/oauth2/oauth-provider.ts index 333bb94c60..e707195cfb 100644 --- a/packages/core/src/oauth2/oauth-provider.ts +++ b/packages/core/src/oauth2/oauth-provider.ts @@ -25,7 +25,10 @@ export type OAuthIdTokenConfig = issuer?: (string | string[]) | undefined; /** Expected `aud`, usually the client ID. */ audience: string | string[]; - /** Permitted JWS algorithms. Defaults to the token's `alg` header. */ + /** + * Permitted JWS algorithms. Set this to the algorithms the provider signs with. + * When omitted, jose accepts any algorithm the resolved key supports. + */ algorithms?: string[] | undefined; /** Maximum token age passed to jose (e.g. `"1h"`). */ maxTokenAge?: string | undefined; diff --git a/packages/core/src/oauth2/validate-authorization-code.ts b/packages/core/src/oauth2/validate-authorization-code.ts index 7e4efa307b..6b4b8566db 100644 --- a/packages/core/src/oauth2/validate-authorization-code.ts +++ b/packages/core/src/oauth2/validate-authorization-code.ts @@ -158,6 +158,14 @@ export async function validateAuthorizationCode({ return tokens; } +/** + * Verifies a JWT's signature against the key set at `jwksEndpoint`. + * + * Beyond the signature, only `exp` and `nbf` (when present) and the claims named + * in `options` are checked. Without `audience` and `issuer`, a token the same key + * set signed for another client or issuer also verifies, so pass both whenever + * the token proves identity. + */ export async function validateToken( token: string, jwksEndpoint: string, diff --git a/packages/core/src/oauth2/verify-id-token.test.ts b/packages/core/src/oauth2/verify-id-token.test.ts index 126d340fda..2d85ea6a3d 100644 --- a/packages/core/src/oauth2/verify-id-token.test.ts +++ b/packages/core/src/oauth2/verify-id-token.test.ts @@ -2,7 +2,11 @@ import { createHash } from "node:crypto"; import type { JWK } from "jose"; import { createLocalJWKSet, exportJWK, generateKeyPair, SignJWT } from "jose"; import { describe, expect, it } from "vitest"; +import { apple } from "../social-providers/apple"; +import { cognito } from "../social-providers/cognito"; import { facebook } from "../social-providers/facebook"; +import { google } from "../social-providers/google"; +import { microsoft } from "../social-providers/microsoft-entra-id"; import { paypal } from "../social-providers/paypal"; import type { OAuthIdTokenConfig } from "./oauth-provider"; import { @@ -93,6 +97,26 @@ describe("verifyProviderIdToken", () => { expect(await verifyProviderIdToken(provider, none)).toBe(false); }); + it("requires the exp and iat claims every ID token carries", async () => { + const { jwks, privateKey } = await makeKeyset(); + const provider = providerWith({ jwks, issuer: ISSUER, audience: AUDIENCE }); + const withoutExp = await new SignJWT({ sub: "u1" }) + .setProtectedHeader({ alg: "RS256", kid: "test-key" }) + .setIssuer(ISSUER) + .setAudience(AUDIENCE) + .setIssuedAt() + .sign(privateKey); + const withoutIat = await new SignJWT({ sub: "u1" }) + .setProtectedHeader({ alg: "RS256", kid: "test-key" }) + .setIssuer(ISSUER) + .setAudience(AUDIENCE) + .setExpirationTime("1h") + .sign(privateKey); + + expect(await verifyProviderIdToken(provider, withoutExp)).toBe(false); + expect(await verifyProviderIdToken(provider, withoutIat)).toBe(false); + }); + it("rejects a wrong issuer or audience", async () => { const { jwks, sign } = await makeKeyset(); const token = await sign({ sub: "u1" }); @@ -251,5 +275,37 @@ describe("verifyProviderIdToken", () => { await verifyProviderIdToken(provider, "opaque-graph-access-token"), ).toBe(true); }); + + it("pins the signing algorithms of every provider that verifies against a JWKS", () => { + const providers = [ + apple({ clientId: "c", clientSecret: "s" }), + cognito({ + clientId: "c", + clientSecret: "s", + domain: "auth.example.com", + region: "us-east-1", + userPoolId: "us-east-1_pool", + }), + facebook({ clientId: "c", clientSecret: "s" }), + google({ clientId: "c", clientSecret: "s" }), + microsoft({ clientId: "c", clientSecret: "s" }), + ]; + const pinned = Object.fromEntries( + providers.map((provider) => [ + provider.id, + provider.idToken && "jwks" in provider.idToken + ? provider.idToken.algorithms + : undefined, + ]), + ); + + expect(pinned).toEqual({ + apple: ["RS256"], + cognito: ["RS256"], + facebook: ["RS256"], + google: ["RS256"], + microsoft: ["RS256"], + }); + }); }); }); diff --git a/packages/core/src/oauth2/verify-id-token.ts b/packages/core/src/oauth2/verify-id-token.ts index 8bfd015c5d..01890954f1 100644 --- a/packages/core/src/oauth2/verify-id-token.ts +++ b/packages/core/src/oauth2/verify-id-token.ts @@ -1,11 +1,42 @@ -import { decodeProtectedHeader, jwtVerify } from "jose"; +import type { JWTVerifyOptions } from "jose"; +import { jwtVerify } from "jose"; import type { GenericEndpointContext } from "../types"; import type { OAuthProvider, ProviderOptions } from "./oauth-provider"; /** * Clock skew allowed on `exp`, `nbf`, and `iat`, matching openid-client and oauth4webapi. */ -export const ID_TOKEN_CLOCK_TOLERANCE_SECONDS = 30; +const ID_TOKEN_CLOCK_TOLERANCE_SECONDS = 30; + +/** + * Claims every ID token must contain. + * + * @see https://openid.net/specs/openid-connect-core-1_0.html#IDToken + */ +const ID_TOKEN_REQUIRED_CLAIMS = ["iss", "sub", "aud", "exp", "iat"]; + +/** + * Builds the jose options shared by every ID token verifier, so the required + * claims and clock tolerance stay the same across providers. Algorithms come + * from the caller's trusted configuration, never from the token header. + * + * @internal + */ +export function getIdTokenVerifyOptions(options: { + issuer?: (string | string[]) | undefined; + audience: string | string[]; + algorithms?: string[] | undefined; + maxTokenAge?: (string | number) | undefined; +}) { + return { + issuer: options.issuer, + audience: options.audience, + algorithms: options.algorithms, + maxTokenAge: options.maxTokenAge, + requiredClaims: ID_TOKEN_REQUIRED_CLAIMS, + clockTolerance: ID_TOKEN_CLOCK_TOLERANCE_SECONDS, + } satisfies JWTVerifyOptions; +} type ProviderWithIdTokenConfig = Pick; @@ -94,16 +125,12 @@ export async function verifyProviderIdToken( return config.allowOpaqueToken === true; } // `kid` is optional in JWS: a JWKS resolver can still select a key by algorithm, so - // key selection is left to config.jwks. The token's `alg` only seeds the default - // allowed-algorithms list when the provider does not pin one. - const { alg } = decodeProtectedHeader(token); - const { payload } = await jwtVerify(token, config.jwks, { - issuer: config.issuer, - audience: config.audience, - algorithms: config.algorithms ?? (alg ? [alg] : undefined), - maxTokenAge: config.maxTokenAge, - clockTolerance: ID_TOKEN_CLOCK_TOLERANCE_SECONDS, - }); + // key selection is left to config.jwks. + const { payload } = await jwtVerify( + token, + config.jwks, + getIdTokenVerifyOptions(config), + ); if ( nonce && !(await nonceMatches(payload.nonce, nonce, config.nonceComparison)) diff --git a/packages/core/src/social-providers/apple.test.ts b/packages/core/src/social-providers/apple.test.ts index 27619924bb..d3124f5c39 100644 --- a/packages/core/src/social-providers/apple.test.ts +++ b/packages/core/src/social-providers/apple.test.ts @@ -17,12 +17,12 @@ import { apple } from "./apple"; const mockedBetterFetch = vi.mocked(betterFetch); async function createSignedAppleToken(payloadNonce: string) { - const { publicKey, privateKey } = await generateKeyPair("ES256", { + const { publicKey, privateKey } = await generateKeyPair("RS256", { extractable: true, }); const publicJWK = await exportJWK(publicKey); publicJWK.kid = "test-apple-key"; - publicJWK.alg = "ES256"; + publicJWK.alg = "RS256"; publicJWK.use = "sig"; const token = await new SignJWT({ @@ -31,7 +31,7 @@ async function createSignedAppleToken(payloadNonce: string) { email_verified: true, nonce: payloadNonce, }) - .setProtectedHeader({ alg: "ES256", kid: "test-apple-key" }) + .setProtectedHeader({ alg: "RS256", kid: "test-apple-key" }) .setIssuer("https://appleid.apple.com") .setAudience("com.example.app") .setIssuedAt() diff --git a/packages/core/src/social-providers/apple.ts b/packages/core/src/social-providers/apple.ts index aab88b6006..b07196705e 100644 --- a/packages/core/src/social-providers/apple.ts +++ b/packages/core/src/social-providers/apple.ts @@ -125,6 +125,7 @@ export const apple = (options: AppleOptions) => { idToken: { jwks: (header) => getApplePublicKey(header.kid!), issuer: "https://appleid.apple.com", + algorithms: ["RS256"], audience: options.audience && options.audience.length ? options.audience diff --git a/packages/core/src/social-providers/cognito.ts b/packages/core/src/social-providers/cognito.ts index e98b979d9d..a8b7fd3729 100644 --- a/packages/core/src/social-providers/cognito.ts +++ b/packages/core/src/social-providers/cognito.ts @@ -161,6 +161,7 @@ export const cognito = (options: CognitoOptions) => { getCognitoPublicKey(header.kid!, options.region, options.userPoolId), issuer: `https://cognito-idp.${options.region}.amazonaws.com/${options.userPoolId}`, audience: options.clientId, + algorithms: ["RS256"], maxTokenAge: "1h", }, diff --git a/packages/core/src/social-providers/facebook.test.ts b/packages/core/src/social-providers/facebook.test.ts index 5751ce4654..56359ba21e 100644 --- a/packages/core/src/social-providers/facebook.test.ts +++ b/packages/core/src/social-providers/facebook.test.ts @@ -6,6 +6,7 @@ vi.mock("@better-fetch/fetch", () => ({ import { betterFetch } from "@better-fetch/fetch"; +import { verifyProviderIdToken } from "../oauth2/verify-id-token"; import { facebook } from "./facebook"; const mockedBetterFetch = vi.mocked(betterFetch); @@ -134,3 +135,125 @@ describe("facebook.getUserInfo (opaque access token)", () => { expect(mockedBetterFetch).not.toHaveBeenCalled(); }); }); + +describe("facebook custom getUserInfo on client id_token sign-in", () => { + const expectDebugTokenInput = (inputToken: string) => + expect(mockedBetterFetch).toHaveBeenCalledWith( + "https://graph.facebook.com/debug_token", + expect.objectContaining({ + query: expect.objectContaining({ input_token: inputToken }), + }), + ); + + beforeEach(() => { + mockedBetterFetch.mockReset(); + }); + + it("does not resolve identity from an access token issued to another app", async () => { + mockGraphResponses({ + debug: debugTokenResponse({ + is_valid: true, + app_id: "someone-elses-app", + user_id: "victim", + }), + }); + const getUserInfo = vi.fn().mockResolvedValue({ + user: { email: "victim@example.com", emailVerified: true }, + data: fbProfile("victim"), + }); + const provider = facebook({ ...options, getUserInfo }); + + const res = await provider.getUserInfo({ + idToken: "placeholder", + accessToken: "foreign-app-token", + }); + + expect(res).toBeNull(); + expectDebugTokenInput("foreign-app-token"); + expect(getUserInfo).not.toHaveBeenCalled(); + }); + + it.each([undefined, ""])( + "checks the opaque id token when the access token is %j", + async (accessToken) => { + mockGraphResponses({ + debug: debugTokenResponse({ + is_valid: true, + app_id: "someone-elses-app", + user_id: "victim", + }), + }); + const getUserInfo = vi.fn(); + const provider = facebook({ ...options, getUserInfo }); + + const res = await provider.getUserInfo({ + idToken: "foreign-app-token", + accessToken, + }); + + expect(res).toBeNull(); + expectDebugTokenInput("foreign-app-token"); + expect(getUserInfo).not.toHaveBeenCalled(); + }, + ); + + it("does not trust a foreign access token sent alongside a signed id token", async () => { + mockGraphResponses({ + debug: debugTokenResponse({ + is_valid: true, + app_id: "someone-elses-app", + user_id: "victim", + }), + }); + const getUserInfo = vi.fn(); + const provider = facebook({ ...options, getUserInfo }); + + const res = await provider.getUserInfo({ + idToken: "header.payload.signature", + accessToken: "foreign-app-token", + }); + + expect(res).toBeNull(); + expectDebugTokenInput("foreign-app-token"); + expect(getUserInfo).not.toHaveBeenCalled(); + }); + + it("passes a signed id token without an access token straight to a custom getUserInfo", async () => { + mockGraphResponses({}); + const getUserInfo = vi.fn().mockResolvedValue({ + user: { email: "u1@example.com", emailVerified: false }, + data: { sub: "u1" }, + }); + const provider = facebook({ ...options, getUserInfo }); + + await provider.getUserInfo({ idToken: "header.payload.signature" }); + + expect(getUserInfo).toHaveBeenCalledOnce(); + }); + + it("passes an access token bound to this app to a custom getUserInfo", async () => { + mockGraphResponses({ + debug: debugTokenResponse({ + is_valid: true, + app_id: "fb-app", + user_id: "u1", + }), + }); + const getUserInfo = vi.fn().mockResolvedValue({ + user: { email: "u1@example.com", emailVerified: true }, + data: fbProfile("u1"), + }); + const provider = facebook({ ...options, getUserInfo }); + + await expect(verifyProviderIdToken(provider, "placeholder")).resolves.toBe( + true, + ); + await provider.getUserInfo({ + idToken: "placeholder", + accessToken: "opaque-access-token", + }); + + expectDebugTokenInput("opaque-access-token"); + expect(getUserInfo).toHaveBeenCalledOnce(); + }); +}); diff --git a/packages/core/src/social-providers/facebook.ts b/packages/core/src/social-providers/facebook.ts index 470c76d6d7..416ac35cd5 100644 --- a/packages/core/src/social-providers/facebook.ts +++ b/packages/core/src/social-providers/facebook.ts @@ -176,6 +176,19 @@ export const facebook = (options: FacebookOptions) => { }, async getUserInfo(token) { if (options.getUserInfo) { + // On the client sign-in path the access token, or an opaque `idToken`, reaches + // here unverified, so bind it to this app before a custom getUserInfo resolves + // identity from it. + const isOpaqueIdToken = + !!token.idToken && token.idToken.split(".").length !== 3; + const clientToken = + token.accessToken || (isOpaqueIdToken ? token.idToken : undefined); + if ( + clientToken && + !(await verifyFacebookAccessToken(clientToken, options)) + ) { + return null; + } return options.getUserInfo(token); } diff --git a/packages/core/src/social-providers/google.ts b/packages/core/src/social-providers/google.ts index 22ad804562..4efa3c8fbc 100644 --- a/packages/core/src/social-providers/google.ts +++ b/packages/core/src/social-providers/google.ts @@ -10,7 +10,7 @@ import { refreshAccessToken, validateAuthorizationCode, } from "../oauth2"; -import { ID_TOKEN_CLOCK_TOLERANCE_SECONDS } from "../oauth2/verify-id-token"; +import { getIdTokenVerifyOptions } from "../oauth2/verify-id-token"; export interface GoogleProfile { aud: string; @@ -108,13 +108,16 @@ export const verifyGoogleIdToken = async ({ const publicKeys = await getGooglePublicKeys(kid); for (const publicKey of publicKeys) { try { - const { payload: jwtClaims } = await jwtVerify(token, publicKey, { - algorithms: GOOGLE_ID_TOKEN_ALGORITHMS, - issuer: ["https://accounts.google.com", "accounts.google.com"], - audience, - maxTokenAge: GOOGLE_ID_TOKEN_MAX_AGE, - clockTolerance: ID_TOKEN_CLOCK_TOLERANCE_SECONDS, - }); + const { payload: jwtClaims } = await jwtVerify( + token, + publicKey, + getIdTokenVerifyOptions({ + issuer: ["https://accounts.google.com", "accounts.google.com"], + audience, + algorithms: GOOGLE_ID_TOKEN_ALGORITHMS, + maxTokenAge: GOOGLE_ID_TOKEN_MAX_AGE, + }), + ); if (nonce && jwtClaims.nonce !== nonce) { return null; @@ -225,6 +228,7 @@ export const google = (options: GoogleOptions) => { jwks: (header) => getGooglePublicKey(header.kid!), issuer: ["https://accounts.google.com", "accounts.google.com"], audience: options.clientId, + algorithms: GOOGLE_ID_TOKEN_ALGORITHMS, maxTokenAge: GOOGLE_ID_TOKEN_MAX_AGE, // Google's `hd` authorization parameter is only a UI hint and can be // removed or changed by the user. When a hosted domain is configured,