diff --git a/.changeset/id-token-claim-authority.md b/.changeset/id-token-claim-authority.md index 90ccae3ac3..76769950ab 100644 --- a/.changeset/id-token-claim-authority.md +++ b/.changeset/id-token-claim-authority.md @@ -2,6 +2,10 @@ "@better-auth/oauth-provider": minor --- -ID tokens now report `acr: "0"` instead of the InCommon Bronze URI. The default OpenID discovery document no longer advertises `acr_values_supported`, since the provider does not support requestable ACR classes yet. +ID tokens now use `acr: "0"`, indicating that authentication did not meet +ISO/IEC 29115 level 1, and OpenID discovery advertises only `"0"`. Because +`acr_values` is voluntary, requests for other classes continue instead of +failing. Essential `claims.id_token.acr` requests in OpenID Connect flows still +fail when their required `value` or `values` cannot be met. `customIdTokenClaims`, extension ID-token claims, and per-issuance `idTokenClaims` can no longer set OIDC/JWT protocol claims such as issuer, subject, audience, token lifetime, nonce, session or hash binding, `auth_time`, `acr`, `amr`, or `azp`. Namespaced custom claims still appear in ID tokens. diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index 32b96e7816..c61febabc2 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -1721,8 +1721,7 @@ The metadata endpoint can be customized so that the publicized scopes and claims All scopes inside the advertisedMetadata section MUST be listed in `scopes` otherwise initialization will fail. -Better Auth advertises `acr_values_supported: ["0"]`, which is the unspecified authentication context. Custom ACR policies are not currently supported; authorization requests for other -`acr_values` are rejected with `invalid_request`. +Better Auth advertises `acr_values_supported: ["0"]`. In OIDC Core, `"0"` means the authentication did not meet ISO/IEC 29115 level 1. Custom ACR policies are not currently supported. Because `acr_values` is voluntary, requests for other classes continue and the ID token reports `acr: "0"`. In an OpenID Connect request, an essential `claims.id_token.acr` request fails when its `value` or `values` does not include `"0"`. #### Scopes diff --git a/packages/oauth-provider/src/authentication-context.ts b/packages/oauth-provider/src/authentication-context.ts index 1407ec2af7..58853ef520 100644 --- a/packages/oauth-provider/src/authentication-context.ts +++ b/packages/oauth-provider/src/authentication-context.ts @@ -1,12 +1,13 @@ import { logger } from "@better-auth/core/env"; /** - * RFC 6711 "unspecified" Authentication Context Class Reference. + * OIDC Core Authentication Context Class Reference for authentication that + * does not meet ISO/IEC 29115 level 1. * * Better Auth does not currently evaluate a stronger ACR policy, so discovery * and ID tokens must not claim an assurance profile such as InCommon bronze. */ -export const UNSPECIFIED_ACR = "0"; +export const LEVEL_0_ACR = "0"; const RESERVED_ID_TOKEN_CLAIMS = new Set([ "iss", diff --git a/packages/oauth-provider/src/authorize.test.ts b/packages/oauth-provider/src/authorize.test.ts index 95e0dfc973..3d5681da4e 100644 --- a/packages/oauth-provider/src/authorize.test.ts +++ b/packages/oauth-provider/src/authorize.test.ts @@ -3,7 +3,10 @@ import { createAuthEndpoint } from "@better-auth/core/api"; import { sessionMiddleware } from "better-auth/api"; import { createAuthClient } from "better-auth/client"; import { generateRandomString, makeSignature } from "better-auth/crypto"; -import { createAuthorizationURL } from "better-auth/oauth2"; +import { + createAuthorizationURL, + generateCodeChallenge, +} from "better-auth/oauth2"; import { jwt } from "better-auth/plugins/jwt"; import { getTestInstance } from "better-auth/test"; import { afterEach, beforeAll, describe, expect, it, vi } from "vitest"; @@ -499,7 +502,9 @@ describe("oauth authorize - max_age (OIDC Core 1.0 §3.1.2.1)", async () => { }); }); -describe("oauth authorize - acr_values (OIDC Core 1.0 §3.1.2.1)", async () => { +describe("oauth authorize - ACR requests", async () => { + type AcrClaimRequest = Record; + const authServerBaseUrl = "http://localhost:3000"; const rpBaseUrl = "http://localhost:5000"; const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ @@ -532,66 +537,171 @@ describe("oauth authorize - acr_values (OIDC Core 1.0 §3.1.2.1)", async () => { }); }); - function authorizeUrl(acrValues: string) { + function authorizeUrl( + acrValues: string | undefined, + acrClaim?: AcrClaimRequest, + scope = "openid", + codeChallenge = generateRandomString(43), + ) { if (!oauthClient?.client_id) throw new Error("beforeAll not run properly"); const url = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); url.searchParams.set("client_id", oauthClient.client_id); url.searchParams.set("redirect_uri", redirectUri); url.searchParams.set("response_type", "code"); - url.searchParams.set("scope", "openid"); + url.searchParams.set("scope", scope); url.searchParams.set("state", "acr-state"); - url.searchParams.set("code_challenge", generateRandomString(43)); + url.searchParams.set("code_challenge", codeChallenge); url.searchParams.set("code_challenge_method", "S256"); - url.searchParams.set("acr_values", acrValues); + if (acrValues !== undefined) { + url.searchParams.set("acr_values", acrValues); + } + if (acrClaim) { + url.searchParams.set( + "claims", + JSON.stringify({ id_token: { acr: acrClaim } }), + ); + } return url.toString(); } - async function redirectFor(acrValues: string) { + async function redirectFor( + acrValues: string | undefined, + acrClaim?: AcrClaimRequest, + scope?: string, + codeChallenge?: string, + ) { let location = ""; - await authenticatedClient.$fetch(authorizeUrl(acrValues), { - onError(context) { - location = context.response.headers.get("Location") || ""; + await authenticatedClient.$fetch( + authorizeUrl(acrValues, acrClaim, scope, codeChallenge), + { + onError(context) { + location = context.response.headers.get("Location") || ""; + }, }, - }); + ); return location; } - /** - * @see https://github.com/better-auth/better-auth/pull/10152 - */ - it("rejects unsupported acr_values instead of silently downgrading", async () => { - const location = await redirectFor("1"); - const errorRedirect = new URL(location); + describe("claims.id_token.acr (OIDC Core 1.0 §5.5.1.1)", () => { + it("accepts an unsupported voluntary claim", async () => { + const location = await redirectFor(undefined, { values: ["1"] }); + const callbackRedirect = new URL(location); - expect(errorRedirect.origin + errorRedirect.pathname).toBe(redirectUri); - expect(errorRedirect.searchParams.get("error")).toBe("invalid_request"); - expect(errorRedirect.searchParams.get("error_description")).toBe( - "unsupported acr_values", - ); - expect(errorRedirect.searchParams.get("state")).toBe("acr-state"); - expect(errorRedirect.searchParams.get("code")).toBeNull(); - }); + expect(callbackRedirect.searchParams.get("error")).toBeNull(); + expect(callbackRedirect.searchParams.get("code")).toEqual( + expect.any(String), + ); + }); - it("accepts the advertised unspecified acr value", async () => { - const location = await redirectFor("0"); - const callbackRedirect = new URL(location); + it.each<[string, AcrClaimRequest]>([ + ["value", { essential: true, value: "1" }], + ["values", { essential: true, values: ["1"] }], + ])("rejects an unsupported essential %s", async (_, request) => { + const location = await redirectFor(undefined, request); + const callbackRedirect = new URL(location); - expect(callbackRedirect.origin + callbackRedirect.pathname).toBe( - redirectUri, - ); - expect(callbackRedirect.searchParams.get("code")).toBeTruthy(); - expect(callbackRedirect.searchParams.get("error")).toBeNull(); - }); + expect(callbackRedirect.origin + callbackRedirect.pathname).toBe( + redirectUri, + ); + expect(callbackRedirect.searchParams.get("error")).toBe("access_denied"); + expect(callbackRedirect.searchParams.get("state")).toBe("acr-state"); + expect(callbackRedirect.searchParams.get("code")).toBeNull(); + }); - it("accepts multiple requested acr values when one is supported", async () => { - const location = await redirectFor("1 0"); - const callbackRedirect = new URL(location); + it.each<[string, AcrClaimRequest]>([ + ["value", { essential: true, value: 1 }], + ["values", { essential: true, values: "1" }], + ])("rejects a malformed essential %s", async (_, request) => { + const location = await redirectFor(undefined, request); + const callbackRedirect = new URL(location); - expect(callbackRedirect.origin + callbackRedirect.pathname).toBe( - redirectUri, - ); - expect(callbackRedirect.searchParams.get("code")).toBeTruthy(); - expect(callbackRedirect.searchParams.get("error")).toBeNull(); + expect(callbackRedirect.searchParams.get("error")).toBe( + "invalid_request", + ); + expect(callbackRedirect.searchParams.get("state")).toBe("acr-state"); + expect(callbackRedirect.searchParams.get("code")).toBeNull(); + }); + + it.each([ + { essential: true, value: "1" }, + { essential: true, value: 1 }, + ])("ignores claims in OAuth-only requests", async (request) => { + if (!oauthClient?.client_id || !oauthClient.client_secret) { + throw new Error("beforeAll not run properly"); + } + const codeVerifier = generateRandomString(43); + const codeChallenge = await generateCodeChallenge(codeVerifier); + const location = await redirectFor( + undefined, + request, + "profile", + codeChallenge, + ); + const callbackRedirect = new URL(location); + const code = callbackRedirect.searchParams.get("code"); + + expect(callbackRedirect.searchParams.get("error")).toBeNull(); + if (!code) throw new Error("authorization code not issued"); + + const tokenResponse = await authenticatedClient.$fetch<{ + access_token: string; + }>("/oauth2/token", { + method: "POST", + body: new URLSearchParams({ + grant_type: "authorization_code", + code, + code_verifier: codeVerifier, + redirect_uri: redirectUri, + }), + headers: { + authorization: `Basic ${Buffer.from( + `${oauthClient.client_id}:${oauthClient.client_secret}`, + ).toString("base64")}`, + }, + }); + + expect(tokenResponse.error).toBeNull(); + expect(tokenResponse.data?.access_token).toEqual(expect.any(String)); + }); + + it("accepts the current ACR in an essential values request", async () => { + const location = await redirectFor(undefined, { + essential: true, + values: ["1", "0"], + }); + const callbackRedirect = new URL(location); + + expect(callbackRedirect.searchParams.get("error")).toBeNull(); + expect(callbackRedirect.searchParams.get("code")).toEqual( + expect.any(String), + ); + }); + + it("accepts matching value and values selectors", async () => { + const location = await redirectFor(undefined, { + essential: true, + value: "0", + values: ["0"], + }); + const callbackRedirect = new URL(location); + + expect(callbackRedirect.searchParams.get("error")).toBeNull(); + expect(callbackRedirect.searchParams.get("code")).toEqual( + expect.any(String), + ); + }); + + it("rejects conflicting value and values selectors", async () => { + const location = await redirectFor(undefined, { + essential: true, + value: "0", + values: ["1"], + }); + const callbackRedirect = new URL(location); + + expect(callbackRedirect.searchParams.get("error")).toBe("access_denied"); + expect(callbackRedirect.searchParams.get("code")).toBeNull(); + }); }); }); diff --git a/packages/oauth-provider/src/authorize.ts b/packages/oauth-provider/src/authorize.ts index 142564b2e2..b88e113b5f 100644 --- a/packages/oauth-provider/src/authorize.ts +++ b/packages/oauth-provider/src/authorize.ts @@ -5,8 +5,12 @@ import { getSessionFromCtx } from "better-auth/api"; import { generateRandomString, makeSignature } from "better-auth/crypto"; import type { Verification } from "better-auth/db"; import { APIError } from "better-call"; -import { UNSPECIFIED_ACR } from "./authentication-context"; -import { getRequestedUserInfoClaims } from "./claims-request"; +import { LEVEL_0_ACR } from "./authentication-context"; +import { + canSatisfyEssentialAcrRequest, + getRequestedUserInfoClaims, + isValidOidcClaimsRequest, +} from "./claims-request"; import { oAuthState } from "./oauth"; import type { OAuthErrorCode, OAuthRedirectOnError } from "./oauth-endpoint"; import { mapIssuesToOAuthError } from "./oauth-endpoint"; @@ -63,6 +67,13 @@ function removeMaxAgeFromAuthorizationQuery( return queryWithoutMaxAge; } +function removeClaimsFromAuthorizationQuery( + query: OAuthAuthorizationQuery, +): OAuthAuthorizationQuery { + const { claims: _claims, ...queryWithoutClaims } = query; + return queryWithoutClaims; +} + /** * Formats an error url. Per OIDC Core 1.0 §5 / RFC 6749 §4.2.2.1, errors on * implicit and hybrid flows are delivered in the URL fragment, not the query. @@ -137,12 +148,6 @@ function getAuthorizationRequestParameters( return { ...parameters } as unknown as OAuthAuthorizationQuery; } -function isAcrValuesRequestSupported(acrValues: string | undefined): boolean { - if (acrValues === undefined) return true; - const requestedAcrValues = acrValues.split(" ").filter(Boolean); - return requestedAcrValues.includes(UNSPECIFIED_ACR); -} - export const handleRedirect = ( ctx: GenericEndpointContext, uri: string, @@ -443,13 +448,6 @@ export async function authorizeEndpoint( } query = parsedQuery.data as OAuthAuthorizationQuery; ctx.query = query; - if (!isAcrValuesRequestSupported(query.acr_values)) { - return authorizeRedirectOnError(opts)({ - error: "invalid_request", - error_description: "unsupported acr_values", - ctx, - }); - } await oAuthState.set({ query: serializeAuthorizationQuery(query).toString(), }); @@ -525,7 +523,6 @@ export async function authorizeEndpoint( getErrorURL(ctx, "invalid_redirect", "invalid redirect uri"), ); } - // Check for invalid scopes if requested from query let requestedScopes = query.scope?.split(" ").filter((s) => s); if (requestedScopes) { @@ -551,6 +548,38 @@ export async function authorizeEndpoint( requestedScopes = client.scopes ?? opts.scopes ?? []; query.scope = requestedScopes.join(" "); } + const openidRequested = requestedScopes.includes("openid"); + if (openidRequested && !isValidOidcClaimsRequest(query.claims)) { + return handleRedirect( + ctx, + formatErrorURL( + query.redirect_uri, + "invalid_request", + "claims must be a valid Claims request object", + query.state, + getIssuer(ctx, opts), + ), + ); + } + if (!openidRequested && query.claims !== undefined) { + query = removeClaimsFromAuthorizationQuery(query); + ctx.query = query; + } + if ( + openidRequested && + !canSatisfyEssentialAcrRequest(query.claims, LEVEL_0_ACR) + ) { + return handleRedirect( + ctx, + formatErrorURL( + query.redirect_uri, + "access_denied", + "essential acr requirement cannot be met", + query.state, + getIssuer(ctx, opts), + ), + ); + } const requestedUserInfoClaims = getRequestedUserInfoClaims( query.claims, getSupportedClaims(opts), diff --git a/packages/oauth-provider/src/claims-request.ts b/packages/oauth-provider/src/claims-request.ts index 0e870bf3cf..e516141fc4 100644 --- a/packages/oauth-provider/src/claims-request.ts +++ b/packages/oauth-provider/src/claims-request.ts @@ -1,16 +1,25 @@ import * as z from "zod"; -const claimRequestMemberSchema = z.union([ - z.null(), - z.record(z.string(), z.unknown()), -]); +const claimRequestMemberSchema = z.record(z.string(), z.unknown()).nullable(); -const claimsRequestObjectSchema = z - .object({ - userinfo: z.record(z.string(), claimRequestMemberSchema).optional(), - id_token: z.record(z.string(), claimRequestMemberSchema).optional(), +const acrClaimRequestMemberSchema = z + .looseObject({ + essential: z.boolean().optional(), + value: z.string().optional(), + values: z.array(z.string()).optional(), }) - .passthrough(); + .nullable(); + +const idTokenClaimsRequestSchema = z + .object({ + acr: acrClaimRequestMemberSchema.optional(), + }) + .catchall(claimRequestMemberSchema); + +const oidcClaimsRequestObjectSchema = z.looseObject({ + userinfo: z.record(z.string(), claimRequestMemberSchema).optional(), + id_token: idTokenClaimsRequestSchema.optional(), +}); function parseClaimsRequestValue(value: unknown) { if (typeof value === "string") { @@ -23,28 +32,38 @@ function parseClaimsRequestValue(value: unknown) { return value; } -function parseClaimsRequestObject(value: unknown) { +function parseOidcClaimsRequestObject(value: unknown) { const parsed = parseClaimsRequestValue(value); - const result = claimsRequestObjectSchema.safeParse(parsed); + const result = oidcClaimsRequestObjectSchema.safeParse(parsed); return result.success ? result.data : undefined; } -export const claimsRequestParameterSchema = z - .union([z.string(), z.record(z.string(), z.unknown())]) - .superRefine((value, ctx) => { - if (!parseClaimsRequestObject(value)) { - ctx.addIssue({ - code: "custom", - message: "claims must be a JSON object", - }); - } - }); +const claimsRequestParameterValueSchema = z.union([ + z.string(), + z.record(z.string(), z.unknown()), +]); + +export const claimsRequestInputSchema = claimsRequestParameterValueSchema; + +export const claimsRequestParameterSchema = + claimsRequestParameterValueSchema.refine( + (value) => parseOidcClaimsRequestObject(value) !== undefined, + { + error: "claims must be a valid Claims request object", + }, + ); + +export function isValidOidcClaimsRequest(value: unknown) { + return ( + value === undefined || parseOidcClaimsRequestObject(value) !== undefined + ); +} export function getRequestedUserInfoClaims( value: unknown, supportedClaims?: Iterable, ) { - const claimsRequest = parseClaimsRequestObject(value); + const claimsRequest = parseOidcClaimsRequestObject(value); const userInfoClaims = claimsRequest?.userinfo; if (!userInfoClaims) return []; // `Object.keys` over a parsed object is already unique; no duplicate filtering needed. @@ -54,11 +73,28 @@ export function getRequestedUserInfoClaims( return names.filter((name) => allowed.has(name)); } +export function canSatisfyEssentialAcrRequest( + value: unknown, + currentAcr: string, +) { + const claimsRequest = parseOidcClaimsRequestObject(value); + if (!claimsRequest) return value === undefined; + + const acrRequest = claimsRequest.id_token?.acr; + if (!acrRequest || acrRequest.essential !== true) return true; + + const valueMatches = + acrRequest.value === undefined || acrRequest.value === currentAcr; + const valuesMatch = + acrRequest.values === undefined || acrRequest.values.includes(currentAcr); + return valueMatches && valuesMatch; +} + export function filterClaimsRequestUserInfoClaims( value: unknown, allowedUserInfoClaims: string[], ) { - const claimsRequest = parseClaimsRequestObject(value); + const claimsRequest = parseOidcClaimsRequestObject(value); if (!claimsRequest) return undefined; const allowedClaimSet = new Set(allowedUserInfoClaims); const userInfoClaims = Object.fromEntries( diff --git a/packages/oauth-provider/src/metadata.ts b/packages/oauth-provider/src/metadata.ts index 2cfd0621f1..0d364bc154 100644 --- a/packages/oauth-provider/src/metadata.ts +++ b/packages/oauth-provider/src/metadata.ts @@ -4,7 +4,7 @@ import { PRIVATE_KEY_JWT_SIGNING_ALGORITHMS, } from "@better-auth/core/oauth2"; import type { JWSAlgorithms, JwtOptions } from "better-auth/plugins"; -import { UNSPECIFIED_ACR } from "./authentication-context"; +import { LEVEL_0_ACR } from "./authentication-context"; import { validateIssuerUrl } from "./authorize"; import { applyOAuthProviderMetadataExtensions, @@ -178,7 +178,7 @@ export function oidcServerMetadata( subject_types_supported: opts.pairwiseSecret ? ["public", "pairwise"] : ["public"], - acr_values_supported: [UNSPECIFIED_ACR], + acr_values_supported: [LEVEL_0_ACR], id_token_signing_alg_values_supported: (() => { if (opts.disableJwtPlugin) return ["HS256" as const]; // Advertise every algorithm the plugin can sign with: the primary diff --git a/packages/oauth-provider/src/oidc-rs256-profile.test.ts b/packages/oauth-provider/src/oidc-rs256-profile.test.ts index 01b4a3d3fa..cf2bb17e58 100644 --- a/packages/oauth-provider/src/oidc-rs256-profile.test.ts +++ b/packages/oauth-provider/src/oidc-rs256-profile.test.ts @@ -99,7 +99,17 @@ describe("OpenID Connect RS256 provider profile", async () => { ); }); - it("issues an RS256 ID token through the authorization-code flow", async () => { + it.each<[string, Record]>([ + ["acr_values", { acr_values: "1" }], + [ + "voluntary ACR claim", + { + claims: JSON.stringify({ + id_token: { acr: { values: ["1"] } }, + }), + }, + ], + ])("issues an RS256 ID token with the current ACR for unsupported %s", async (_, additionalParams) => { if (!oauthClient?.client_id || !oauthClient.client_secret) { throw new Error("beforeAll not run properly"); } @@ -118,6 +128,7 @@ describe("OpenID Connect RS256 provider profile", async () => { scopes, codeVerifier, nonce, + additionalParams, }); let callbackRedirectUrl = ""; diff --git a/packages/oauth-provider/src/token.ts b/packages/oauth-provider/src/token.ts index 69acf3040f..301cd714d4 100644 --- a/packages/oauth-provider/src/token.ts +++ b/packages/oauth-provider/src/token.ts @@ -17,8 +17,8 @@ import type { Session, User } from "better-auth/types"; import type { JWTPayload } from "jose"; import { base64url, decodeProtectedHeader, SignJWT } from "jose"; import { + LEVEL_0_ACR, stripReservedIdTokenClaims, - UNSPECIFIED_ACR, } from "./authentication-context"; import { resolveAccessTokenClaims } from "./claims"; import { getRequestedUserInfoClaims } from "./claims-request"; @@ -374,7 +374,7 @@ async function createIdToken( const payload: JWTPayload = { ...ID_TOKEN_SCOPE_CLAIM_GUARDS, auth_time: authTimeSec, - acr: UNSPECIFIED_ACR, + acr: LEVEL_0_ACR, ...customClaims, at_hash: atHash, iss: jwtPluginOptions?.jwt?.issuer ?? ctx.context.baseURL, diff --git a/packages/oauth-provider/src/types/zod.ts b/packages/oauth-provider/src/types/zod.ts index 9658a1e927..6a22d3977f 100644 --- a/packages/oauth-provider/src/types/zod.ts +++ b/packages/oauth-provider/src/types/zod.ts @@ -1,6 +1,9 @@ import { SafeUrlSchema } from "@better-auth/core/utils/redirect-uri"; import * as z from "zod"; -import { claimsRequestParameterSchema } from "../claims-request"; +import { + claimsRequestInputSchema, + claimsRequestParameterSchema, +} from "../claims-request"; /** * Re-exported from `@better-auth/core` so every OAuth provider plugin shares one @@ -136,7 +139,7 @@ export const authorizationQuerySchema = z .pipe(z.enum(["S256"])) .optional(), nonce: z.string().optional(), - claims: claimsRequestParameterSchema.optional(), + claims: claimsRequestInputSchema.optional(), dpop_jkt: dpopJktSchema.optional(), resource: z .union([ResourceUriSchema, z.array(ResourceUriSchema).min(1)]) @@ -152,6 +155,7 @@ export const authorizationQuerySchema = z // "malformed verification value". const storedAuthorizationQuerySchema = authorizationQuerySchema.extend({ redirect_uri: SafeUrlSchema.optional(), + claims: claimsRequestParameterSchema.optional(), }); /**