From cfad76ba6405d85161eb9258f187b71fa2cf19a3 Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Sat, 11 Apr 2026 10:07:11 +0100 Subject: [PATCH] refactor(oauth-provider): resolve signing key once, add runtime alg validation - Replace resolveSigningAlgorithm with resolveSigningKey: resolves the full key once and passes it to signJWT, eliminating redundant DB lookup and key rotation race between alg resolution and signing - Add ResolvedSigningKey type for the resolved key contract - Validate signed ID token header alg against declared alg when using custom jwt.sign callbacks (OIDC Core 3.1.3.6) - Remove stale @ts-expect-error directives --- .changeset/oauth-provider-at-hash.md | 2 +- packages/better-auth/src/plugins/jwt/index.ts | 2 +- packages/better-auth/src/plugins/jwt/sign.ts | 63 ++++++++++--------- packages/better-auth/src/plugins/jwt/types.ts | 13 ++++ packages/oauth-provider/src/token.test.ts | 2 - packages/oauth-provider/src/token.ts | 48 +++++++++----- 6 files changed, 81 insertions(+), 49 deletions(-) diff --git a/.changeset/oauth-provider-at-hash.md b/.changeset/oauth-provider-at-hash.md index 968ae0a031..e682180fcc 100644 --- a/.changeset/oauth-provider-at-hash.md +++ b/.changeset/oauth-provider-at-hash.md @@ -7,4 +7,4 @@ feat(oauth-provider): compute `at_hash` in ID tokens per OIDC Core §3.1.3.6 ID tokens issued alongside an access token now include the `at_hash` claim, which cryptographically binds the two tokens to prevent token substitution attacks. The hash algorithm is selected based on the actual signing key's algorithm (EdDSA/Ed25519 uses SHA-512, RS/ES/PS384 uses SHA-384, RS/ES/PS512 uses SHA-512, all others use SHA-256). -A new `resolveSigningAlgorithm()` export is available from `better-auth/plugins` to query the algorithm of the current JWKS signing key. +A new `resolveSigningKey()` export is available from `better-auth/plugins` to resolve the current JWKS signing key (including its algorithm). When using a custom `jwt.sign` callback, the signed ID token's header is validated against the declared algorithm to prevent `at_hash` mismatches. diff --git a/packages/better-auth/src/plugins/jwt/index.ts b/packages/better-auth/src/plugins/jwt/index.ts index 8b4921ef47..d798c9ac53 100644 --- a/packages/better-auth/src/plugins/jwt/index.ts +++ b/packages/better-auth/src/plugins/jwt/index.ts @@ -16,7 +16,7 @@ import type { JwtOptions } from "./types"; import { createJwk } from "./utils"; import { verifyJWT as verifyJWTHelper } from "./verify"; -export { resolveSigningAlgorithm, signJWT } from "./sign"; +export { resolveSigningKey, signJWT } from "./sign"; export type * from "./types"; export { createJwk, generateExportedKeyPair, toExpJWT } from "./utils"; export { verifyJWT } from "./verify"; diff --git a/packages/better-auth/src/plugins/jwt/sign.ts b/packages/better-auth/src/plugins/jwt/sign.ts index c82807ceff..a702e89ff9 100644 --- a/packages/better-auth/src/plugins/jwt/sign.ts +++ b/packages/better-auth/src/plugins/jwt/sign.ts @@ -4,7 +4,7 @@ import type { JWTPayload } from "jose"; import { importJWK, SignJWT } from "jose"; import { symmetricDecrypt } from "../../crypto"; import { getJwksAdapter } from "./adapter"; -import type { JwtOptions } from "./types"; +import type { JwtOptions, ResolvedSigningKey } from "./types"; import { createJwk, toExpJWT } from "./utils"; type JWTPayloadWithOptional = { @@ -62,26 +62,42 @@ type JWTPayloadWithOptional = { }; /** - * Resolves the signing algorithm that signJWT will use. - * Mirrors signJWT's key resolution so at_hash and similar - * spec-required hashes use the correct algorithm. + * Resolves the JWKS signing key, decrypts it, and imports it + * for use with jose's SignJWT. Returns null when signing is + * delegated to a custom jwt.sign callback. * - * Returns undefined when signing is delegated to a custom - * jwt.sign callback and no explicit algorithm is configured. + * Callers that need the signing algorithm before constructing + * the JWT payload (e.g. for OIDC at_hash) should call this + * first, read `.alg`, then pass the result to `signJWT` via + * the `resolvedKey` option to avoid a redundant DB lookup. */ -export async function resolveSigningAlgorithm( +export async function resolveSigningKey( ctx: GenericEndpointContext, options?: JwtOptions, -): Promise { +): Promise { if (options?.jwt?.sign) { - return options?.jwks?.keyPairConfig?.alg; + return null; } const adapter = getJwksAdapter(ctx.context.adapter, options); let key = await adapter.getLatestKey(ctx); if (!key || (key.expiresAt && key.expiresAt < new Date())) { key = await createJwk(ctx, options); } - return key.alg ?? options?.jwks?.keyPairConfig?.alg ?? "EdDSA"; + const privateKeyEncryptionEnabled = + !options?.jwks?.disablePrivateKeyEncryption; + const privateWebKey = privateKeyEncryptionEnabled + ? await symmetricDecrypt({ + key: ctx.context.secretConfig, + data: JSON.parse(key.privateKey), + }).catch(() => { + throw new BetterAuthError( + "Failed to decrypt private key. Make sure the secret currently in use is the same as the one used to encrypt the private key. If you are using a different secret, either clean up your JWKS or disable private key encryption.", + ); + }) + : key.privateKey; + const alg = key.alg ?? options?.jwks?.keyPairConfig?.alg ?? "EdDSA"; + const privateKey = await importJWK(JSON.parse(privateWebKey), alg); + return { alg, kid: key.id, privateKey }; } export async function signJWT( @@ -89,6 +105,8 @@ export async function signJWT( config: { options?: JwtOptions | undefined; payload: JWTPayloadWithOptional; + /** Pre-resolved key from resolveSigningKey. Skips redundant DB lookup. */ + resolvedKey?: ResolvedSigningKey; }, ) { const { options } = config; @@ -136,31 +154,14 @@ export async function signJWT( return options.jwt.sign(jwtPayload); } - const adapter = getJwksAdapter(ctx.context.adapter, options); - let key = await adapter.getLatestKey(ctx); - if (!key || (key.expiresAt && key.expiresAt < new Date())) { - key = await createJwk(ctx, options); - } - const privateKeyEncryptionEnabled = - !options?.jwks?.disablePrivateKeyEncryption; - - const privateWebKey = privateKeyEncryptionEnabled - ? await symmetricDecrypt({ - key: ctx.context.secretConfig, - data: JSON.parse(key.privateKey), - }).catch(() => { - throw new BetterAuthError( - "Failed to decrypt private key. Make sure the secret currently in use is the same as the one used to encrypt the private key. If you are using a different secret, either clean up your JWKS or disable private key encryption.", - ); - }) - : key.privateKey; - const alg = key.alg ?? options?.jwks?.keyPairConfig?.alg ?? "EdDSA"; - const privateKey = await importJWK(JSON.parse(privateWebKey), alg); + // Use pre-resolved key if available, otherwise resolve from DB + const { alg, kid, privateKey } = + config.resolvedKey ?? (await resolveSigningKey(ctx, options))!; const jwt = new SignJWT(payload) .setProtectedHeader({ alg, - kid: key.id, + kid, }) .setExpirationTime(exp) .setIssuer(iss ?? defaultIss) diff --git a/packages/better-auth/src/plugins/jwt/types.ts b/packages/better-auth/src/plugins/jwt/types.ts index a89876ab92..c8918ae661 100644 --- a/packages/better-auth/src/plugins/jwt/types.ts +++ b/packages/better-auth/src/plugins/jwt/types.ts @@ -206,3 +206,16 @@ export interface Jwk { alg?: JWSAlgorithms | undefined; crv?: ("Ed25519" | "P-256" | "P-521") | undefined; } + +/** + * A fully resolved signing key ready for JWT signing. + * Produced by `resolveSigningKey`, consumed by `signJWT`. + * Separates key resolution from signing so callers can + * read the `alg` before constructing the JWT payload + * (required for OIDC hash claims like at_hash). + */ +export interface ResolvedSigningKey { + alg: string; + kid: string; + privateKey: CryptoKey | Uint8Array; +} diff --git a/packages/oauth-provider/src/token.test.ts b/packages/oauth-provider/src/token.test.ts index 137b96e5c6..c1962fa8ed 100644 --- a/packages/oauth-provider/src/token.test.ts +++ b/packages/oauth-provider/src/token.test.ts @@ -1490,7 +1490,6 @@ describe("oauth token - config", async () => { const refreshedTokens = await client.oauth2.token( { resource: validAudience, - // @ts-expect-error refresh token is sent refresh_token: tokens.data?.refresh_token, grant_type: "refresh_token", client_id: oauthClient?.client_id, @@ -1600,7 +1599,6 @@ describe("oauth token - config", async () => { // Refresh token const refreshedTokens = await client.oauth2.token( { - // @ts-expect-error refresh token is sent refresh_token: tokens.data?.refresh_token, grant_type: "refresh_token", client_id: oauthClient?.client_id, diff --git a/packages/oauth-provider/src/token.ts b/packages/oauth-provider/src/token.ts index 5c77efb86f..1b54e05760 100644 --- a/packages/oauth-provider/src/token.ts +++ b/packages/oauth-provider/src/token.ts @@ -2,14 +2,10 @@ import type { GenericEndpointContext } from "@better-auth/core"; import { APIError } from "better-auth/api"; import { generateRandomString } from "better-auth/crypto"; import { generateCodeChallenge } from "better-auth/oauth2"; -import { - resolveSigningAlgorithm, - signJWT, - toExpJWT, -} from "better-auth/plugins"; +import { resolveSigningKey, signJWT, toExpJWT } from "better-auth/plugins"; import type { Session, User } from "better-auth/types"; import type { JWTPayload } from "jose"; -import { base64url, SignJWT } from "jose"; +import { base64url, decodeProtectedHeader, SignJWT } from "jose"; import type { OAuthOptions, OAuthRefreshToken, @@ -185,13 +181,19 @@ async function createIdToken( ? undefined : getJwtPlugin(ctx.context).options; + // Resolve the signing key once: used for both at_hash and signing + const resolvedKey = + !opts.disableJwtPlugin && !jwtPluginOptions?.jwt?.sign + ? await resolveSigningKey(ctx, jwtPluginOptions) + : undefined; + + // For custom signer, alg is guaranteed set by JWT plugin init validation const signingAlg = opts.disableJwtPlugin ? "HS256" - : await resolveSigningAlgorithm(ctx, jwtPluginOptions); - const atHash = - accessToken && signingAlg - ? await computeOidcHash(accessToken, signingAlg) - : undefined; + : (resolvedKey?.alg ?? jwtPluginOptions?.jwks?.keyPairConfig?.alg!); + const atHash = accessToken + ? await computeOidcHash(accessToken, signingAlg) + : undefined; const payload: JWTPayload = { ...userClaims, @@ -214,8 +216,8 @@ async function createIdToken( return undefined; } - return opts.disableJwtPlugin - ? new SignJWT(payload) + const idToken = opts.disableJwtPlugin + ? await new SignJWT(payload) .setProtectedHeader({ alg: "HS256" }) .sign( new TextEncoder().encode( @@ -226,10 +228,28 @@ async function createIdToken( ), ), ) - : signJWT(ctx, { + : await signJWT(ctx, { options: jwtPluginOptions, payload, + resolvedKey: resolvedKey ?? undefined, }); + + // When using a custom jwt.sign callback, validate that the actual + // signing algorithm matches what was used for at_hash (OIDC Core §3.1.3.6) + if (idToken && atHash && jwtPluginOptions?.jwt?.sign) { + const header = decodeProtectedHeader(idToken); + if (header.alg !== signingAlg) { + throw new APIError("INTERNAL_SERVER_ERROR", { + error_description: + `ID token signed with "${header.alg}" but at_hash was computed ` + + `for "${signingAlg}". Ensure jwt.sign uses the algorithm ` + + `declared in keyPairConfig.alg.`, + error: "server_error", + }); + } + } + + return idToken; } /**