diff --git a/.changeset/id-token-claim-authority.md b/.changeset/id-token-claim-authority.md new file mode 100644 index 0000000000..90ccae3ac3 --- /dev/null +++ b/.changeset/id-token-claim-authority.md @@ -0,0 +1,7 @@ +--- +"@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. + +`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 46816e502e..e3dbab0c41 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -1178,9 +1178,11 @@ oauthProvider({ ### Claims -Internally, we support the following claims are supported: \["sub", "iss", "aud", "exp", "iat", "sid", "scope", "azp"]. +Internally supported claims include \["sub", "iss", "aud", "exp", "iat", "sid", "scope", "azp"]. -Id token and user info claims should be namespaced when possible to avoid potential future conflicts. +ID token and user info claims should be namespaced when possible to avoid potential future conflicts. + +`customIdTokenClaims` is additive for protocol-owned ID token claims. Reserved names such as `iss`, `sub`, `aud`, token lifetime claims, `nonce`, `sid`, hash claims, `auth_time`, `acr`, `amr`, and `azp` are stripped at issuance with a warning log. Use namespaced claim names such as `https://example.com/org` for application-specific data. Claims added inside `customIdTokenClaims` and `customUserInfoClaims` should be added to the `advertisedMetadata.claims_supported` so clients can validate that claim received. In the following example, it would be the base claims plus `locale` and `https://example.com/org`. @@ -1475,13 +1477,13 @@ authenticate: async ({ ctx, opts, assertion, expectedAudience }) => { #### Claim precedence -Three claim surfaces resolve contributions in a fixed order. Across all three, third-party extension claims are strictly additive, while the operator's own first-party callbacks may override identity claims; subject identity is always pinned by the provider. +Three claim surfaces resolve contributions in a fixed order. Across all three, third-party extension claims are strictly additive, while the operator's own first-party callbacks may override profile-style identity claims; protocol-owned identity, lifetime, binding, and authentication-context claims are always pinned or reserved by the provider. -| Token | Order (lowest to highest authority) | Always provider-owned | -| ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------- | -| Access token | extension `claims.accessToken` \< per-issuance `accessTokenClaims` \< `customAccessTokenClaims` \< per-resource `customClaims` | reserved RFC 9068 names (`iss`, `sub`, `aud`, `exp`, `iat`, `jti`, `client_id`, `scope`, `auth_time`, `acr`, `amr`), stripped before signing | -| ID token | base identity claims \< `auth_time`/`acr` \< `customIdTokenClaims`; extension and per-issuance `idTokenClaims` are additive (fill absent keys only) | `at_hash`, `iss`, `sub`, `aud`, `nonce`, `iat`, `exp`, `sid` | -| UserInfo | base identity claims \< extension `claims.userInfo` (additive only) \< `customUserInfoClaims` | `sub` (re-pinned last) | +| Token | Order (lowest to highest authority) | Pinned or reserved by provider | +| ------------ | ------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Access token | extension `claims.accessToken` \< per-issuance `accessTokenClaims` \< `customAccessTokenClaims` \< per-resource `customClaims` | reserved RFC 9068 names (`iss`, `sub`, `aud`, `exp`, `iat`, `jti`, `client_id`, `scope`, `auth_time`, `acr`, `amr`), stripped before signing | +| ID token | base identity claims \< `customIdTokenClaims`; extension and per-issuance `idTokenClaims` are reserved-filtered and additive | reserved OIDC/JWT names (`iss`, `sub`, `aud`, `exp`, `nbf`, `iat`, `jti`, `nonce`, `sid`, `at_hash`, `c_hash`, `s_hash`, `auth_time`, `acr`, `amr`, `azp`) | +| UserInfo | base identity claims \< extension `claims.userInfo` (additive only) \< `customUserInfoClaims` | `sub` (re-pinned last) | Per-issuance `accessTokenClaims` are JWT-only: an opaque access token persists no per-issuance claims, so they do not reappear at introspection. A claim that must be visible at opaque-token introspection belongs in a grant-type-stable `claims.accessToken` contributor, which the introspection path re-derives. diff --git a/packages/oauth-provider/src/authentication-context.ts b/packages/oauth-provider/src/authentication-context.ts new file mode 100644 index 0000000000..1407ec2af7 --- /dev/null +++ b/packages/oauth-provider/src/authentication-context.ts @@ -0,0 +1,59 @@ +import { logger } from "@better-auth/core/env"; + +/** + * RFC 6711 "unspecified" Authentication Context Class Reference. + * + * 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"; + +const RESERVED_ID_TOKEN_CLAIMS = new Set([ + "iss", + "sub", + "aud", + "exp", + "nbf", + "iat", + "jti", + "auth_time", + "nonce", + "acr", + "amr", + "azp", + "sid", + "at_hash", + "c_hash", + "s_hash", +]); + +/** + * Removes provider-owned ID token claim names from custom claims. + * + * `customIdTokenClaims` is an extension point for additional claims. It must not + * replace the issuer, subject, audience, lifetime, nonce, authorized party, + * token-binding hashes, session binding, or authentication context that the + * provider owns. + */ +export function stripReservedIdTokenClaims( + claims: Record | null | undefined, +): Record { + if (!claims) return {}; + const stripped: string[] = []; + const safeClaims = Object.create(null) as Record; + for (const [key, value] of Object.entries(claims)) { + if (RESERVED_ID_TOKEN_CLAIMS.has(key)) { + stripped.push(key); + continue; + } + safeClaims[key] = value; + } + if (stripped.length > 0) { + logger.warn( + `oauth-provider: stripped reserved id-token claim name(s): ${stripped.join( + ", ", + )}. The AS owns these claim values.`, + ); + } + return safeClaims; +} diff --git a/packages/oauth-provider/src/extensions.test.ts b/packages/oauth-provider/src/extensions.test.ts index 3d1c913519..69b0baef47 100644 --- a/packages/oauth-provider/src/extensions.test.ts +++ b/packages/oauth-provider/src/extensions.test.ts @@ -73,6 +73,13 @@ describe("oauth-provider extensions", async () => { idTokenClaims: { grant_id_claim: "grant-id", sub: "malicious-sub", + auth_time: 0, + amr: ["malicious-grant"], + azp: "malicious-authorized-party", + nbf: 0, + jti: "malicious-jti", + c_hash: "malicious-code-hash", + s_hash: "malicious-state-hash", // Collides with the extension idToken contributor below; // the per-issuance value must win. order_probe: "grant", @@ -183,6 +190,8 @@ describe("oauth-provider extensions", async () => { extension_id_claim: "extension-id", sub: "malicious-extension-sub", email: "evil@malicious.example", + azp: "malicious-extension-authorized-party", + jti: "malicious-extension-jti", order_probe: "extension", }), userInfo: () => ({ @@ -537,8 +546,15 @@ describe("oauth-provider extensions", async () => { expect(idTokenPayload.extension_id_claim).toBe("extension-id"); expect(idTokenPayload.grant_id_claim).toBe("grant-id"); expect(idTokenPayload.sub).toBe(user.id); + expect(idTokenPayload.auth_time).toBeUndefined(); + expect(idTokenPayload.amr).toBeUndefined(); + expect(idTokenPayload.azp).toBeUndefined(); + expect(idTokenPayload.nbf).toBeUndefined(); + expect(idTokenPayload.jti).toBeUndefined(); + expect(idTokenPayload.c_hash).toBeUndefined(); + expect(idTokenPayload.s_hash).toBeUndefined(); // Extension id-token claims are additive: they cannot replace the user's - // identity claims. + // identity claims or fill reserved OIDC/JWT claim names. expect(idTokenPayload.email).toBe(user.email); expect(idTokenPayload.email).not.toBe("evil@malicious.example"); // Per-issuance idTokenClaims win a collision against an extension idToken diff --git a/packages/oauth-provider/src/metadata.test.ts b/packages/oauth-provider/src/metadata.test.ts index cf7d62616c..aa919bd0c3 100644 --- a/packages/oauth-provider/src/metadata.test.ts +++ b/packages/oauth-provider/src/metadata.test.ts @@ -122,7 +122,6 @@ describe("oauth metadata", async () => { subject_types_supported: ["public"], id_token_signing_alg_values_supported: ["EdDSA"], end_session_endpoint: `${baseURL}/oauth2/end-session`, - acr_values_supported: ["urn:mace:incommon:iap:bronze"], prompt_values_supported: [ "login", "consent", diff --git a/packages/oauth-provider/src/metadata.ts b/packages/oauth-provider/src/metadata.ts index f2e544cbed..de625b7fc3 100644 --- a/packages/oauth-provider/src/metadata.ts +++ b/packages/oauth-provider/src/metadata.ts @@ -188,7 +188,6 @@ export function oidcServerMetadata( return Array.from(new Set([primary, ...extras])); })(), end_session_endpoint: `${baseURL}/oauth2/end-session`, - acr_values_supported: ["urn:mace:incommon:iap:bronze"], prompt_values_supported: [ "login", "consent", diff --git a/packages/oauth-provider/src/token.test.ts b/packages/oauth-provider/src/token.test.ts index 879b04f275..3a92296491 100644 --- a/packages/oauth-provider/src/token.test.ts +++ b/packages/oauth-provider/src/token.test.ts @@ -2216,18 +2216,25 @@ describe("id token claim override security", async () => { customIdTokenClaims: () => ({ acr: "silver", auth_time: 0, + amr: ["evil"], sub: "evil", iss: "https://evil.com", aud: "evil-client", nonce: "evil-nonce", iat: 0, + nbf: 0, exp: 0, + jti: "evil-jti", + azp: "evil-authorized-party", sid: "evil-sid", + at_hash: "evil-at-hash", + c_hash: "evil-code-hash", + s_hash: "evil-state-hash", + "https://example.com/custom": "safe-custom-claim", }), }), ], }); - const { headers } = await signInWithTestUser(); const client = createAuthClient({ plugins: [oauthProviderClient()], @@ -2323,15 +2330,16 @@ describe("id token claim override security", async () => { return decodeJwt(tokens.data!.id_token!); } - it("customIdTokenClaims can override acr and auth_time", async ({ + it("customIdTokenClaims cannot override authentication context claims", async ({ expect, }) => { const claims = await getIdTokenClaims(); - expect(claims.acr).toBe("silver"); - expect(claims.auth_time).toBe(0); + expect(claims.acr).toBe("0"); + expect(claims.auth_time).not.toBe(0); + expect(claims.amr).toBeUndefined(); }); - it("customIdTokenClaims cannot override pinned security claims", async ({ + it("customIdTokenClaims cannot override provider-owned id token claims", async ({ expect, }) => { const claims = await getIdTokenClaims(); @@ -2340,8 +2348,15 @@ describe("id token claim override security", async () => { expect(claims.aud).not.toBe("evil-client"); expect(claims.nonce).not.toBe("evil-nonce"); expect(claims.iat).not.toBe(0); + expect(claims.nbf).not.toBe(0); expect(claims.exp).not.toBe(0); + expect(claims.jti).not.toBe("evil-jti"); + expect(claims.azp).not.toBe("evil-authorized-party"); expect(claims.sid).not.toBe("evil-sid"); + expect(claims.at_hash).not.toBe("evil-at-hash"); + expect(claims.c_hash).not.toBe("evil-code-hash"); + expect(claims.s_hash).not.toBe("evil-state-hash"); + expect(claims["https://example.com/custom"]).toBe("safe-custom-claim"); }); }); diff --git a/packages/oauth-provider/src/token.ts b/packages/oauth-provider/src/token.ts index fbff328b4e..c26f5584c0 100644 --- a/packages/oauth-provider/src/token.ts +++ b/packages/oauth-provider/src/token.ts @@ -12,6 +12,10 @@ import { resolveSigningKey, signJWT, toExpJWT } from "better-auth/plugins"; import type { Session, User } from "better-auth/types"; import type { JWTPayload } from "jose"; import { base64url, decodeProtectedHeader, SignJWT } from "jose"; +import { + stripReservedIdTokenClaims, + UNSPECIFIED_ACR, +} from "./authentication-context"; import { resolveAccessTokenClaims } from "./claims"; import { getDpopProofJwt, getEndpointUrl } from "./dpop"; import { @@ -320,18 +324,16 @@ async function createIdToken( const resolvedSub = await resolveSubjectIdentifier(user.id, client, opts); const authTimeSec = authTime != null ? Math.floor(authTime.getTime() / 1000) : undefined; - // TODO: this should be validated against the login process - // - bronze : password only - // - silver : mfa - const acr = "urn:mace:incommon:iap:bronze"; - const customClaims = opts.customIdTokenClaims - ? await opts.customIdTokenClaims({ - user, - scopes, - metadata: parseClientMetadata(client.metadata), - }) - : {}; + const customClaims = stripReservedIdTokenClaims( + opts.customIdTokenClaims + ? await opts.customIdTokenClaims({ + user, + scopes, + metadata: parseClientMetadata(client.metadata), + }) + : undefined, + ); const jwtPluginOptions = opts.disableJwtPlugin ? undefined @@ -357,7 +359,7 @@ async function createIdToken( const payload: JWTPayload = { ...userClaims, auth_time: authTimeSec, - acr, + acr: UNSPECIFIED_ACR, ...customClaims, at_hash: atHash, iss: jwtPluginOptions?.jwt?.issuer ?? ctx.context.baseURL, @@ -368,11 +370,11 @@ async function createIdToken( exp, sid: emitSid ? sessionId : undefined, }; - // Extension and grant claims are additive: they only fill keys the provider - // does not already own, so a contributor cannot replace an identity claim - // (email/name/...), the authentication context, or an AS-owned claim. - // First-party `customIdTokenClaims` already overrode auth_time/acr above. - Object.assign(payload, pickClaims(extraClaims, payload)); + // Extension and grant claims are additive: reserved OIDC/JWT names are + // stripped, and the remaining claims only fill keys the provider does not + // already own. + const additiveClaims = stripReservedIdTokenClaims(extraClaims); + Object.assign(payload, pickClaims(additiveClaims, payload)); // Public clients without a client secret cannot receive an idToken as it can't be verified // Confidential clients would still receive an idToken signed by the clientSecret diff --git a/packages/oauth-provider/src/types/index.ts b/packages/oauth-provider/src/types/index.ts index 312e21bb14..50d11ed7cb 100644 --- a/packages/oauth-provider/src/types/index.ts +++ b/packages/oauth-provider/src/types/index.ts @@ -1011,6 +1011,11 @@ export interface OAuthOptions< * example.com should namespace an organization at * https://example.com/organization. * + * Reserved ID token claim names (`iss`, `sub`, `aud`, `exp`, `nbf`, `iat`, + * `jti`, `nonce`, `sid`, `at_hash`, `c_hash`, `s_hash`, `auth_time`, `acr`, + * `amr`, `azp`) are stripped at issuance with a warning log. The + * authorization server owns these values. + * * @param info - context that may be useful when creating custom claims */ customIdTokenClaims?: (info: { diff --git a/packages/oauth-provider/src/types/oauth.ts b/packages/oauth-provider/src/types/oauth.ts index 7b4c696e17..c11266c2ee 100644 --- a/packages/oauth-provider/src/types/oauth.ts +++ b/packages/oauth-provider/src/types/oauth.ts @@ -258,19 +258,16 @@ export interface OIDCMetadata extends AuthServerMetadata { */ userinfo_endpoint: string; /** - * acr_values supported. + * Authentication Context Class Reference values supported. * - * - `urn:mace:incommon:iap:silver`: Silver level of assurance - * - `urn:mace:incommon:iap:bronze`: Bronze level of assurance - * - * Determination of acr_value is considered bronze by default. - * Silver level determination coming soon. + * Better Auth does not advertise this field by default because it does not + * currently evaluate requested Authentication Context Class References. * * @default - * ["urn:mace:incommon:iap:bronze"] - * @see https://incommon.org/federation/attributes.html + * undefined + * @see https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata */ - acr_values_supported: string[]; + acr_values_supported?: string[]; /** * Supported subject types. *