feat(oauth): show account choices for returning users#36
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Add fresh_login parameter to skip account selection when user just logged in as part of the OAuth flow. Show account selection only when user was already signed in before starting the OAuth flow. - Modified authorize route to add fresh_login=true when redirecting to login - Modified authorize route to skip account selection when fresh_login=true - Modified switchAccount to add fresh_login=true after switching accounts - Added E2E tests for both scenarios Co-authored-by: sirily11 <32106111+sirily11@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the OAuth authorization flow to better control when the account-selection screen is shown, and adds E2E coverage to validate the intended UX for first-time vs returning users.
Changes:
- Add
fresh_login=trueto the login redirect for unauthenticated/api/oauth/authorizerequests, and skip account selection whenfresh_loginis present. - Update
switchAccountto includefresh_login=trueto avoid looping back through account selection after switching accounts. - Add Playwright E2E tests covering “first-time sign-in skips account selection” and “already signed-in shows account selection”.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| e2e/oauth/account-select.spec.ts | Adds E2E coverage for account selection behavior in OAuth flows. |
| app/api/oauth/authorize/route.ts | Introduces fresh_login handling to skip account selection after a fresh login. |
| actions/oauth/account-select.ts | Adds fresh_login when redirecting to login during account switching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const accountConfirmed = searchParams.get("account_confirmed"); | ||
| if (!accountConfirmed) { | ||
| const freshLogin = searchParams.get("fresh_login"); | ||
| if (!accountConfirmed && !freshLogin) { |
There was a problem hiding this comment.
fresh_login/account_confirmed are treated as truthy based on presence, so fresh_login=false (or any non-empty value) will still skip account selection. Consider checking for explicit values (e.g. === "true") and/or removing/ignoring untrusted fresh_login values to prevent clients/users from bypassing the account confirmation step by adding the param themselves.
|
|
||
| const searchParams = new URLSearchParams(oauthParams); | ||
| searchParams.set("fresh_login", "true"); | ||
| const loginUrl = `/login?redirect=/api/oauth/authorize?${searchParams.toString()}`; |
There was a problem hiding this comment.
The redirect query param here is not URL-encoded, so the & characters from searchParams.toString() will be parsed as separate /login query params and redirect will be truncated. Build the login URL with new URL() / URLSearchParams or at least encodeURIComponent(/api/oauth/authorize?${...}) when setting redirect.
| const loginUrl = `/login?redirect=/api/oauth/authorize?${searchParams.toString()}`; | |
| const redirectUrl = `/api/oauth/authorize?${searchParams.toString()}`; | |
| const loginParams = new URLSearchParams({ redirect: redirectUrl }); | |
| const loginUrl = `/login?${loginParams.toString()}`; |
| test.describe("OAuth Account Selection", () => { | ||
| let clientId: string; | ||
|
|
||
| test.beforeAll(async ({ browser }, testInfo) => { |
There was a problem hiding this comment.
testInfo is accepted by this beforeAll callback but never used. With the Next.js TypeScript ESLint config, unused variables can fail lint; consider removing testInfo (or prefixing with _ if you intend to keep it).
| test.beforeAll(async ({ browser }, testInfo) => { | |
| test.beforeAll(async ({ browser }) => { |
app/api/oauth/authorize/route.tsto addfresh_login=trueparam when redirecting unauthenticated users to login, and skip account selection whenfresh_login=trueis presentactions/oauth/account-select.ts(switchAccount) to addfresh_login=trueso switching accounts doesn't loop back to account select💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.