Skip to content

feat(oauth): show account choices for returning users#36

Merged
sirily11 merged 2 commits into
mainfrom
copilot/add-e2e-tests-for-account-choose
Mar 11, 2026
Merged

feat(oauth): show account choices for returning users#36
sirily11 merged 2 commits into
mainfrom
copilot/add-e2e-tests-for-account-choose

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 11, 2026

  • Modify app/api/oauth/authorize/route.ts to add fresh_login=true param when redirecting unauthenticated users to login, and skip account selection when fresh_login=true is present
  • Modify actions/oauth/account-select.ts (switchAccount) to add fresh_login=true so switching accounts doesn't loop back to account select
  • Add E2E test: first-time sign-in during OAuth flow should NOT show account selection
  • Add E2E test: previously signed-in user starting OAuth flow SHOULD show account selection
  • Run all E2E tests to verify no regressions
  • Run code review and security checks

💡 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.

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 11, 2026

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

Project Deployment Actions Updated (UTC)
rxlab-auth Ready Ready Preview, Comment Mar 11, 2026 4:55pm

Request Review

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>
@sirily11 sirily11 marked this pull request as ready for review March 11, 2026 17:24
Copilot AI review requested due to automatic review settings March 11, 2026 17:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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=true to the login redirect for unauthenticated /api/oauth/authorize requests, and skip account selection when fresh_login is present.
  • Update switchAccount to include fresh_login=true to 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.

Comment on lines 117 to +119
const accountConfirmed = searchParams.get("account_confirmed");
if (!accountConfirmed) {
const freshLogin = searchParams.get("fresh_login");
if (!accountConfirmed && !freshLogin) {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

const searchParams = new URLSearchParams(oauthParams);
searchParams.set("fresh_login", "true");
const loginUrl = `/login?redirect=/api/oauth/authorize?${searchParams.toString()}`;
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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()}`;

Copilot uses AI. Check for mistakes.
test.describe("OAuth Account Selection", () => {
let clientId: string;

test.beforeAll(async ({ browser }, testInfo) => {
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
test.beforeAll(async ({ browser }, testInfo) => {
test.beforeAll(async ({ browser }) => {

Copilot uses AI. Check for mistakes.
@sirily11 sirily11 changed the title [WIP] Add e2e to show account choices for returning users Add e2e to show account choices for returning users Mar 11, 2026
@autopilot-project-manager autopilot-project-manager Bot changed the title Add e2e to show account choices for returning users feat(oauth): show account choices for returning users Mar 11, 2026
@sirily11 sirily11 merged commit e792acd into main Mar 11, 2026
7 checks passed
@sirily11 sirily11 deleted the copilot/add-e2e-tests-for-account-choose branch March 11, 2026 17:32
Copilot stopped work on behalf of sirily11 due to an error March 11, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants