Skip to content

Commit 207048e

Browse files
committed
fix(oidc): properly enforce consent requirements per OIDC spec (#4974)
1 parent 6a2883d commit 207048e

File tree

3 files changed

+190
-79
lines changed

3 files changed

+190
-79
lines changed

packages/better-auth/src/plugins/oidc-provider/authorize.ts

Lines changed: 42 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,34 @@ export async function authorize(
182182
const code = generateRandomString(32, "a-z", "A-Z", "0-9");
183183
const codeExpiresInMs = opts.codeExpiresIn * 1000;
184184
const expiresAt = new Date(Date.now() + codeExpiresInMs);
185+
186+
// Determine if consent is required
187+
// Consent is ALWAYS required unless:
188+
// 1. The client is trusted (skipConsent = true)
189+
// 2. The user has already consented and prompt is not "consent"
190+
const skipConsentForTrustedClient = client.skipConsent;
191+
const hasAlreadyConsented = await ctx.context.adapter
192+
.findOne<{
193+
consentGiven: boolean;
194+
}>({
195+
model: "oauthConsent",
196+
where: [
197+
{
198+
field: "clientId",
199+
value: client.clientId,
200+
},
201+
{
202+
field: "userId",
203+
value: session.user.id,
204+
},
205+
],
206+
})
207+
.then((res) => !!res?.consentGiven);
208+
209+
const requireConsent =
210+
!skipConsentForTrustedClient &&
211+
(!hasAlreadyConsented || query.prompt === "consent");
212+
185213
try {
186214
/**
187215
* Save the code in the database
@@ -195,19 +223,16 @@ export async function authorize(
195223
userId: session.user.id,
196224
authTime: new Date(session.session.createdAt).getTime(),
197225
/**
198-
* If the prompt is set to `consent`, then we need
199-
* to require the user to consent to the scopes.
200-
*
201-
* This means the code now needs to be treated as a
202-
* consent request.
226+
* Consent is required per OIDC spec unless:
227+
* 1. Client is trusted (skipConsent = true)
228+
* 2. User has already consented (and prompt is not "consent")
203229
*
204-
* once the user consents, the code will be updated
205-
* with the actual code. This is to prevent the
206-
* client from using the code before the user
207-
* consents.
230+
* When consent is required, the code needs to be treated as a
231+
* consent request. Once the user consents, the code will be
232+
* updated with the actual authorization code.
208233
*/
209-
requireConsent: query.prompt === "consent",
210-
state: query.prompt === "consent" ? query.state : null,
234+
requireConsent,
235+
state: requireConsent ? query.state : null,
211236
codeChallenge: query.code_challenge,
212237
codeChallengeMethod: query.code_challenge_method,
213238
nonce: query.nonce,
@@ -227,40 +252,15 @@ export async function authorize(
227252
);
228253
}
229254

230-
const redirectURIWithCode = new URL(redirectURI);
231-
redirectURIWithCode.searchParams.set("code", code);
232-
redirectURIWithCode.searchParams.set("state", ctx.query.state);
233-
234-
if (query.prompt !== "consent") {
255+
// If consent is not required, redirect with the code immediately
256+
if (!requireConsent) {
257+
const redirectURIWithCode = new URL(redirectURI);
258+
redirectURIWithCode.searchParams.set("code", code);
259+
redirectURIWithCode.searchParams.set("state", ctx.query.state);
235260
return handleRedirect(redirectURIWithCode.toString());
236261
}
237262

238-
// Check if this is a trusted client that should skip consent
239-
if (client.skipConsent) {
240-
return handleRedirect(redirectURIWithCode.toString());
241-
}
242-
243-
const hasAlreadyConsented = await ctx.context.adapter
244-
.findOne<{
245-
consentGiven: boolean;
246-
}>({
247-
model: "oauthConsent",
248-
where: [
249-
{
250-
field: "clientId",
251-
value: client.clientId,
252-
},
253-
{
254-
field: "userId",
255-
value: session.user.id,
256-
},
257-
],
258-
})
259-
.then((res) => !!res?.consentGiven);
260-
261-
if (hasAlreadyConsented) {
262-
return handleRedirect(redirectURIWithCode.toString());
263-
}
263+
// Consent is required - redirect to consent page or show consent HTML
264264

265265
if (options?.consentPage) {
266266
// Set cookie to support cookie-based consent flows

packages/better-auth/src/plugins/oidc-provider/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ export const oidcProvider = (options: OIDCOptions) => {
262262
return;
263263
}
264264
ctx.query = JSON.parse(cookie);
265-
ctx.query!.prompt = "consent";
265+
// Don't force prompt to "consent" - let the authorize function
266+
// determine if consent is needed based on OIDC spec requirements
266267
ctx.context.session = session;
267268
const response = await authorize(ctx, opts);
268269
return response;
@@ -320,7 +321,7 @@ export const oidcProvider = (options: OIDCOptions) => {
320321
method: "POST",
321322
body: z.object({
322323
accept: z.boolean(),
323-
consent_code: z.string().optional(),
324+
consent_code: z.string().optional().nullish(),
324325
}),
325326
use: [sessionMiddleware],
326327
metadata: {

0 commit comments

Comments
 (0)