mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-22 15:59:56 -05:00
fix(core): verify id_tokens with one shared policy (#11919)
This commit is contained in:
@@ -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.
|
||||
@@ -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;
|
||||
|
||||
@@ -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<PayloadType = JWTPayload>(
|
||||
token: string,
|
||||
jwksEndpoint: string,
|
||||
|
||||
@@ -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"],
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<OAuthProvider, "idToken" | "options">;
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user