fix: require consent on account creation

[Kzoeps]

Previously the condition to show the consent page was if !newAccount but this doesnt make sense since its still good to display what perms the app is asking.

Also the stock pds also displays the consent page even if theyre creating a new account

Summary by CodeRabbit

• Refactor
  • Simplified consent requirement logic for client authentication flows

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
epds-demo	Ready	Preview, Comment	Mar 11, 2026 7:22am

[coderabbitai]

📝 Walkthrough

Walkthrough

Two files were modified to simplify consent logic by removing isNewAccount checks. The needsConsent calculation now relies solely on clientId presence and previous login history, and an intermediate constant in the callback flow was eliminated.

Changes

Cohort / File(s)	Summary
Consent Logic Simplification packages/auth-service/src/routes/complete.ts	Simplified needsConsent calculation by removing the isNewAccount condition; consent now required when clientId exists and user hasn't previously logged in with that client.
Callback Flow Cleanup packages/pds-core/src/index.ts	Removed the intermediate _isNewAccount constant; subsequent logic continues to use newAccountStr directly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 The consent check grows lean and bright,
No more account-age in the light,
Just clientId and history true,
Simplicity hops right on through!
What once was complex, now is clean—
The clearest logic ever seen! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: require consent on account creation' directly aligns with the main change: removing the isNewAccount check so that consent is now required for all new accounts with a clientId, not exempting new accounts from the consent flow.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/require-consent-on-signup

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/auth-service/src/routes/complete.ts (1)

102-109: ⚠️ Potential issue | 🟠 Major

Preserve the real new-account flag when redirecting to consent.

Line 108 now forces new=0 for every consent flow, so brand-new signups will render the existing-account consent UI. packages/auth-service/src/routes/consent.ts:27-88 derives isNew from that query param, and packages/auth-service/src/routes/consent.ts:180-233 uses it for the page title, the “A new account will be created” notice, and the hidden is_new field. We should keep the stricter needsConsent logic, but still pass through isNewAccount here.

Suggested fix

       consentUrl.searchParams.set('flow_id', flowId)
       consentUrl.searchParams.set('email', email)
-      consentUrl.searchParams.set('new', '0')
+      consentUrl.searchParams.set('new', isNewAccount ? '1' : '0')
       res.redirect(303, consentUrl.pathname + consentUrl.search)
       return

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/auth-service/src/routes/complete.ts` around lines 102 - 109, The
redirect to the consent page currently forces consentUrl.searchParams.set('new',
'0'), which overwrites the real new-account flag; instead use the existing
isNewAccount value when building the consentUrl (e.g., set 'new' to isNewAccount
? '1' : '0' or the string form used elsewhere) so the consent handler (isNew
derived from that query param) receives the correct flag; update the
construction that sets 'new' to reference the isNewAccount variable (leave
flowId, email, and needsConsent logic unchanged) and then call res.redirect with
consentUrl.pathname + consentUrl.search as before.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/auth-service/src/routes/complete.ts`:
- Around line 102-109: The redirect to the consent page currently forces
consentUrl.searchParams.set('new', '0'), which overwrites the real new-account
flag; instead use the existing isNewAccount value when building the consentUrl
(e.g., set 'new' to isNewAccount ? '1' : '0' or the string form used elsewhere)
so the consent handler (isNew derived from that query param) receives the
correct flag; update the construction that sets 'new' to reference the
isNewAccount variable (leave flowId, email, and needsConsent logic unchanged)
and then call res.redirect with consentUrl.pathname + consentUrl.search as
before.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 990c6a60-ebe8-4304-a065-b42e60b30c63

📥 Commits

Reviewing files that changed from the base of the PR and between 9351243 and 1371671.

📒 Files selected for processing (2)

• packages/auth-service/src/routes/complete.ts
• packages/pds-core/src/index.ts

💤 Files with no reviewable changes (1)

• packages/pds-core/src/index.ts

[aspiers]

@Kzoeps This will be superseded by #21 since the consent was broken anyway. So I think we can close this as requiring consent to the wrong things is as bad or perhaps even worse than not asking for any consent 😅