diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index d1549279c6..a46522d9ac 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -1090,6 +1090,76 @@ oauthProvider({ ``` +### PKCE Configuration + +PKCE (Proof Key for Code Exchange) is a security mechanism that prevents authorization code interception attacks. This plugin follows the OAuth 2.1 specification, which requires PKCE by default for all authorization code flows. + +#### Default Behavior + +By default, PKCE is required for all clients. This provides maximum security and follows OAuth 2.1 best practices. + +**PKCE is always required for:** +- Public clients (native/user-agent-based applications) +- Any authorization request with the `offline_access` scope (refresh tokens) + +#### Per-Client PKCE Configuration + +Individual clients can opt-out of PKCE requirement during registration if needed for compatibility: + +```ts title="register-client.ts" +// Register a confidential client that doesn't support PKCE +const response = await auth.api.createOAuthClient({ + headers, + body: { + client_name: 'Legacy Backend Service', + redirect_uris: ['https://app.example.com/callback'], + token_endpoint_auth_method: 'client_secret_post', + grant_types: ['authorization_code'], + require_pkce: false, // Opt-out of PKCE requirement + } +}); +``` + +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) + +**When to use `require_pkce: false`:** +- Migrating from OAuth 2.0 with legacy confidential clients that don't support PKCE +- Backend-to-backend integrations where updating the client is not feasible +- Temporary compatibility during a phased migration + +**Recommendation:** Keep PKCE enabled (default) whenever possible. PKCE provides defense-in-depth even for confidential clients. + +#### Migrating from oidc-provider + +If you're migrating from the deprecated `oidc-provider` plugin and have confidential clients that don't support PKCE: + +1. **For legacy clients, opt-out per-client:** + Set `require_pkce: false` when registering clients that cannot be updated to support PKCE. + +2. **For new clients, use PKCE:** + New client registrations should always use PKCE (the default) for better security. + +3. **Phase out non-PKCE clients:** + Plan to upgrade or replace clients that don't support PKCE over time. + +4. **Monitor usage:** + Track which clients have `require_pkce: false` for migration planning. + +#### Security Considerations + +PKCE prevents authorization code interception attacks. Even for confidential clients with client_secret authentication, PKCE provides additional security: + +- **Defense in depth**: Multiple security layers +- **Protection against misconfiguration**: Accidental secret exposure +- **Future-proof**: Aligns with OAuth 2.1 best practices + +Only disable PKCE for confidential clients when absolutely necessary for legacy compatibility. + + ### Organizations OAuth Clients are tied to either a user or `reference_id` at registration and is immutable. If you are utilizing the [organization plugin](/docs/plugins/organization), you must ensure that the [`activeOrganizationId`](/docs/plugins/organization#active-organization) is set on your active session when you create new clients. @@ -1868,7 +1938,7 @@ To improve lookup performance, database adapters may map the field `client_id` o - **`clientRegistrationDefaultScopes`** (previously `defaultScope`) is now in array format instead of a space-separated string - **`consentPage`** is now required - **`getConsentHTML`** is removed in favor of the `consentPage` as raw html is not a response type supported by the authorize endpoint in OAuth -- **`requirePKCE`** is removed as PKCE is required in OAuth 2.1 +- **`requirePKCE`** (global option) is removed. PKCE is now required by default per OAuth 2.1. Individual clients can opt-out using `require_pkce: false` during registration if needed for legacy compatibility. - **`allowPlainCodeChallengeMethod`** is removed as the `plain` code challenge is considered less secure than the default `S256` method - **`customUserInfoClaims`** (previously `getAdditionalUserInfoClaim`) passes the jwt payload instead of the client of the access token used in the request. - **`storeClientSecret`** now defaults to `hashed`, or `encrypted` if `disableJwtPlugin: true` (previously `plain`). @@ -1903,6 +1973,7 @@ const defaultHasher = async (value: string) => { - Clients with `type: "user-agent-based"`: set `public: true` and `clientSecret: undefined` - Clients with `clientSecret: undefined`: set `public: true` - `redirectURLs` renamed to `redirectUris` +- `requirePkce` field added (optional, defaults to `true`). For existing confidential clients that don't support PKCE, set `requirePkce: false`. - `metadata` is now stored in database as individual fields instead of a JSON object. Parse the metadata into their respective fields. The OIDC plugin did not utilize this field but this OAuth plugin may utilize them in the future. ##### Table: `oauthAccessToken` diff --git a/packages/oauth-provider/src/authorize.ts b/packages/oauth-provider/src/authorize.ts index eb9d16e0df..5233e9f976 100644 --- a/packages/oauth-provider/src/authorize.ts +++ b/packages/oauth-provider/src/authorize.ts @@ -11,7 +11,14 @@ import type { Scope, VerificationValue, } from "./types"; -import { getClient, getJwtPlugin, parsePrompt, storeToken } from "./utils"; + +import { + getClient, + getJwtPlugin, + isPKCERequired, + parsePrompt, + storeToken, +} from "./utils"; /** * Formats an error url @@ -198,12 +205,7 @@ export async function authorizeEndpoint( if (requestedScopes) { const validScopes = new Set(client.scopes ?? opts.scopes); const invalidScopes = requestedScopes.filter((scope) => { - return ( - !validScopes?.has(scope) || - // offline access must be requested through PKCE - (scope === "offline_access" && - (query.code_challenge_method !== "S256" || !query.code_challenge)) - ); + return !validScopes?.has(scope); }); if (invalidScopes.length) { throw ctx.redirect( @@ -223,30 +225,52 @@ export async function authorizeEndpoint( query.scope = requestedScopes.join(" "); } - if (!query.code_challenge || !query.code_challenge_method) { - throw ctx.redirect( - formatErrorURL( - query.redirect_uri, - "invalid_request", - "pkce is required", - query.state, - getIssuer(ctx, opts), - ), - ); + // Check if PKCE is required for this client and scope + const pkceRequired = isPKCERequired(client, requestedScopes); + + // Validate PKCE parameters if required + if (pkceRequired) { + if (!query.code_challenge || !query.code_challenge_method) { + throw ctx.redirect( + formatErrorURL( + query.redirect_uri, + "invalid_request", + pkceRequired.valueOf(), + query.state, + getIssuer(ctx, opts), + ), + ); + } } - // Check code challenges - const codeChallengesSupported = ["S256"]; - if (!codeChallengesSupported.includes(query.code_challenge_method)) { - throw ctx.redirect( - formatErrorURL( - query.redirect_uri, - "invalid_request", - "invalid code_challenge method", - query.state, - getIssuer(ctx, opts), - ), - ); + // If PKCE parameters are provided, validate them (even if not required) + if (query.code_challenge || query.code_challenge_method) { + // Both parameters must be provided together + if (!query.code_challenge || !query.code_challenge_method) { + throw ctx.redirect( + formatErrorURL( + query.redirect_uri, + "invalid_request", + "code_challenge and code_challenge_method must both be provided", + query.state, + getIssuer(ctx, opts), + ), + ); + } + + // Check code challenge method is supported (only S256) + const codeChallengesSupported = ["S256"]; + if (!codeChallengesSupported.includes(query.code_challenge_method)) { + throw ctx.redirect( + formatErrorURL( + query.redirect_uri, + "invalid_request", + "invalid code_challenge method, only S256 is supported", + query.state, + getIssuer(ctx, opts), + ), + ); + } } // Check for session diff --git a/packages/oauth-provider/src/oauthClient/index.ts b/packages/oauth-provider/src/oauthClient/index.ts index dd5c0055a7..1229c201fd 100644 --- a/packages/oauth-provider/src/oauthClient/index.ts +++ b/packages/oauth-provider/src/oauthClient/index.ts @@ -56,6 +56,7 @@ export const adminCreateOAuthClient = (opts: OAuthOptions) => .default(0), skip_consent: z.boolean().optional(), enable_end_session: z.boolean().optional(), + require_pkce: z.boolean().optional(), metadata: z.record(z.string(), z.unknown()).optional(), }), metadata: { @@ -194,6 +195,11 @@ export const adminCreateOAuthClient = (opts: OAuthOptions) => type: "boolean", description: "Whether the client is disabled", }, + require_pkce: { + type: "boolean", + description: "Whether the client requires PKCE", + default: true, + }, metadata: { type: "object", additionalProperties: true, diff --git a/packages/oauth-provider/src/pkce-optional.test.ts b/packages/oauth-provider/src/pkce-optional.test.ts new file mode 100644 index 0000000000..670b2884fa --- /dev/null +++ b/packages/oauth-provider/src/pkce-optional.test.ts @@ -0,0 +1,697 @@ +import { createAuthClient } from "better-auth/client"; +import { generateRandomString } from "better-auth/crypto"; +import { + createAuthorizationCodeRequest, + createAuthorizationURL, +} from "better-auth/oauth2"; +import { jwt } from "better-auth/plugins/jwt"; +import { getTestInstance } from "better-auth/test"; +import { beforeAll, describe, expect, it } from "vitest"; +import { oauthProviderClient } from "./client"; +import { oauthProvider } from "./oauth"; +import type { OAuthClient } from "./types/oauth"; + +/** + * Resolves a URL that may be relative or absolute + */ +function resolveUrl(url: string, baseUrl: string): URL { + try { + // Try to parse as absolute URL first + return new URL(url); + } catch { + // If it fails, resolve as relative URL + return new URL(url, baseUrl); + } +} + +describe("PKCE optional - default behavior", async () => { + const authServerBaseUrl = "http://localhost:3000"; + const rpBaseUrl = "http://localhost:5000"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const authenticatedClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: authServerBaseUrl, + fetchOptions: { + customFetchImpl, + headers, + }, + }); + + let confidentialClient: OAuthClient; + let publicClient: OAuthClient; + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + + beforeAll(async () => { + // Create confidential client + const confResponse = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + }, + }); + confidentialClient = confResponse; + + // Create public client + const pubResponse = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + token_endpoint_auth_method: "none", + }, + }); + publicClient = pubResponse; + }); + + it("public client without PKCE should fail", async () => { + // Try to authorize without PKCE + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", publicClient.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "123"); + // Intentionally omit code_challenge and code_challenge_method + + 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+is+required+for+public+clients"); + }); + + it("confidential client without PKCE should fail with default settings", async () => { + // Try to authorize without PKCE + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", confidentialClient.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "123"); + // Intentionally omit code_challenge and code_challenge_method + + 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+is+required+for+this+client"); + }); + + it("confidential client with PKCE should succeed", async () => { + const codeVerifier = generateRandomString(64); + const authUrl = await createAuthorizationURL({ + id: providerId, + options: { + clientId: confidentialClient.client_id, + clientSecret: confidentialClient.client_secret, + }, + redirectURI: redirectUri, + state: "123", + scopes: ["openid"], + responseType: "code", + codeVerifier, + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + }); + + 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).toContain("state=123"); + expect(callbackUrl).not.toContain("error="); + }); +}); + +describe("PKCE optional - per-client opt-out", async () => { + const authServerBaseUrl = "http://localhost:3001"; + const rpBaseUrl = "http://localhost:5001"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const authenticatedClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: authServerBaseUrl, + fetchOptions: { + customFetchImpl, + headers, + }, + }); + + let confidentialClient: OAuthClient; + let publicClient: OAuthClient; + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + + beforeAll(async () => { + // Create confidential client with PKCE disabled + const confResponse = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + require_pkce: false, + }, + }); + confidentialClient = confResponse; + + // Create public client + const pubResponse = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + token_endpoint_auth_method: "none", + }, + }); + publicClient = pubResponse; + }); + + it("public client without PKCE should always fail", async () => { + // Try to authorize without PKCE + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", publicClient.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "123"); + // Intentionally omit code_challenge and code_challenge_method + + 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+is+required+for+public+clients"); + }); + + it("confidential client without PKCE should succeed", async () => { + // Authorize without PKCE + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", confidentialClient.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "123"); + + let callbackUrl = ""; + await authenticatedClient.$fetch(authUrl.toString(), { + onError(context) { + callbackUrl = context.response.headers.get("Location") || ""; + }, + }); + + expect(callbackUrl).toContain(redirectUri); + expect(callbackUrl).not.toContain("code_challenge="); + expect(callbackUrl).toContain("state=123"); + expect(callbackUrl).not.toContain("error="); + + // Extract code and exchange for token with client_secret + const url = resolveUrl(callbackUrl, authServerBaseUrl); + const code = url.searchParams.get("code"); + expect(code).toBeDefined(); + + const { body, headers } = createAuthorizationCodeRequest({ + 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(); + }); +}); + +describe("PKCE optional - offline_access scope", async () => { + const authServerBaseUrl = "http://localhost:3002"; + const rpBaseUrl = "http://localhost:5002"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const authenticatedClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: authServerBaseUrl, + fetchOptions: { + customFetchImpl, + headers, + }, + }); + + let confidentialClient: OAuthClient; + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + + beforeAll(async () => { + const confResponse = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + require_pkce: false, // Explicitly optional + }, + }); + confidentialClient = confResponse; + }); + + it("offline_access without PKCE should fail even with requirePKCE: false", async () => { + // Try to authorize with offline_access but without PKCE + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", confidentialClient.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid offline_access"); + authUrl.searchParams.set("state", "123"); + + 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+is+required+when+requesting+offline_access+scope", + ); + }); + + it("offline_access with PKCE should succeed", async () => { + const codeVerifier = generateRandomString(64); + 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", + codeVerifier, + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + }); + + 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="); + + // Exchange for token - should get refresh token + const url = resolveUrl(callbackUrl, authServerBaseUrl); + const code = url.searchParams.get("code"); + expect(code).toBeDefined(); + + const { body, headers } = createAuthorizationCodeRequest({ + code: code!, + redirectURI: redirectUri, + codeVerifier, + 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?.refresh_token).toBeDefined(); + }); +}); + +describe("PKCE optional - consistency checks", async () => { + const authServerBaseUrl = "http://localhost:3003"; + const rpBaseUrl = "http://localhost:5003"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const authenticatedClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: authServerBaseUrl, + fetchOptions: { + customFetchImpl, + headers, + }, + }); + + let confidentialClient: OAuthClient; + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + + beforeAll(async () => { + const confResponse = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + require_pkce: false, + }, + }); + confidentialClient = confResponse; + }); + + it("PKCE in auth but not in token should fail", async () => { + // Authorize WITH PKCE + const codeVerifier = generateRandomString(64); + const authUrl = await createAuthorizationURL({ + id: providerId, + options: { + clientId: confidentialClient.client_id, + clientSecret: confidentialClient.client_secret, + }, + redirectURI: redirectUri, + state: "123", + scopes: ["openid"], + responseType: "code", + codeVerifier, + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + }); + + let callbackUrl = ""; + await authenticatedClient.$fetch(authUrl.toString(), { + onError(context) { + callbackUrl = context.response.headers.get("Location") || ""; + }, + }); + + const url = resolveUrl(callbackUrl, authServerBaseUrl); + const code = url.searchParams.get("code"); + expect(code).toBeDefined(); + + // Try to exchange WITHOUT code_verifier (should fail) + const { body, headers } = createAuthorizationCodeRequest({ + code: code!, + redirectURI: redirectUri, + // Intentionally omit codeVerifier + options: { + clientId: confidentialClient.client_id, + clientSecret: confidentialClient.client_secret, + redirectURI: redirectUri, + }, + }); + + const tokenResponse = await authenticatedClient.$fetch("/oauth2/token", { + method: "POST", + body, + headers, + onError(context) { + expect(context.response.status).toBe(401); + }, + }); + + expect(tokenResponse.error).toBeDefined(); + expect((tokenResponse.error as any).error_description).toContain( + "code_verifier required because PKCE was used in authorization", + ); + }); + + it("PKCE not in auth but in token should fail", async () => { + // Authorize WITHOUT PKCE + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", confidentialClient.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "123"); + + let callbackUrl = ""; + await authenticatedClient.$fetch(authUrl.toString(), { + onError(context) { + callbackUrl = context.response.headers.get("Location") || ""; + }, + }); + + const url = resolveUrl(callbackUrl, authServerBaseUrl); + const code = url.searchParams.get("code"); + expect(code).toBeDefined(); + + // Try to exchange WITH code_verifier (should fail) + const wrongCodeVerifier = generateRandomString(64); + const { body, headers } = createAuthorizationCodeRequest({ + code: code!, + redirectURI: redirectUri, + codeVerifier: wrongCodeVerifier, + options: { + clientId: confidentialClient.client_id, + clientSecret: confidentialClient.client_secret, + redirectURI: redirectUri, + }, + }); + + const tokenResponse = await authenticatedClient.$fetch("/oauth2/token", { + method: "POST", + body, + headers, + onError(context) { + expect(context.response.status).toBe(401); + }, + }); + + expect(tokenResponse.error).toBeDefined(); + expect((tokenResponse.error as any).error_description).toContain( + "code_verifier provided but PKCE was not used in authorization", + ); + }); + + it("mismatched PKCE challenge should fail", async () => { + // Authorize with PKCE + const codeVerifier = generateRandomString(64); + const authUrl = await createAuthorizationURL({ + id: providerId, + options: { + clientId: confidentialClient.client_id, + clientSecret: confidentialClient.client_secret, + }, + redirectURI: redirectUri, + state: "123", + scopes: ["openid"], + responseType: "code", + codeVerifier, + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + }); + + let callbackUrl = ""; + await authenticatedClient.$fetch(authUrl.toString(), { + onError(context) { + callbackUrl = context.response.headers.get("Location") || ""; + }, + }); + + const url = resolveUrl(callbackUrl, authServerBaseUrl); + const code = url.searchParams.get("code"); + expect(code).toBeDefined(); + + // Try to exchange with WRONG code_verifier + const wrongVerifier = generateRandomString(64); + const { body, headers } = createAuthorizationCodeRequest({ + code: code!, + redirectURI: redirectUri, + codeVerifier: wrongVerifier, + options: { + clientId: confidentialClient.client_id, + clientSecret: confidentialClient.client_secret, + redirectURI: redirectUri, + }, + }); + + const tokenResponse = await authenticatedClient.$fetch<"string", "string">( + "/oauth2/token", + { + method: "POST", + body, + headers, + onError(context) { + expect(context.response.status).toBe(401); + }, + }, + ); + + expect(tokenResponse.error).toBeDefined(); + expect((tokenResponse.error as any).error_description).toContain( + "code verification failed", + ); + }); +}); + +describe("PKCE optional - registration restrictions", async () => { + const authServerBaseUrl = "http://localhost:3004"; + const rpBaseUrl = "http://localhost:5004"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + allowDynamicClientRegistration: true, + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const authenticatedClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: authServerBaseUrl, + fetchOptions: { + customFetchImpl, + headers, + }, + }); + + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + + it("admin create endpoint should persist require_pkce", async () => { + const pkceDisabledClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + require_pkce: false, + }, + }); + expect(pkceDisabledClient.require_pkce).toBe(false); + + const pkceRequiredClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + require_pkce: true, + }, + }); + expect(pkceRequiredClient.require_pkce).toBe(true); + }); + + it.each([ + ["dynamic registration", "/oauth2/register"], + ["non-admin create-client", "/oauth2/create-client"], + ])("should ignore require_pkce false (%s)", async (_, endpoint) => { + // require_pkce isn't a parameter for this endpoint, so it should be ignored and default to true. + // The client should be created successfully, but PKCE should still be required. + + const response = await authenticatedClient.$fetch(endpoint, { + method: "POST", + body: { + redirect_uris: [redirectUri], + require_pkce: false, + }, + }); + + expect(response.data?.client_id).toBeDefined(); + expect(response.data?.require_pkce).not.toBe(true); + + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", response.data!.client_id); + authUrl.searchParams.set("redirect_uri", redirectUri); + authUrl.searchParams.set("response_type", "code"); + authUrl.searchParams.set("scope", "openid"); + authUrl.searchParams.set("state", "123"); + + 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+is+required+for+this+client"); + }); +}); diff --git a/packages/oauth-provider/src/register.ts b/packages/oauth-provider/src/register.ts index 579d491483..334a816fb5 100644 --- a/packages/oauth-provider/src/register.ts +++ b/packages/oauth-provider/src/register.ts @@ -128,6 +128,13 @@ export async function checkOAuthClient( } } } + + if (settings?.isRegister && client.require_pkce === false) { + throw new APIError("BAD_REQUEST", { + error: "invalid_client_metadata", + error_description: `pkce is required for registered clients.`, + }); + } } export async function createOAuthClientEndpoint( @@ -251,6 +258,7 @@ export function oauthToSchema(input: OAuthClient): SchemaClient { disabled, skip_consent: skipConsent, enable_end_session: enableEndSession, + require_pkce: requirePKCE, reference_id: referenceId, metadata: inputMetadata, // All other metadata @@ -304,6 +312,7 @@ export function oauthToSchema(input: OAuthClient): SchemaClient { // All other metadata skipConsent, enableEndSession, + requirePKCE, referenceId, metadata, }; @@ -350,6 +359,7 @@ export function schemaToOAuth(input: SchemaClient): OAuthClient { // All other metadata skipConsent, enableEndSession, + requirePKCE, referenceId, metadata, // in JSON format } = input; @@ -402,6 +412,7 @@ export function schemaToOAuth(input: SchemaClient): OAuthClient { disabled: disabled ?? undefined, skip_consent: skipConsent ?? undefined, enable_end_session: enableEndSession ?? undefined, + require_pkce: requirePKCE ?? undefined, reference_id: referenceId ?? undefined, }; } diff --git a/packages/oauth-provider/src/schema.ts b/packages/oauth-provider/src/schema.ts index d372c7cf16..2359e3bf0c 100644 --- a/packages/oauth-provider/src/schema.ts +++ b/packages/oauth-provider/src/schema.ts @@ -116,6 +116,10 @@ export const schema = { type: "string", required: false, }, + requirePKCE: { + type: "boolean", + required: false, + }, // All other metadata referenceId: { type: "string", diff --git a/packages/oauth-provider/src/token.ts b/packages/oauth-provider/src/token.ts index d1b99493d1..70b1e5fc14 100644 --- a/packages/oauth-provider/src/token.ts +++ b/packages/oauth-provider/src/token.ts @@ -20,6 +20,7 @@ import { decryptStoredClientSecret, getJwtPlugin, getStoredToken, + isPKCERequired, parseClientMetadata, storeToken, validateClientCredentials, @@ -609,10 +610,9 @@ async function handleAuthorizationCodeGrant( const isAuthCodeWithSecret = client_id && client_secret; const isAuthCodeWithPkce = client_id && code && code_verifier; - if (!(isAuthCodeWithPkce || isAuthCodeWithSecret)) { + if (!isAuthCodeWithSecret && !isAuthCodeWithPkce) { throw new APIError("BAD_REQUEST", { - error_description: - "Missing a required credential value for authorization_code grant", + error_description: "Either code_verifier or client_secret is required", error: "invalid_request", }); } @@ -642,31 +642,70 @@ async function handleAuthorizationCodeGrant( scopes, ); - /** Check challenge */ - const challenge = - code_verifier && verificationValue.query?.code_challenge_method === "S256" - ? await generateCodeChallenge(code_verifier) - : undefined; - if ( - // AuthCodeWithSecret - Required if sent - isAuthCodeWithSecret && - (challenge || verificationValue?.query?.code_challenge) && - challenge !== verificationValue.query?.code_challenge - ) { - throw new APIError("UNAUTHORIZED", { - error_description: "code verification failed", - error: "invalid_request", - }); + // Parse scopes from the authorization request + const requestedScopes = + (verificationValue.query?.scope as string)?.split(" ") || []; + + // Check if PKCE is required for this client + const pkceRequired = isPKCERequired(client, requestedScopes); + + // Validate credentials based on requirements + if (pkceRequired) { + // PKCE is required - must have code_verifier + if (!isAuthCodeWithPkce) { + throw new APIError("BAD_REQUEST", { + error_description: "PKCE is required for this client", + error: "invalid_request", + }); + } + } else { + // PKCE is optional - must have either PKCE or client_secret + if (!(isAuthCodeWithPkce || isAuthCodeWithSecret)) { + throw new APIError("BAD_REQUEST", { + error_description: + "Either PKCE (code_verifier) or client authentication (client_secret) is required", + error: "invalid_request", + }); + } } - if ( - // AuthCodeWithPkce - Always required - isAuthCodeWithPkce && - challenge !== verificationValue.query?.code_challenge - ) { - throw new APIError("UNAUTHORIZED", { - error_description: "code verification failed", - error: "invalid_request", - }); + + /** Check PKCE challenge if verifier is provided */ + const pkceUsedInAuth = !!verificationValue.query?.code_challenge; + const pkceUsedInToken = !!code_verifier; + + if (pkceUsedInAuth || pkceUsedInToken) { + // PKCE was used - must verify consistency + + if (pkceUsedInAuth && !pkceUsedInToken) { + // PKCE was used in authorization but not in token exchange + throw new APIError("UNAUTHORIZED", { + error_description: + "code_verifier required because PKCE was used in authorization", + error: "invalid_request", + }); + } + + if (!pkceUsedInAuth && pkceUsedInToken) { + // PKCE was not used in authorization but verifier provided + throw new APIError("UNAUTHORIZED", { + error_description: + "code_verifier provided but PKCE was not used in authorization", + error: "invalid_request", + }); + } + + // Both sides used PKCE - verify the challenge + const challenge = + verificationValue.query?.code_challenge_method === "S256" + ? await generateCodeChallenge(code_verifier!) + : undefined; + + if (challenge !== verificationValue.query?.code_challenge) { + throw new APIError("UNAUTHORIZED", { + error_description: "code verification failed", + error: "invalid_request", + }); + } } /** Get user */ diff --git a/packages/oauth-provider/src/types/index.ts b/packages/oauth-provider/src/types/index.ts index 82756eb5c7..3938bf2a9d 100644 --- a/packages/oauth-provider/src/types/index.ts +++ b/packages/oauth-provider/src/types/index.ts @@ -892,6 +892,15 @@ export interface SchemaClient< * - user-agent-based - A user-agent-based application (public client) */ type?: "web" | "native" | "user-agent-based"; + /** + * Whether this client requires PKCE for authorization code flow. + * + * @default true + * + * Note: PKCE is always required for public clients and when + * requesting offline_access scope, regardless of this setting. + */ + requirePKCE?: boolean; //---- All other metadata ----// /** Used to indicate if consent screen can be skipped */ skipConsent?: boolean; diff --git a/packages/oauth-provider/src/types/oauth.ts b/packages/oauth-provider/src/types/oauth.ts index d16482df0f..760d25d77d 100644 --- a/packages/oauth-provider/src/types/oauth.ts +++ b/packages/oauth-provider/src/types/oauth.ts @@ -298,6 +298,15 @@ export interface OAuthClient { disabled?: boolean; skip_consent?: boolean; enable_end_session?: boolean; + /** + * Whether this client requires PKCE for authorization code flow. + * + * @default true + * + * Note: PKCE is always required for public clients and when + * requesting offline_access scope, regardless of this setting. + */ + require_pkce?: boolean; //---- All other metadata ----// reference_id?: string; [key: string]: unknown; diff --git a/packages/oauth-provider/src/utils/index.ts b/packages/oauth-provider/src/utils/index.ts index 5238b12596..e7e0987605 100644 --- a/packages/oauth-provider/src/utils/index.ts +++ b/packages/oauth-provider/src/utils/index.ts @@ -425,3 +425,50 @@ export function deleteFromPrompt(query: URLSearchParams, prompt: Prompt) { } return Object.fromEntries(query); } + +enum PKCERequirementErrors { + PUBLIC_CLIENT = "pkce is required for public clients", + OFFLINE_ACCESS_SCOPE = "pkce is required when requesting offline_access scope", + CLIENT_REQUIRE_PKCE = "pkce is required for this client", +} +/** + * Determines if PKCE is required for a given client and scope. + * + * PKCE is always required for: + * 1. Public clients (cannot securely store client_secret) + * 2. Requests with offline_access scope (refresh token security) + * + * For confidential clients without offline_access: + * - Uses client.requirePKCE if set (defaults to true) + * + * Returns false if PKCE is not required, or the reason it is required. + * + * @internal + */ +export function isPKCERequired( + client: SchemaClient, + requestedScopes?: string[], +): false | PKCERequirementErrors { + // Determine if client is public + const isPublicClient = + client.tokenEndpointAuthMethod === "none" || + client.type === "native" || + client.type === "user-agent-based" || + client.public === true; + + // PKCE always required for public clients + if (isPublicClient) { + return PKCERequirementErrors.PUBLIC_CLIENT; + } + + // PKCE always required for offline_access scope (refresh tokens) + if (requestedScopes?.includes("offline_access")) { + return PKCERequirementErrors.OFFLINE_ACCESS_SCOPE; + } + + if (client.requirePKCE ?? true) { + return PKCERequirementErrors.CLIENT_REQUIRE_PKCE; + } + + return false; +} diff --git a/packages/sso/src/index.ts b/packages/sso/src/index.ts index f8b8036b22..9b5c3d8ccd 100644 --- a/packages/sso/src/index.ts +++ b/packages/sso/src/index.ts @@ -147,6 +147,7 @@ export function sso( ): { id: "sso"; endpoints: SSOEndpoints; + options: O; }; export function sso(