diff --git a/.changeset/generic-oauth-verify-id-token.md b/.changeset/generic-oauth-verify-id-token.md index 9c1f40ce40..f26056334a 100644 --- a/.changeset/generic-oauth-verify-id-token.md +++ b/.changeset/generic-oauth-verify-id-token.md @@ -2,7 +2,9 @@ "better-auth": minor --- -genericOAuth providers configured with a `discoveryUrl` now verify the provider's `id_token` against its published JWKS (signature, issuer, audience, and advertised algorithms). A sign-in whose `id_token` fails verification is rejected. +genericOAuth providers configured with a `discoveryUrl` now verify the provider's `id_token` against its published JWKS (signature, issuer, audience, and advertised algorithms) and bind it to the authorization request with a server-generated OIDC `nonce`. A sign-in whose `id_token` fails verification, or does not echo the expected `nonce`, is rejected. + +Set `disableIdTokenNonceBinding: true` on a provider that does not return the `nonce` claim in the authorization-code flow. These providers also accept client-submitted id_token sign-in through `signIn.social({ idToken })`, which previously returned `ID_TOKEN_NOT_SUPPORTED`. diff --git a/.changeset/oauth-additional-params.md b/.changeset/oauth-additional-params.md index 45267a55f7..d16f803910 100644 --- a/.changeset/oauth-additional-params.md +++ b/.changeset/oauth-additional-params.md @@ -18,7 +18,7 @@ Unified escape hatch for customizing the provider authorization URL on a per-req ### Security -- The shared `createAuthorizationURL` helper silently drops any caller-supplied key in `RESERVED_AUTHORIZATION_PARAMS` (`state`, `client_id`, `redirect_uri`, `response_type`, `code_challenge`, `code_challenge_method`, `scope`). The request-body Zod schema rejects the same keys with 400, so misuse is visible at the edge rather than silently overriding security-critical parameters. +- The shared `createAuthorizationURL` helper silently drops any caller-supplied key in `RESERVED_AUTHORIZATION_PARAMS` (`state`, `client_id`, `redirect_uri`, `response_type`, `code_challenge`, `code_challenge_method`, `nonce`, `scope`). The request-body Zod schema rejects the same keys with 400, so misuse is visible at the edge rather than silently overriding security-critical parameters. `nonce` is reserved so a caller cannot replace the OIDC nonce Better Auth generates when binding a discovery provider's `id_token` to the authorization request. - Providers that use non-standard client identifiers (`wechat` → `appid`, `tiktok` → `client_key`) additionally filter those keys so a caller cannot swap the configured OAuth app. - Provider protocol constants that are required for the integration to function (`atlassian` → `audience`, `notion` → `owner`) are merged last so caller-supplied `additionalParams` cannot override them. Configured defaults that represent operator intent (e.g. Google `include_granted_scopes`, Cognito `identityProvider`) remain caller-overridable. - `signIn.sso` rejects `additionalParams` with 400 when the resolved provider is SAML; the SAML AuthnRequest is signed and cannot carry caller-supplied query parameters, so silently dropping them would mislead integrators. diff --git a/docs/content/docs/plugins/generic-oauth.mdx b/docs/content/docs/plugins/generic-oauth.mdx index 6a66401688..d1a218d134 100644 --- a/docs/content/docs/plugins/generic-oauth.mdx +++ b/docs/content/docs/plugins/generic-oauth.mdx @@ -297,7 +297,9 @@ interface GenericOAuthConfig { **providerId**: A unique string to identify the OAuth provider configuration. -**discoveryUrl**: (Optional) URL to fetch the provider's OAuth 2.0/OIDC configuration. If provided, endpoints like `authorizationUrl`, `tokenUrl`, and `userInfoUrl` will be auto-discovered at server startup. When the discovery document publishes a `jwks_uri`, `id_token`s returned by the provider are verified against it (signature, issuer, audience, and advertised signing algorithms) before their claims are used; a token that fails verification rejects the sign-in. +**discoveryUrl**: (Optional) URL to fetch the provider's OAuth 2.0/OIDC configuration. If provided, endpoints like `authorizationUrl`, `tokenUrl`, and `userInfoUrl` will be auto-discovered at server startup. When the discovery document publishes a `jwks_uri`, `id_token`s returned by the provider are verified against it (signature, issuer, audience, and advertised signing algorithms) before their claims are used; a token that fails verification rejects the sign-in. Discovery providers also bind the `id_token` to the authorization request with a server-generated OIDC `nonce` and reject a callback whose `id_token` does not echo it. + +**disableIdTokenNonceBinding**: (Optional) Turn off OIDC `nonce` binding for a discovery provider's `id_token`. Binding is on by default and rejects an `id_token` that does not echo the server-generated `nonce` (OIDC Core 1.0 §3.1.3.7). Set this to `true` only for providers that do not return the `nonce` claim in the authorization-code flow; it removes `id_token` replay protection for the provider. **authorizationUrl**: (Optional) The OAuth provider's authorization endpoint. Not required if using `discoveryUrl`. @@ -333,7 +335,7 @@ interface GenericOAuthConfig { **mapProfileToUser**: (Optional) A function to map the provider's user profile to your app's user object. Useful for custom field mapping or transformations. -**authorizationUrlParams**: (Optional) Additional query parameters to add to the authorization URL. These can override default parameters. +**authorizationUrlParams**: (Optional) Additional query parameters to add to the authorization URL. Reserved OAuth keys (`state`, `client_id`, `redirect_uri`, `response_type`, `code_challenge`, `code_challenge_method`, `nonce`, `scope`) are ignored so they cannot replace the values Better Auth manages for the flow; any other key overrides the default. **tokenUrlParams**: (Optional) Additional query parameters to add to the token URL. Parameters already set by Better Auth are preserved. Configure token endpoint client authentication with `clientId`, `clientSecret`, and `tokenEndpointAuth`. diff --git a/packages/better-auth/src/api/routes/account.test.ts b/packages/better-auth/src/api/routes/account.test.ts index c2f87f0599..f55b79857a 100644 --- a/packages/better-auth/src/api/routes/account.test.ts +++ b/packages/better-auth/src/api/routes/account.test.ts @@ -630,6 +630,35 @@ describe("account", async () => { }); }); + it("should pass idTokenNonce to providers that require redirect nonce binding", async () => { + const googleProvider = ctx.socialProviders.find((v) => v.id === "google")!; + const previousRequiresIdTokenNonce = googleProvider.requiresIdTokenNonce; + googleProvider.requiresIdTokenNonce = true; + const createAuthorizationURLSpy = vi.spyOn( + googleProvider, + "createAuthorizationURL", + ); + + try { + const { runWithUser: runWithClient2 } = await signInWithTestUser(); + await runWithClient2(async () => { + const linkAccountRes = await client.linkSocial({ + provider: "google", + callbackURL: "/callback", + }); + expect(linkAccountRes.error).toBeNull(); + expect(createAuthorizationURLSpy).toHaveBeenCalledWith( + expect.objectContaining({ + idTokenNonce: expect.any(String), + }), + ); + }); + } finally { + googleProvider.requiresIdTokenNonce = previousRequiresIdTokenNonce; + createAuthorizationURLSpy.mockRestore(); + } + }); + it("should link second account from the same provider", async () => { const { runWithUser: runWithClient2 } = await signInWithTestUser(); await runWithClient2(async (headers) => { diff --git a/packages/better-auth/src/api/routes/account.ts b/packages/better-auth/src/api/routes/account.ts index 7e846acbaf..3ca0622bd8 100644 --- a/packages/better-auth/src/api/routes/account.ts +++ b/packages/better-auth/src/api/routes/account.ts @@ -20,7 +20,7 @@ import { parseAccountOutput } from "../../db/schema"; import { missingEmailLogMessage } from "../../oauth2/errors"; import { persistOAuthAccount } from "../../oauth2/persist-account"; import { applyUpdateUserInfoOnLink } from "../../oauth2/resolve-account"; -import { generateState } from "../../oauth2/state"; +import { generateIdTokenNonce, generateState } from "../../oauth2/state"; import { decryptOAuthToken } from "../../oauth2/token-encryption"; import { freshSessionMiddleware, @@ -195,7 +195,7 @@ export const linkSocialAccount = createAuthEndpoint( /** * Extra query parameters to append to the provider authorization URL. * Reserved OAuth keys (state, client_id, redirect_uri, response_type, - * code_challenge, code_challenge_method, scope) are rejected. + * code_challenge, code_challenge_method, nonce, scope) are rejected. */ additionalParams: additionalAuthorizationParamsSchema, /** @@ -383,9 +383,11 @@ export const linkSocialAccount = createAuthEndpoint( // Handle OAuth flow const stateNonce = generateRandomString(32); const codeVerifier = generateRandomString(128); + const idTokenNonce = generateIdTokenNonce(provider); const { url, requestedScopes } = await provider.createAuthorizationURL({ state: stateNonce, codeVerifier, + idTokenNonce, redirectURI: `${c.context.baseURL}${provider.callbackPath}`, scopes: c.body.scopes, loginHint: c.body.loginHint, @@ -400,6 +402,7 @@ export const linkSocialAccount = createAuthEndpoint( requestedScopes, state: stateNonce, codeVerifier, + idTokenNonce, }); if (!c.body.disableRedirect) { diff --git a/packages/better-auth/src/api/routes/callback.ts b/packages/better-auth/src/api/routes/callback.ts index 1e805e52fc..a3b9a7bb30 100644 --- a/packages/better-auth/src/api/routes/callback.ts +++ b/packages/better-auth/src/api/routes/callback.ts @@ -12,7 +12,11 @@ import { import { persistOAuthAccount } from "../../oauth2/persist-account"; import { applyUpdateUserInfoOnLink } from "../../oauth2/resolve-account"; import { signInWithOAuthIdentity } from "../../oauth2/sign-in-with-oauth-identity"; -import { generateState, parseState } from "../../oauth2/state"; +import { + generateIdTokenNonce, + generateState, + parseState, +} from "../../oauth2/state"; import { HIDE_METADATA } from "../../utils/hide-metadata"; import { isAPIError } from "../../utils/is-api-error"; import { assertValidUserInfo } from "../../utils/validate-user-info"; @@ -99,13 +103,20 @@ export const callbackOAuth = createAuthEndpoint( // bounce-back callback has no scope fallback (RFC 6749 §5.1). const state = generateRandomString(32); const codeVerifier = generateRandomString(128); + const idTokenNonce = generateIdTokenNonce(provider); const { url: authUrl, requestedScopes } = await provider.createAuthorizationURL({ state, codeVerifier, + idTokenNonce, redirectURI: `${c.context.baseURL}${provider.callbackPath}`, }); - await generateState(c, { requestedScopes, state, codeVerifier }); + await generateState(c, { + requestedScopes, + state, + codeVerifier, + idTokenNonce, + }); throw c.redirect(authUrl.toString()); } } @@ -125,6 +136,7 @@ export const callbackOAuth = createAuthEndpoint( newUserURL, requestSignUp, requestedScopes, + idTokenNonce, } = await parseState(c); function redirectOnError(error: string, description?: string | undefined) { @@ -170,6 +182,19 @@ export const callbackOAuth = createAuthEndpoint( throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.ISSUER_MISMATCH); } + // Fail closed: a provider that requires id_token nonce binding must carry + // an expected nonce recovered from state. If it is absent (a flow minted + // before binding was enabled, or a dropped state field), the redirect + // cannot enforce the binding, so refuse rather than trust an unbound + // id_token. + if (provider.requiresIdTokenNonce && !idTokenNonce) { + c.context.logger.error( + "OAuth id_token nonce binding required but no expected nonce was found in state", + { providerId: provider.id }, + ); + throw redirectOnError(OAUTH_CALLBACK_ERROR_CODES.NONCE_BINDING_MISSING); + } + let tokens: OAuth2Tokens | null; try { tokens = await provider.validateAuthorizationCode({ @@ -197,6 +222,7 @@ export const callbackOAuth = createAuthEndpoint( const providerResult = await provider.getUserInfo({ ...tokens, + ...(idTokenNonce ? { expectedIdTokenNonce: idTokenNonce } : {}), /** * The user object from the provider * This is only available for some providers like Apple diff --git a/packages/better-auth/src/api/routes/sign-in.ts b/packages/better-auth/src/api/routes/sign-in.ts index dc17d5a375..12bfdfdae6 100644 --- a/packages/better-auth/src/api/routes/sign-in.ts +++ b/packages/better-auth/src/api/routes/sign-in.ts @@ -18,7 +18,7 @@ import { OAUTH_CALLBACK_ERROR_CODES, } from "../../oauth2/errors"; import { signInWithOAuthIdentity } from "../../oauth2/sign-in-with-oauth-identity"; -import { generateState } from "../../utils"; +import { generateIdTokenNonce, generateState } from "../../utils"; import { formCsrfMiddleware } from "../middlewares/origin-check"; import { createEmailVerificationToken } from "./email-verification"; @@ -186,7 +186,7 @@ const socialSignInBodySchema = z.object({ /** * Extra query parameters to append to the provider authorization URL. * Reserved OAuth keys (state, client_id, redirect_uri, response_type, - * code_challenge, code_challenge_method, scope) are rejected. + * code_challenge, code_challenge_method, nonce, scope) are rejected. */ additionalParams: additionalAuthorizationParamsSchema, /** @@ -376,9 +376,11 @@ export const signInSocial = () => const state = generateRandomString(32); const codeVerifier = generateRandomString(128); + const idTokenNonce = generateIdTokenNonce(provider); const { url, requestedScopes } = await provider.createAuthorizationURL({ state, codeVerifier, + idTokenNonce, redirectURI: `${c.context.baseURL}${provider.callbackPath}`, scopes: c.body.scopes, loginHint: c.body.loginHint, @@ -389,6 +391,7 @@ export const signInSocial = () => requestedScopes, state, codeVerifier, + idTokenNonce, }); if (!c.body.disableRedirect) { diff --git a/packages/better-auth/src/oauth2/errors.ts b/packages/better-auth/src/oauth2/errors.ts index 6c4094a57d..6eb845679d 100644 --- a/packages/better-auth/src/oauth2/errors.ts +++ b/packages/better-auth/src/oauth2/errors.ts @@ -12,6 +12,7 @@ export const OAUTH_CALLBACK_ERROR_CODES = { ISSUER_MISSING: "issuer_missing", ISSUER_MISMATCH: "issuer_mismatch", INVALID_CODE: "invalid_code", + NONCE_BINDING_MISSING: "nonce_binding_missing", UNABLE_TO_GET_USER_INFO: "unable_to_get_user_info", NO_CALLBACK_URL: "no_callback_url", UNABLE_TO_LINK_ACCOUNT: "unable_to_link_account", diff --git a/packages/better-auth/src/oauth2/state.ts b/packages/better-auth/src/oauth2/state.ts index ae0165371f..16b2df1213 100644 --- a/packages/better-auth/src/oauth2/state.ts +++ b/packages/better-auth/src/oauth2/state.ts @@ -6,6 +6,19 @@ import type { StateData } from "../state"; import { generateGenericState, parseGenericState, StateError } from "../state"; import { redirectOnError } from "./errors"; +/** + * Mint the OIDC `nonce` for the redirect flow, or `undefined` when the provider + * does not require ID-token nonce binding. Every redirect entrypoint (social + * sign-in, account linking, IDP-initiated bounce, and the OAuth popup) mints + * through this helper, so the value sent on the authorization URL and the value + * persisted in state are produced one way and cannot drift apart. + */ +export function generateIdTokenNonce(provider: { + requiresIdTokenNonce?: boolean | undefined; +}): string | undefined { + return provider.requiresIdTokenNonce ? generateRandomString(32) : undefined; +} + /** * Inputs for {@link generateState}. Grouped into one object so call sites read * by name instead of by position. @@ -29,6 +42,8 @@ export interface GenerateStateOptions { state?: string | undefined; /** The PKCE `codeVerifier` already used to build the authorization URL. Minted when omitted. */ codeVerifier?: string | undefined; + /** The OIDC nonce already sent as the authorization URL `nonce` parameter. */ + idTokenNonce?: string | undefined; } export async function generateState( @@ -61,6 +76,7 @@ export async function generateState( expiresAt: Date.now() + 10 * 60 * 1000, requestSignUp: c.body?.requestSignUp, requestedScopes: options?.requestedScopes, + idTokenNonce: options?.idTokenNonce, }; await setOAuthState(stateData); diff --git a/packages/better-auth/src/plugins/generic-oauth/generic-oauth.test.ts b/packages/better-auth/src/plugins/generic-oauth/generic-oauth.test.ts index edf335e915..83d36bc1f4 100644 --- a/packages/better-auth/src/plugins/generic-oauth/generic-oauth.test.ts +++ b/packages/better-auth/src/plugins/generic-oauth/generic-oauth.test.ts @@ -3429,6 +3429,9 @@ describe("oauth2", async () => { "http://localhost:3000/api/auth/callback/idp-initiated", ); expect(url.searchParams.get("code")).toBeNull(); + // Discovery providers bind the id_token to a nonce, including on the + // server-side bounce restart minted in callback.ts. + expect(url.searchParams.get("nonce")).toBeTruthy(); }); it("should redirect to the error page when a stateless callback arrives for a provider without the flag", async () => { @@ -3744,6 +3747,343 @@ describe("oauth2", async () => { } }); + /** + * @see https://github.com/better-auth/better-auth/issues/7571 + */ + it("should bind a discovery id_token to the authorization request nonce", async () => { + let authorizationNonce = ""; + const { customFetchImpl, cookieSetter } = await getTestInstance({ + plugins: [ + genericOAuth({ + config: [ + { + providerId: "nonce-match", + discoveryUrl: `http://localhost:${port}/.well-known/openid-configuration`, + clientId, + clientSecret, + pkce: true, + authorizationUrlParams: { nonce: "configured-nonce" }, + getToken: async () => ({ + accessToken: "nonce-match-access-token", + idToken: await server.issuer.buildToken({ + scopesOrTransform: (_header, payload) => { + Object.assign(payload, { + aud: clientId, + sub: "nonce-match-user", + email: "nonce-match@test.com", + email_verified: true, + name: "Nonce Match", + nonce: authorizationNonce, + }); + }, + }), + tokenType: "bearer", + }), + }, + ], + }), + ], + }); + const client = createAuthClient({ + baseURL: "http://localhost:3000", + fetchOptions: { customFetchImpl }, + }); + const headers = new Headers(); + const res = await client.signIn.social({ + provider: "nonce-match", + callbackURL: "http://localhost:3000/dashboard", + newUserCallbackURL: "http://localhost:3000/new_user", + fetchOptions: { onSuccess: cookieSetter(headers) }, + }); + + authorizationNonce = + new URL(res.data?.url || "").searchParams.get("nonce") || ""; + expect(authorizationNonce).toBeTruthy(); + expect(authorizationNonce).not.toBe("configured-nonce"); + + const { callbackURL, headers: sessionHeaders } = await simulateOAuthFlow( + res.data?.url || "", + headers, + customFetchImpl, + ); + expect(callbackURL).toBe("http://localhost:3000/new_user"); + + const session = await client.getSession({ + fetchOptions: { headers: sessionHeaders }, + }); + expect(session.data?.user.email).toBe("nonce-match@test.com"); + }); + + /** + * @see https://github.com/better-auth/better-auth/issues/7571 + */ + it("should reject a discovery id_token with a mismatched nonce", async () => { + const { customFetchImpl, cookieSetter } = await getTestInstance({ + plugins: [ + genericOAuth({ + config: [ + { + providerId: "nonce-mismatch", + discoveryUrl: `http://localhost:${port}/.well-known/openid-configuration`, + clientId, + clientSecret, + pkce: true, + getToken: async () => ({ + accessToken: "nonce-mismatch-access-token", + idToken: await server.issuer.buildToken({ + scopesOrTransform: (_header, payload) => { + Object.assign(payload, { + aud: clientId, + sub: "nonce-mismatch-user", + email: "nonce-mismatch@test.com", + email_verified: true, + name: "Nonce Mismatch", + nonce: "different-nonce", + }); + }, + }), + tokenType: "bearer", + }), + }, + ], + }), + ], + }); + const client = createAuthClient({ + baseURL: "http://localhost:3000", + fetchOptions: { customFetchImpl }, + }); + const headers = new Headers(); + const res = await client.signIn.social({ + provider: "nonce-mismatch", + callbackURL: "http://localhost:3000/dashboard", + newUserCallbackURL: "http://localhost:3000/new_user", + fetchOptions: { onSuccess: cookieSetter(headers) }, + }); + + expect( + new URL(res.data?.url || "").searchParams.get("nonce"), + ).toBeTruthy(); + + const { callbackURL } = await simulateOAuthFlow( + res.data?.url || "", + headers, + customFetchImpl, + ); + expect(callbackURL).toContain("?error="); + }); + + /** + * @see https://github.com/better-auth/better-auth/issues/7571 + */ + it("should reject a discovery id_token that omits the nonce claim", async () => { + const { customFetchImpl, cookieSetter } = await getTestInstance({ + plugins: [ + genericOAuth({ + config: [ + { + providerId: "nonce-missing", + discoveryUrl: `http://localhost:${port}/.well-known/openid-configuration`, + clientId, + clientSecret, + pkce: true, + getToken: async () => ({ + accessToken: "nonce-missing-access-token", + idToken: await server.issuer.buildToken({ + scopesOrTransform: (_header, payload) => { + Object.assign(payload, { + aud: clientId, + sub: "nonce-missing-user", + email: "nonce-missing@test.com", + email_verified: true, + name: "Nonce Missing", + }); + }, + }), + tokenType: "bearer", + }), + }, + ], + }), + ], + }); + const client = createAuthClient({ + baseURL: "http://localhost:3000", + fetchOptions: { customFetchImpl }, + }); + const headers = new Headers(); + const res = await client.signIn.social({ + provider: "nonce-missing", + callbackURL: "http://localhost:3000/dashboard", + newUserCallbackURL: "http://localhost:3000/new_user", + fetchOptions: { onSuccess: cookieSetter(headers) }, + }); + + expect( + new URL(res.data?.url || "").searchParams.get("nonce"), + ).toBeTruthy(); + + const { callbackURL } = await simulateOAuthFlow( + res.data?.url || "", + headers, + customFetchImpl, + ); + expect(callbackURL).toContain("?error="); + }); + + it("should not bind a nonce when id_token nonce binding is disabled", async () => { + const { customFetchImpl, cookieSetter } = await getTestInstance({ + plugins: [ + genericOAuth({ + config: [ + { + providerId: "nonce-disabled", + discoveryUrl: `http://localhost:${port}/.well-known/openid-configuration`, + clientId, + clientSecret, + pkce: true, + disableIdTokenNonceBinding: true, + getToken: async () => ({ + accessToken: "nonce-disabled-access-token", + idToken: await server.issuer.buildToken({ + scopesOrTransform: (_header, payload) => { + Object.assign(payload, { + aud: clientId, + sub: "nonce-disabled-user", + email: "nonce-disabled@test.com", + email_verified: true, + name: "Nonce Disabled", + }); + }, + }), + tokenType: "bearer", + }), + }, + ], + }), + ], + }); + const client = createAuthClient({ + baseURL: "http://localhost:3000", + fetchOptions: { customFetchImpl }, + }); + const headers = new Headers(); + const res = await client.signIn.social({ + provider: "nonce-disabled", + callbackURL: "http://localhost:3000/dashboard", + newUserCallbackURL: "http://localhost:3000/new_user", + fetchOptions: { onSuccess: cookieSetter(headers) }, + }); + + expect(new URL(res.data?.url || "").searchParams.get("nonce")).toBeNull(); + + const { callbackURL, headers: sessionHeaders } = await simulateOAuthFlow( + res.data?.url || "", + headers, + customFetchImpl, + ); + expect(callbackURL).toBe("http://localhost:3000/new_user"); + + const session = await client.getSession({ + fetchOptions: { headers: sessionHeaders }, + }); + expect(session.data?.user.email).toBe("nonce-disabled@test.com"); + }); + + it("should drop a configured nonce param for providers without nonce binding", async () => { + const { customFetchImpl, cookieSetter } = await getTestInstance({ + plugins: [ + genericOAuth({ + config: [ + { + providerId: "nonce-static", + authorizationUrl: `http://localhost:${port}/authorize`, + tokenUrl: `http://localhost:${port}/token`, + clientId, + clientSecret, + pkce: true, + authorizationUrlParams: { nonce: "configured-nonce" }, + }, + ], + }), + ], + }); + const client = createAuthClient({ + baseURL: "http://localhost:3000", + fetchOptions: { customFetchImpl }, + }); + const headers = new Headers(); + const res = await client.signIn.social({ + provider: "nonce-static", + callbackURL: "http://localhost:3000/dashboard", + fetchOptions: { onSuccess: cookieSetter(headers) }, + }); + + expect(new URL(res.data?.url || "").searchParams.get("nonce")).toBeNull(); + }); + + /** + * @see https://github.com/better-auth/better-auth/pull/10095#discussion_r3417330694 + */ + it("should reject a discovery id_token when state carries no expected nonce", async () => { + const { customFetchImpl, cookieSetter, auth } = await getTestInstance({ + plugins: [ + genericOAuth({ + config: [ + { + providerId: "nonce-skew", + discoveryUrl: `http://localhost:${port}/.well-known/openid-configuration`, + clientId, + clientSecret, + pkce: true, + getToken: async () => ({ + accessToken: "nonce-skew-access-token", + idToken: await server.issuer.buildToken({ + scopesOrTransform: (_header, payload) => { + Object.assign(payload, { + aud: clientId, + sub: "nonce-skew-user", + email: "nonce-skew@test.com", + email_verified: true, + name: "Nonce Skew", + }); + }, + }), + tokenType: "bearer", + }), + }, + ], + }), + ], + }); + const ctx = await auth.$context; + const provider = ctx.socialProviders.find((p) => p.id === "nonce-skew")!; + const client = createAuthClient({ + baseURL: "http://localhost:3000", + fetchOptions: { customFetchImpl }, + }); + + // Mint state before binding is required, so it carries no expected nonce. + provider.requiresIdTokenNonce = false; + const headers = new Headers(); + const res = await client.signIn.social({ + provider: "nonce-skew", + callbackURL: "http://localhost:3000/dashboard", + newUserCallbackURL: "http://localhost:3000/new_user", + fetchOptions: { onSuccess: cookieSetter(headers) }, + }); + expect(new URL(res.data?.url || "").searchParams.get("nonce")).toBeNull(); + + // The provider now requires binding, but the in-flight state has none. + provider.requiresIdTokenNonce = true; + const { callbackURL } = await simulateOAuthFlow( + res.data?.url || "", + headers, + customFetchImpl, + ); + expect(callbackURL).toContain("error=nonce_binding_missing"); + }); + it("should sign in with a client-submitted id_token for a discovery provider", async () => { const token = await server.issuer.buildToken({ scopesOrTransform: (_header, payload) => { diff --git a/packages/better-auth/src/plugins/generic-oauth/index.ts b/packages/better-auth/src/plugins/generic-oauth/index.ts index 99d11e1e2b..553693a5ed 100644 --- a/packages/better-auth/src/plugins/generic-oauth/index.ts +++ b/packages/better-auth/src/plugins/generic-oauth/index.ts @@ -299,6 +299,9 @@ export const genericOAuth = ( callbackPath: `/callback/${c.providerId}`, issuer, idToken: idTokenConfig, + requiresIdTokenNonce: + idTokenConfig !== undefined && + c.disableIdTokenNonceBinding !== true, allowIdpInitiated: c.allowIdpInitiated, createAuthorizationURL(data) { if (!authorizationUrl) { @@ -329,6 +332,7 @@ export const genericOAuth = ( accessType: c.accessType, responseType: c.responseType, responseMode: c.responseMode, + nonce: data.idTokenNonce, additionalParams: { ...(c.authorizationUrlParams ?? {}), ...(data.additionalParams ?? {}), @@ -370,23 +374,25 @@ export const genericOAuth = ( ); }, async getUserInfo(tokens) { + const { expectedIdTokenNonce, ...oauthTokens } = tokens; // Fail closed: when discovery published a JWKS, an id_token // that cannot be verified must not become an identity source. - if (tokens.idToken && provider.idToken) { + if (oauthTokens.idToken && provider.idToken) { const verified = await verifyProviderIdToken( provider, - tokens.idToken, + oauthTokens.idToken, + expectedIdTokenNonce, ); if (!verified) { ctx.logger.error( - `Provider "${c.providerId}": id_token failed verification against the discovery JWKS`, + `Provider "${c.providerId}": id_token failed verification against the discovery JWKS or expected nonce`, ); return null; } } const raw = c.getUserInfo - ? await c.getUserInfo(tokens) - : await fetchUserInfo(tokens, userInfoUrl); + ? await c.getUserInfo(oauthTokens) + : await fetchUserInfo(oauthTokens, userInfoUrl); if (!raw) { return null; } diff --git a/packages/better-auth/src/plugins/generic-oauth/types.ts b/packages/better-auth/src/plugins/generic-oauth/types.ts index 9030640ba4..5e4307873f 100644 --- a/packages/better-auth/src/plugins/generic-oauth/types.ts +++ b/packages/better-auth/src/plugins/generic-oauth/types.ts @@ -215,4 +215,17 @@ export interface GenericOAuthConfig { * @default false */ allowIdpInitiated?: boolean | undefined; + /** + * Disable OIDC `nonce` binding for this provider's `id_token`. + * + * Providers configured with `discoveryUrl` that publish a JWKS bind the + * `id_token` to the authorization request by default: Better Auth sends a + * server-generated `nonce` and rejects a callback whose `id_token` does not + * echo it (OIDC Core 1.0 §3.1.3.7). Set this to `true` only for OIDC + * providers that do not return the `nonce` claim in the authorization-code + * flow; doing so removes `id_token` replay protection for this provider. + * + * @default false + */ + disableIdTokenNonceBinding?: boolean | undefined; } diff --git a/packages/better-auth/src/plugins/oauth-popup/index.ts b/packages/better-auth/src/plugins/oauth-popup/index.ts index 67af4393ce..81b83d0a9b 100644 --- a/packages/better-auth/src/plugins/oauth-popup/index.ts +++ b/packages/better-auth/src/plugins/oauth-popup/index.ts @@ -17,6 +17,7 @@ import { splitSetCookieHeader, } from "../../cookies"; import { generateRandomString } from "../../crypto"; +import { generateIdTokenNonce } from "../../oauth2/state"; import type { StateData } from "../../state"; import { generateGenericState, INTERNAL_STATE_KEYS } from "../../state"; import { HIDE_METADATA } from "../../utils/hide-metadata"; @@ -215,6 +216,7 @@ const oauthPopupStart = createAuthEndpoint( let url: URL; try { const codeVerifier = generateRandomString(128); + const idTokenNonce = generateIdTokenNonce(provider); const parsedAdditionalData = c.query.additionalData ? (safeJSONParse>(c.query.additionalData) ?? {}) : {}; @@ -227,6 +229,7 @@ const oauthPopupStart = createAuthEndpoint( ...additionalData, callbackURL, codeVerifier, + idTokenNonce, errorURL: c.query.errorCallbackURL, newUserURL: c.query.newUserCallbackURL, requestSignUp: c.query.requestSignUp === "true" ? true : undefined, @@ -249,6 +252,7 @@ const oauthPopupStart = createAuthEndpoint( ({ url } = await provider.createAuthorizationURL({ state, codeVerifier, + idTokenNonce, redirectURI: `${c.context.baseURL}/callback/${provider.id}`, scopes: c.query.scopes ? c.query.scopes.split(",") : undefined, })); diff --git a/packages/better-auth/src/state.ts b/packages/better-auth/src/state.ts index 424e6599c7..619ce0a76e 100644 --- a/packages/better-auth/src/state.ts +++ b/packages/better-auth/src/state.ts @@ -26,6 +26,11 @@ const stateDataSchema = z.looseObject({ }) .optional(), requestSignUp: z.boolean().optional(), + /** + * OIDC nonce sent as the authorization request `nonce` parameter when the + * provider requires an ID token to be bound to this redirect flow. + */ + idTokenNonce: z.string().optional(), /** * The effective set of scopes requested in the authorization URL, captured * from `createAuthorizationURL`. Persisted so the callback can fall back to diff --git a/packages/core/src/oauth2/create-authorization-url.test.ts b/packages/core/src/oauth2/create-authorization-url.test.ts index 4eb9507b16..d3f9745f3f 100644 --- a/packages/core/src/oauth2/create-authorization-url.test.ts +++ b/packages/core/src/oauth2/create-authorization-url.test.ts @@ -29,6 +29,7 @@ describe("createAuthorizationURL", () => { it("silently drops reserved OAuth params supplied via additionalParams", async () => { const { url } = await createAuthorizationURL({ ...baseInput, + nonce: "server-nonce", additionalParams: { state: "attacker-controlled", client_id: "attacker", @@ -36,6 +37,7 @@ describe("createAuthorizationURL", () => { response_type: "token", code_challenge: "malicious", code_challenge_method: "plain", + nonce: "attacker-nonce", scope: "admin", identity_provider: "OktaSSO", }, @@ -47,6 +49,7 @@ describe("createAuthorizationURL", () => { ); expect(url.searchParams.get("response_type")).toBe("code"); expect(url.searchParams.get("code_challenge")).toBeNull(); + expect(url.searchParams.get("nonce")).toBe("server-nonce"); expect(url.searchParams.get("scope")).toBe("openid"); expect(url.searchParams.get("identity_provider")).toBe("OktaSSO"); }); @@ -59,6 +62,7 @@ describe("createAuthorizationURL", () => { "response_type", "code_challenge", "code_challenge_method", + "nonce", "scope", ]); }); diff --git a/packages/core/src/oauth2/create-authorization-url.ts b/packages/core/src/oauth2/create-authorization-url.ts index bd136799f6..10d3294861 100644 --- a/packages/core/src/oauth2/create-authorization-url.ts +++ b/packages/core/src/oauth2/create-authorization-url.ts @@ -15,6 +15,7 @@ export const RESERVED_AUTHORIZATION_PARAMS = [ "response_type", "code_challenge", "code_challenge_method", + "nonce", "scope", ] as const; @@ -37,6 +38,7 @@ export async function createAuthorizationURL({ responseType, display, loginHint, + nonce, hd, responseMode, additionalParams, @@ -56,6 +58,7 @@ export async function createAuthorizationURL({ responseType?: string | undefined; display?: string | undefined; loginHint?: string | undefined; + nonce?: string | undefined; hd?: string | undefined; responseMode?: string | undefined; additionalParams?: Record | undefined; @@ -77,6 +80,7 @@ export async function createAuthorizationURL({ duration && url.searchParams.set("duration", duration); display && url.searchParams.set("display", display); loginHint && url.searchParams.set("login_hint", loginHint); + nonce && url.searchParams.set("nonce", nonce); prompt && url.searchParams.set("prompt", prompt); hd && url.searchParams.set("hd", hd); accessType && url.searchParams.set("access_type", accessType); diff --git a/packages/core/src/oauth2/oauth-provider.ts b/packages/core/src/oauth2/oauth-provider.ts index 0edc82bb54..66981b919a 100644 --- a/packages/core/src/oauth2/oauth-provider.ts +++ b/packages/core/src/oauth2/oauth-provider.ts @@ -142,6 +142,12 @@ export interface UpstreamProvider< redirectURI: string; display?: string | undefined; loginHint?: string | undefined; + /** + * OIDC nonce generated by the redirect initiator and persisted in OAuth + * state. Providers that set `requiresIdTokenNonce` must forward this to + * the authorization URL as the `nonce` parameter. + */ + idTokenNonce?: string | undefined; /** * Extra query parameters to append to the authorization URL. * Providers forward these to the shared `createAuthorizationURL` helper, @@ -159,6 +165,12 @@ export interface UpstreamProvider< }) => Promise; getUserInfo: ( token: OAuth2Tokens & { + /** + * OIDC nonce recovered from OAuth state. Providers that required an + * ID-token nonce must pass this to `verifyProviderIdToken` before + * trusting ID-token claims. + */ + expectedIdTokenNonce?: string | undefined; /** * The user object from the provider * This is only available for some providers like Apple @@ -195,6 +207,13 @@ export interface UpstreamProvider< * against this value to prevent authorization server mix-up attacks. */ issuer?: string | undefined; + /** + * Require shared OAuth redirect routes to bind ID-token verification to an + * authorization request nonce. When true, routes generate `idTokenNonce`, + * pass it to `createAuthorizationURL`, persist it in state, and provide it + * back to `getUserInfo` as `expectedIdTokenNonce`. + */ + requiresIdTokenNonce?: boolean | undefined; /** * Disable implicit sign up for new users. When set to true for the provider, * sign-in need to be called with with requestSignUp as true to create new users. diff --git a/packages/core/src/oauth2/verify-id-token.ts b/packages/core/src/oauth2/verify-id-token.ts index 111ec7fefd..189d799cbd 100644 --- a/packages/core/src/oauth2/verify-id-token.ts +++ b/packages/core/src/oauth2/verify-id-token.ts @@ -79,6 +79,8 @@ export async function verifyProviderIdToken( // Opaque (non-JWS) tokens carry no signature to check. They are accepted only when the // provider opts in, in which case getUserInfo resolves identity from the access token via // the provider's userinfo endpoint, which validates it (e.g. Facebook Graph access tokens). + // An expected `nonce` is not enforced here: an opaque token carries no `nonce` claim, and the + // access-token-backed userinfo exchange (not the token itself) is the identity source. if (token.split(".").length !== 3) { return config.allowOpaqueToken === true; }