Fix email auth confirmation to use token-hash verification#373
Fix email auth confirmation to use token-hash verification#373gianpaj wants to merge 13 commits into
Conversation
Add a dedicated /auth/confirm route that verifies Supabase email auth links with token_hash and verifyOtp, avoiding PKCE code-verifier failures when users open email links from another browser, device, or email client webview. Keep /auth/callback for OAuth code exchanges, share safe redirect handling between auth routes, and update signup/reset-password redirects to pass final same-origin destinations for Supabase email templates. Document the required Supabase dashboard email template changes and add Vitest and Playwright coverage for the confirmation route, email signup redirect target, malformed confirmation links, unsafe redirects, and verifyOtp failures.
|
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @gianpaj's task in 4m 18s —— View job Claude PR Review
SummarySolid refactor. The I found one regression, one deployment-coordination risk, and a handful of smaller items. Details on the relevant lines via inline comments — high-level summary below. Findings🔴 Regression — analytics + Stripe side effects dropped for email signup 🟡 Deployment-ordering risk — Supabase email templates must be updated before this code lands 🟡 Sentry telemetry hygiene — 🟢 Smaller items (inline)
Things I checked and confirmed are fine
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new /auth/confirm route to handle server-side email OTP verification using token_hash, which improves the reliability of email-based authentication flows across different environments. It centralizes redirect validation and locale detection into a shared utility, updates the signup and password reset actions, and includes new unit and E2E tests. Feedback suggests enhancing the redirect utility to preserve URL hash fragments, ensuring that deep links are maintained after the authentication process.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR updates the SSR auth flow so Supabase email confirmation/password recovery links are verified via token_hash + verifyOtp() on a dedicated /auth/confirm route (instead of relying on PKCE code exchange), while keeping /auth/callback focused on OAuth.
Changes:
- Added
/auth/confirmroute that verifiestoken_hashemail links and safely redirects to validated same-origin destinations. - Refactored safe redirect parsing + callback marker cookie setting into a shared
auth-redirecthelper and reused it in/auth/callback. - Updated signup/password-reset redirect targets, added Vitest + Playwright coverage, and documented required Supabase email template changes.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/devops.md | Documents new Supabase email template links using /auth/confirm with token_hash. |
| apps/web/tests/auth-confirm.test.ts | Adds unit coverage for /auth/confirm success/failure/unsafe redirect handling. |
| apps/web/proxy.ts | Ensures /auth/confirm bypasses proxy/session middleware handling like /auth/callback. |
| apps/web/lib/supabase/middleware.ts | Marks /auth/confirm as a public route. |
| apps/web/lib/supabase/auth-redirect.ts | Introduces shared safe-redirect + callback marker cookie redirect helper. |
| apps/web/e2e/E2E_TEST_PLAN.md | Updates E2E inventory to include email auth coverage. |
| apps/web/e2e/auth-email.spec.ts | Adds Playwright coverage ensuring signup uses final destinations (not /auth/callback) and malformed confirm links go to login. |
| apps/web/app/auth/confirm/route.ts | Implements token-hash verification via verifyOtp() and safe same-origin redirect logic. |
| apps/web/app/auth/callback/route.ts | Reuses shared redirect validation + marker cookie logic for OAuth callback flow. |
| apps/web/app/actions.ts | Updates password reset redirect target to a final same-origin destination (not /auth/callback). |
| apps/web/app/[lang]/(auth)/signup/signup-form.tsx | Updates emailRedirectTo to a final dashboard destination instead of /auth/callback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96af8b8ef3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve merge conflicts in signup form, auth callback route, and proxy: - signup-form: keep main's server-side /auth/signup fetch with next-intl - proxy: allow both /auth/confirm (email PKCE) and /auth/signup routes - auth/callback: combine main's OAuth replay short-circuit and error handling with branch AUTH_* marker naming and auth-redirect helpers - auth-callback tests: update imports to AUTH_* naming
* fix(auth): align email signup e2e with server-side signup route Signup moved to POST /auth/signup, so Playwright mocks of the browser-side Supabase auth/v1/signup endpoint no longer intercept anything. - Pass locale from the signup form to the server route - Set emailRedirectTo to the localized dashboard URL (not /auth/callback) - Short-circuit Supabase in E2E mode and return emailRedirectTo in the response - Update auth-email.spec.ts to assert the /auth/signup response payload * fix(auth): stop trusting Origin header for email redirect URLs Use NEXT_PUBLIC_SITE_URL as the primary origin for signup and password reset redirects, with request.url as a fallback in route handlers only. Reject signup when no trusted origin can be resolved. Extract getSiteUrlOrigin/getRequestOrigin helpers into auth-redirect and add unit tests covering spoofed Origin headers and invalid config. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a546dea297
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const user = data.user ?? (await supabase.auth.getUser()).data.user; | ||
|
|
||
| if (user) { | ||
| await recordSignupSideEffects(user, 'email'); |
There was a problem hiding this comment.
If recordSignupSideEffects() rejects here (for example a Stripe search/create/update failure or PostHog shutdown error), the route has already successfully consumed the one-time token_hash via verifyOtp() but will throw a 500 instead of redirecting the newly confirmed user. The same email link then cannot be retried successfully, so these non-critical provisioning/analytics failures should be caught/logged while still returning the auth redirect.
Useful? React with 👍 / 👎.
| Deployment order matters for these template changes: | ||
|
|
||
| 1. Update the Supabase dashboard Confirm signup and Reset password templates. | ||
| 2. Deploy the app version that serves `/auth/confirm` immediately after the | ||
| dashboard update. |
There was a problem hiding this comment.
With this ordering, any signup or reset email generated after the dashboard template change but before the new app version is deployed will point at /auth/confirm, which the old deployment does not serve, leaving users with broken one-time auth links. The safer rollout order is to deploy the app route first, then switch the Supabase templates, and reverse that order for rollback.
Useful? React with 👍 / 👎.
Summary
/auth/confirmroute that verifies Supabase email links withtoken_hashandverifyOtp()instead of PKCE code exchange./auth/callbackfor OAuth, while sharing safe redirect handling and callback marker cookie logic across auth flows.Testing
pnpm --filter @sexyvoice/web test -- tests/auth-confirm.test.ts tests/auth-callback.test.tspnpm --filter @sexyvoice/web exec playwright test e2e/auth-email.spec.ts --project=chromiumpnpm type-checkpnpm fixallran but still fails on existing repo-wide Biome issues unrelated to this change