mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-22 15:59:56 -05:00
fix(sso): verify OIDC ID tokens with the shared step (#11935)
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@better-auth/sso": patch
|
||||
---
|
||||
|
||||
OIDC SSO sign-in now rejects ID tokens missing `exp` or `iat` and providers with an empty client ID, matching social sign-in.
|
||||
@@ -104,6 +104,11 @@
|
||||
"types": "./dist/oauth2/index.d.mts",
|
||||
"default": "./dist/oauth2/index.mjs"
|
||||
},
|
||||
"./oauth2/internal": {
|
||||
"dev-source": "./src/oauth2/internal.ts",
|
||||
"types": "./dist/oauth2/internal.d.mts",
|
||||
"default": "./dist/oauth2/internal.mjs"
|
||||
},
|
||||
"./instrumentation": {
|
||||
"dev-source": "./src/instrumentation/index.ts",
|
||||
"types": "./dist/instrumentation/index.d.mts",
|
||||
@@ -158,6 +163,9 @@
|
||||
"oauth2": [
|
||||
"dist/oauth2/index.d.mts"
|
||||
],
|
||||
"oauth2/internal": [
|
||||
"dist/oauth2/internal.d.mts"
|
||||
],
|
||||
"instrumentation": [
|
||||
"dist/instrumentation/index.d.mts"
|
||||
]
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
export { verifyIdToken } from "./verify-id-token";
|
||||
@@ -1,8 +1,7 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import type { JWK } from "jose";
|
||||
import { createLocalJWKSet, exportJWK, generateKeyPair, SignJWT } from "jose";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { logger } from "../env";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { apple } from "../social-providers/apple";
|
||||
import { cognito } from "../social-providers/cognito";
|
||||
import { facebook } from "../social-providers/facebook";
|
||||
@@ -261,17 +260,12 @@ describe("verifyProviderIdToken", () => {
|
||||
});
|
||||
|
||||
describe("without a configured audience", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it.each<[string, string | string[] | undefined]>([
|
||||
["missing", undefined],
|
||||
["an empty string", ""],
|
||||
["an empty list", []],
|
||||
["a list of empty strings", [""]],
|
||||
])("rejects a token when the audience is %s", async (_, audience) => {
|
||||
const error = vi.spyOn(logger, "error").mockImplementation(() => {});
|
||||
const { jwks, sign } = await makeKeyset();
|
||||
const provider = providerWith({
|
||||
jwks,
|
||||
@@ -283,9 +277,6 @@ describe("verifyProviderIdToken", () => {
|
||||
await expect(
|
||||
verifyProviderIdToken(provider, await sign({ sub: "u1" })),
|
||||
).resolves.toBe(false);
|
||||
expect(error).toHaveBeenCalledWith(
|
||||
expect.stringContaining("no client ID"),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
import type { JWTVerifyOptions } from "jose";
|
||||
import { jwtVerify } from "jose";
|
||||
import { logger } from "../env";
|
||||
import type { GenericEndpointContext } from "../types";
|
||||
import type { OAuthProvider, ProviderOptions } from "./oauth-provider";
|
||||
import type {
|
||||
OAuthIdTokenConfig,
|
||||
OAuthProvider,
|
||||
ProviderOptions,
|
||||
} from "./oauth-provider";
|
||||
|
||||
/**
|
||||
* Clock skew allowed on `exp`, `nbf`, and `iat`, matching openid-client and oauth4webapi.
|
||||
@@ -69,6 +72,44 @@ async function nonceMatches(
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Verifies an ID token's signature and claims against a verification config,
|
||||
* and its `nonce` when one is expected. Throws when the token is not valid.
|
||||
*
|
||||
* @see https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
|
||||
* @internal
|
||||
*/
|
||||
export async function verifyIdToken(
|
||||
token: string,
|
||||
config: Extract<OAuthIdTokenConfig, { jwks: unknown }>,
|
||||
nonce?: string,
|
||||
) {
|
||||
// An empty audience would skip the audience check entirely.
|
||||
if (![config.audience].flat().some(Boolean)) {
|
||||
throw new Error("ID token audience is not configured");
|
||||
}
|
||||
// `kid` is optional in JWS: a JWKS resolver can still select a key by algorithm, so
|
||||
// key selection is left to config.jwks.
|
||||
const verified = await jwtVerify(
|
||||
token,
|
||||
config.jwks,
|
||||
getIdTokenVerifyOptions(config),
|
||||
);
|
||||
if (
|
||||
nonce &&
|
||||
!(await nonceMatches(verified.payload.nonce, nonce, config.nonceComparison))
|
||||
) {
|
||||
throw new Error("ID token nonce does not match");
|
||||
}
|
||||
// Provider-specific claim check on the now-verified payload (e.g. Google's
|
||||
// hosted-domain `hd`). Standard checks have already passed, so a token that
|
||||
// fails here is authentic but does not meet the provider's extra constraint.
|
||||
if (config.verifyClaims && !config.verifyClaims(verified.payload)) {
|
||||
throw new Error("ID token claims were rejected");
|
||||
}
|
||||
return verified;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a provider can verify a client-submitted id_token.
|
||||
*
|
||||
@@ -128,33 +169,7 @@ export async function verifyProviderIdToken(
|
||||
if (token.split(".").length !== 3) {
|
||||
return config.allowOpaqueToken === true;
|
||||
}
|
||||
// An empty audience would skip the audience check entirely.
|
||||
const hasAudience = [config.audience].flat().some(Boolean);
|
||||
if (!hasAudience) {
|
||||
logger.error(
|
||||
`[better-auth] Rejected an ID token for "${provider.id}" because the provider has no client ID to check its audience against. Set \`clientId\` on the provider.`,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
// `kid` is optional in JWS: a JWKS resolver can still select a key by algorithm, so
|
||||
// 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))
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
// Provider-specific claim check on the now-verified payload (e.g. Google's
|
||||
// hosted-domain `hd`). Standard checks have already passed, so a token that
|
||||
// fails here is authentic but does not meet the provider's extra constraint.
|
||||
if (config.verifyClaims && !config.verifyClaims(payload)) {
|
||||
return false;
|
||||
}
|
||||
await verifyIdToken(token, config, nonce);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
|
||||
@@ -18,6 +18,7 @@ export default defineConfig({
|
||||
"./src/context/index.ts",
|
||||
"./src/env/index.ts",
|
||||
"./src/oauth2/index.ts",
|
||||
"./src/oauth2/internal.ts",
|
||||
"./src/api/index.ts",
|
||||
"./src/social-providers/index.ts",
|
||||
"./src/utils/*.ts",
|
||||
|
||||
@@ -8,13 +8,14 @@
|
||||
* @see https://openid.net/specs/openid-connect-discovery-1_0.html
|
||||
*/
|
||||
|
||||
import { verifyIdToken } from "@better-auth/core/oauth2/internal";
|
||||
import {
|
||||
classifyHost,
|
||||
isPublicRoutableHost,
|
||||
} from "@better-auth/core/utils/host";
|
||||
import type { BetterFetchOption } from "@better-fetch/fetch";
|
||||
import { betterFetch } from "@better-fetch/fetch";
|
||||
import { createRemoteJWKSet, customFetch, jwtVerify } from "jose";
|
||||
import { createRemoteJWKSet, customFetch } from "jose";
|
||||
import type { OIDCConfig } from "../types";
|
||||
import type {
|
||||
DiscoverOIDCConfigParams,
|
||||
@@ -425,8 +426,8 @@ export async function validateOIDCIdToken(
|
||||
token: string,
|
||||
jwksEndpoint: string,
|
||||
options: {
|
||||
audience?: string | string[];
|
||||
issuer?: string | string[];
|
||||
audience: string;
|
||||
issuer: string;
|
||||
},
|
||||
isTrustedOrigin: (url: string) => boolean,
|
||||
) {
|
||||
@@ -434,13 +435,12 @@ export async function validateOIDCIdToken(
|
||||
[customFetch]: (url, init) =>
|
||||
fetchOIDCEndpointResponse("jwksEndpoint", url, init, isTrustedOrigin),
|
||||
});
|
||||
const verified = await jwtVerify(token, jwks, {
|
||||
const verified = await verifyIdToken(token, {
|
||||
jwks,
|
||||
audience: options.audience,
|
||||
issuer: options.issuer,
|
||||
});
|
||||
if (typeof options.audience === "string") {
|
||||
assertOIDCAuthorizedParty(verified.payload, options.audience);
|
||||
}
|
||||
assertOIDCAuthorizedParty(verified.payload, options.audience);
|
||||
return verified;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { assertOIDCAuthorizedParty } from "./discovery";
|
||||
import { exportJWK, generateKeyPair, SignJWT } from "jose";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { assertOIDCAuthorizedParty, validateOIDCIdToken } from "./discovery";
|
||||
|
||||
describe("OIDC authorized party validation", () => {
|
||||
it("accepts the configured client as azp for a multi-audience token", () => {
|
||||
@@ -30,3 +31,71 @@ describe("OIDC authorized party validation", () => {
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* @see https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
|
||||
*/
|
||||
describe("OIDC ID token claim validation", async () => {
|
||||
const jwksEndpoint = "https://idp.example.com/jwks";
|
||||
const issuer = "https://idp.example.com";
|
||||
const clientId = "workforce-client";
|
||||
const { publicKey, privateKey } = await generateKeyPair("RS256");
|
||||
const jwks = { keys: [{ ...(await exportJWK(publicKey)), alg: "RS256" }] };
|
||||
|
||||
const signIdToken = (claims: { exp?: string; iat?: boolean }) => {
|
||||
const jwt = new SignJWT({})
|
||||
.setProtectedHeader({ alg: "RS256" })
|
||||
.setIssuer(issuer)
|
||||
.setAudience(clientId)
|
||||
.setSubject("user-1");
|
||||
if (claims.exp) jwt.setExpirationTime(claims.exp);
|
||||
if (claims.iat) jwt.setIssuedAt();
|
||||
return jwt.sign(privateKey);
|
||||
};
|
||||
const validate = (token: string, audience = clientId) =>
|
||||
validateOIDCIdToken(
|
||||
token,
|
||||
jwksEndpoint,
|
||||
{
|
||||
audience,
|
||||
issuer,
|
||||
},
|
||||
() => true,
|
||||
);
|
||||
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => Response.json(jwks)),
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("accepts an ID token with exp and iat", async () => {
|
||||
const token = await signIdToken({ exp: "5m", iat: true });
|
||||
|
||||
await expect(validate(token)).resolves.toMatchObject({
|
||||
payload: { sub: "user-1" },
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ claim: "exp", claims: { iat: true } },
|
||||
{ claim: "iat", claims: { exp: "5m" } },
|
||||
])("rejects an ID token without $claim", async ({ claim, claims }) => {
|
||||
const token = await signIdToken(claims);
|
||||
|
||||
await expect(validate(token)).rejects.toThrow(`"${claim}" claim`);
|
||||
});
|
||||
|
||||
it("rejects an ID token when the provider has no client ID", async () => {
|
||||
const token = await signIdToken({ exp: "5m", iat: true });
|
||||
|
||||
await expect(validate(token, "")).rejects.toThrow(
|
||||
"ID token audience is not configured",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user