mirror of
https://github.com/better-auth/better-auth.git
synced 2026-07-24 08:58:45 -05:00
fix(oauth-provider): allow nonce-bound offline access without PKCE (#10153)
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@better-auth/oauth-provider": patch
|
||||
---
|
||||
|
||||
Allows confidential OIDC clients that have opted out of PKCE to request `offline_access` when the authorization request includes both `openid` and `nonce`.
|
||||
@@ -1354,7 +1354,7 @@ By default, PKCE is required for all clients. This provides maximum security and
|
||||
**PKCE is always required for:**
|
||||
|
||||
* Public clients (native/user-agent-based applications)
|
||||
* Any authorization request with the `offline_access` scope (refresh tokens)
|
||||
* Authorization requests with the `offline_access` scope, unless a confidential client has opted out of PKCE and the OIDC request includes both `openid` and `nonce`
|
||||
|
||||
#### Per-Client PKCE Configuration
|
||||
|
||||
@@ -1379,7 +1379,7 @@ The `require_pkce` field:
|
||||
* Defaults to `true` (PKCE required)
|
||||
* Only applies to confidential clients
|
||||
* Ignored for public clients (PKCE always required)
|
||||
* Ignored for `offline_access` scope (PKCE always required)
|
||||
* Requires an OIDC request with both `openid` and `nonce` when `offline_access` is requested without PKCE
|
||||
|
||||
#### Dynamic Client Registration PKCE Configuration
|
||||
|
||||
@@ -1392,7 +1392,7 @@ oauthProvider({
|
||||
})
|
||||
```
|
||||
|
||||
This only applies to confidential clients created through dynamic client registration. Public clients and authorization requests with the `offline_access` scope still require PKCE.
|
||||
This only applies to confidential clients created through dynamic client registration. Public clients still require PKCE. Confidential OIDC clients that request `offline_access` without PKCE must send both `openid` and `nonce`.
|
||||
|
||||
**When to use `require_pkce: false`:**
|
||||
|
||||
|
||||
@@ -583,8 +583,11 @@ export async function authorizeEndpoint(
|
||||
}
|
||||
}
|
||||
|
||||
// Check if PKCE is required for this client and scope
|
||||
const pkceRequired = isPKCERequired(client, requestedScopes);
|
||||
// Check if PKCE is required for this client and authorization request
|
||||
const pkceRequired = isPKCERequired(client, {
|
||||
scopes: requestedScopes,
|
||||
nonce: query.nonce,
|
||||
});
|
||||
|
||||
// Validate PKCE parameters if required
|
||||
if (pkceRequired) {
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
} from "better-auth/oauth2";
|
||||
import { jwt } from "better-auth/plugins/jwt";
|
||||
import { getTestInstance } from "better-auth/test";
|
||||
import { decodeJwt } from "jose";
|
||||
import { beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { oauthProviderClient } from "./client";
|
||||
import { oauthProvider } from "./oauth";
|
||||
@@ -465,8 +466,7 @@ describe("PKCE optional - offline_access scope", async () => {
|
||||
confidentialClient = confResponse;
|
||||
});
|
||||
|
||||
it("offline_access without PKCE should fail even with requirePKCE: false", async () => {
|
||||
// Try to authorize with offline_access but without PKCE
|
||||
it("offline_access without PKCE or OIDC nonce should fail even with requirePKCE: false", async () => {
|
||||
const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`);
|
||||
authUrl.searchParams.set("client_id", confidentialClient.client_id);
|
||||
authUrl.searchParams.set("redirect_uri", redirectUri);
|
||||
@@ -483,10 +483,96 @@ describe("PKCE optional - offline_access scope", async () => {
|
||||
|
||||
expect(errorRedirect).toContain("error=invalid_request");
|
||||
expect(errorRedirect).toContain(
|
||||
"pkce+is+required+when+requesting+offline_access+scope",
|
||||
"pkce+or+OIDC+nonce+is+required+when+requesting+offline_access+scope",
|
||||
);
|
||||
});
|
||||
|
||||
it("offline_access without PKCE should succeed for confidential OIDC requests with nonce", async () => {
|
||||
const nonce = "offline-access-nonce";
|
||||
const authUrl = await createAuthorizationURL({
|
||||
id: providerId,
|
||||
options: {
|
||||
clientId: confidentialClient.client_id,
|
||||
clientSecret: confidentialClient.client_secret,
|
||||
},
|
||||
redirectURI: redirectUri,
|
||||
state: "123",
|
||||
scopes: ["openid", "offline_access"],
|
||||
responseType: "code",
|
||||
authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`,
|
||||
nonce,
|
||||
});
|
||||
|
||||
let callbackUrl = "";
|
||||
await authenticatedClient.$fetch(authUrl.toString(), {
|
||||
onError(context) {
|
||||
callbackUrl = context.response.headers.get("Location") || "";
|
||||
},
|
||||
});
|
||||
|
||||
expect(callbackUrl).toContain(redirectUri);
|
||||
expect(callbackUrl).toContain("code=");
|
||||
expect(callbackUrl).not.toContain("error=");
|
||||
|
||||
const url = resolveUrl(callbackUrl, authServerBaseUrl);
|
||||
const code = url.searchParams.get("code");
|
||||
expect(code).toBeDefined();
|
||||
|
||||
const { body, headers } = await authorizationCodeRequest({
|
||||
code: code!,
|
||||
redirectURI: redirectUri,
|
||||
options: {
|
||||
clientId: confidentialClient.client_id,
|
||||
clientSecret: confidentialClient.client_secret,
|
||||
redirectURI: redirectUri,
|
||||
},
|
||||
});
|
||||
|
||||
const tokenResponse = await authenticatedClient.$fetch<{
|
||||
access_token?: string;
|
||||
id_token?: string;
|
||||
refresh_token?: string;
|
||||
}>("/oauth2/token", {
|
||||
method: "POST",
|
||||
body,
|
||||
headers,
|
||||
});
|
||||
|
||||
expect(tokenResponse.data?.access_token).toBeDefined();
|
||||
expect(tokenResponse.data?.id_token).toBeDefined();
|
||||
expect(tokenResponse.data?.refresh_token).toBeDefined();
|
||||
expect(decodeJwt(tokenResponse.data!.id_token!).nonce).toBe(nonce);
|
||||
});
|
||||
|
||||
it("offline_access without PKCE should fail for non-OIDC requests with nonce", async () => {
|
||||
const authUrl = await createAuthorizationURL({
|
||||
id: providerId,
|
||||
options: {
|
||||
clientId: confidentialClient.client_id,
|
||||
clientSecret: confidentialClient.client_secret,
|
||||
},
|
||||
redirectURI: redirectUri,
|
||||
state: "123",
|
||||
scopes: ["offline_access"],
|
||||
responseType: "code",
|
||||
authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`,
|
||||
nonce: "unused-without-openid",
|
||||
});
|
||||
|
||||
let errorRedirect = "";
|
||||
await authenticatedClient.$fetch(authUrl.toString(), {
|
||||
onError(context) {
|
||||
errorRedirect = context.response.headers.get("Location") || "";
|
||||
},
|
||||
});
|
||||
|
||||
expect(errorRedirect).toContain("error=invalid_request");
|
||||
expect(errorRedirect).toContain(
|
||||
"pkce+or+OIDC+nonce+is+required+when+requesting+offline_access+scope",
|
||||
);
|
||||
expect(errorRedirect).not.toContain("code=");
|
||||
});
|
||||
|
||||
it("offline_access with PKCE should succeed", async () => {
|
||||
const codeVerifier = generateRandomString(64);
|
||||
const authUrl = await createAuthorizationURL({
|
||||
|
||||
@@ -1237,8 +1237,11 @@ async function handleAuthorizationCodeGrant(
|
||||
const requestedScopes =
|
||||
(verificationValue.query?.scope as string)?.split(" ") || [];
|
||||
|
||||
// Check if PKCE is required for this client
|
||||
const pkceRequired = isPKCERequired(client, requestedScopes);
|
||||
// Check if PKCE is required for this client and authorization request
|
||||
const pkceRequired = isPKCERequired(client, {
|
||||
scopes: requestedScopes,
|
||||
nonce: verificationValue.query?.nonce,
|
||||
});
|
||||
|
||||
// Validate credentials based on requirements
|
||||
if (pkceRequired) {
|
||||
|
||||
@@ -717,7 +717,8 @@ export interface OAuthOptions<
|
||||
*
|
||||
* This is server-owned registration policy. Dynamic client registration does
|
||||
* not accept `require_pkce` from the client request, and public clients or
|
||||
* authorization requests with `offline_access` still require PKCE.
|
||||
* authorization requests with `offline_access` still require PKCE unless the
|
||||
* confidential OIDC request includes both `openid` and `nonce`.
|
||||
*
|
||||
* @default true
|
||||
*/
|
||||
|
||||
@@ -392,8 +392,9 @@ export interface OAuthClient {
|
||||
*
|
||||
* @default true
|
||||
*
|
||||
* Note: PKCE is always required for public clients and when
|
||||
* requesting offline_access scope, regardless of this setting.
|
||||
* Note: PKCE is always required for public clients. When requesting
|
||||
* offline_access without PKCE, confidential OIDC clients must send both
|
||||
* `openid` and `nonce`.
|
||||
*/
|
||||
require_pkce?: boolean;
|
||||
/**
|
||||
|
||||
@@ -915,17 +915,24 @@ export function removeMaxAgeFromQuery(query: URLSearchParams) {
|
||||
|
||||
enum PKCERequirementErrors {
|
||||
PUBLIC_CLIENT = "pkce is required for public clients",
|
||||
OFFLINE_ACCESS_SCOPE = "pkce is required when requesting offline_access scope",
|
||||
OFFLINE_ACCESS_SCOPE = "pkce or OIDC nonce is required when requesting offline_access scope",
|
||||
CLIENT_REQUIRE_PKCE = "pkce is required for this client",
|
||||
}
|
||||
|
||||
interface AuthorizationPKCEContext {
|
||||
scopes?: string[];
|
||||
nonce?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines if PKCE is required for a given client and scope.
|
||||
* Determines if PKCE is required for a given client and authorization request.
|
||||
*
|
||||
* PKCE is always required for:
|
||||
* 1. Public clients (cannot securely store client_secret)
|
||||
* 2. Requests with offline_access scope (refresh token security)
|
||||
* 2. Requests with offline_access scope unless a confidential OIDC request
|
||||
* already uses nonce as its authorization-code injection countermeasure.
|
||||
*
|
||||
* For confidential clients without offline_access:
|
||||
* For confidential clients:
|
||||
* - Uses client.requirePKCE if set (defaults to true)
|
||||
*
|
||||
* Returns false if PKCE is not required, or the reason it is required.
|
||||
@@ -934,7 +941,7 @@ enum PKCERequirementErrors {
|
||||
*/
|
||||
export function isPKCERequired(
|
||||
client: SchemaClient<Scope[]>,
|
||||
requestedScopes?: string[],
|
||||
request?: AuthorizationPKCEContext,
|
||||
): false | PKCERequirementErrors {
|
||||
// Determine if client is public
|
||||
const isPublicClient =
|
||||
@@ -948,8 +955,13 @@ export function isPKCERequired(
|
||||
return PKCERequirementErrors.PUBLIC_CLIENT;
|
||||
}
|
||||
|
||||
// PKCE always required for offline_access scope (refresh tokens)
|
||||
if (requestedScopes?.includes("offline_access")) {
|
||||
const requestScopes = request?.scopes ?? [];
|
||||
const isOpenIdRequest = requestScopes.includes("openid");
|
||||
const hasNonce =
|
||||
typeof request?.nonce === "string" && request.nonce.length > 0;
|
||||
const hasOidcNonce = isOpenIdRequest && hasNonce;
|
||||
|
||||
if (requestScopes.includes("offline_access") && !hasOidcNonce) {
|
||||
return PKCERequirementErrors.OFFLINE_ACCESS_SCOPE;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user