fix(oauth-provider): handle voluntary and essential ACR requests (#10790)

This commit is contained in:
Taesu
2026-08-13 19:10:36 +00:00
committed by GitHub
parent 8e85b12a62
commit a966815b13
10 changed files with 287 additions and 93 deletions
+5 -1
View File
@@ -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.
+1 -2
View File
@@ -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
@@ -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",
+152 -42
View File
@@ -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<string, unknown>;
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();
});
});
});
+45 -16
View File
@@ -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),
+59 -23
View File
@@ -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<string>,
) {
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(
+2 -2
View File
@@ -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
@@ -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<string, string>]>([
["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 = "";
+2 -2
View File
@@ -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,
+6 -2
View File
@@ -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(),
});
/**