fix(auth): handle magic-link implicit flow so users land signed in#135
Merged
Conversation
When signInWithOtp is called server-side with a service-role client,
Supabase's default {{ .ConfirmationURL }} email template falls back to
the implicit flow and delivers tokens in a URL hash fragment. The
server-side /auth/callback route can't see hash fragments, so it was
bouncing users to /login?error=auth_failed. Session cookies ended up
getting set eventually (via the client-side SDK auto-detecting the
hash on a later page), which is why a refresh "worked" — but the
initial navigation was broken.
Make /auth/callback tolerant of all three email-template shapes:
- ?code=... (PKCE)
- ?token_hash=... (OTP verify, the canonical template)
- #access_token=... (implicit)
On the hash path, redirect to a new /auth/callback/hash client bridge
that POSTs the tokens to /api/auth/set-session, which calls
supabase.auth.setSession() and writes the session cookies atomically
on the response so middleware sees the session on the next request.
Also:
- Extract invite processing (team_members upsert, channel joins,
invitation acceptance, profile backfill, player/parent linking)
into lib/auth/process-invite.ts so both the server-exchange path
and the hash-exchange path run the same logic.
- Simplify legacy /callback page to a thin redirect to the canonical
handlers.
- Retarget HashRedirect at /auth/callback/hash and preserve the
original query string alongside the hash.
- Document the recommended Supabase Dashboard email templates using
{{ .TokenHash }} so the cleanest (server-only) flow can be used
once the dashboard is configured.
https://claude.ai/code/session_01BWDx8ESjArdxZwWKj667Sd
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughRefactored authentication flow to support implicit token delivery via URL hash fragments. Extracted invite-processing logic into a reusable module, added a new API route to exchange tokens for session cookies, and introduced a client-side hash-bridge page for implicit auth flows while simplifying the legacy callback handler. Changes
Sequence DiagramsequenceDiagram
actor User as Browser / User
participant HashBridge as /auth/callback/hash
participant SetSession as /api/auth/set-session
participant Supabase as Supabase Auth
participant DB as Database
User->>HashBridge: GET with URL#access_token=...<br/>&refresh_token=...
activate HashBridge
HashBridge->>HashBridge: Parse hash for tokens<br/>Read query params (next, team, role, etc.)
Note over HashBridge: On mount, extract from window.location
HashBridge->>SetSession: POST /api/auth/set-session<br/>{ access_token, refresh_token, ..metadata }
activate SetSession
SetSession->>SetSession: Validate token shape<br/>(JWT regex + presence checks)
alt Validation Fails
SetSession-->>HashBridge: 400 / 401 Error
HashBridge-->>User: Redirect /login?error=auth_failed
else Validation Succeeds
SetSession->>Supabase: supabase.auth.setSession(tokens)
Supabase-->>SetSession: User { id, email, user_metadata }
SetSession->>DB: processInvite(user, params)<br/>- Upsert team_members<br/>- Join channels<br/>- Accept invitations<br/>- Link players/parents
DB-->>SetSession: Invite processing complete
SetSession-->>HashBridge: { ok: true }
deactivate SetSession
HashBridge-->>User: Redirect to 'next'<br/>(default /dashboard)
end
deactivate HashBridge
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/auth/callback/route.ts (1)
27-32:⚠️ Potential issue | 🟡 MinorType cast is missing
'invite'— add it and align withEmailOtpType.The docs (
magic-link-email-template.md) and theinvite-memberedge function both usetype=invite, but the cast at lines 27–32 excludes it, narrowing to'magiclink' | 'email' | 'recovery' | 'signup' | null. The code works at runtime (the cast is erased andsupabase.auth.verifyOtpaccepts'invite'), but the TypeScript type is inaccurate, creating a trap for future refactors (e.g., adding aswitchontypewould make the'invite'case appear unreachable to the type checker).Import
EmailOtpTypefrom@supabase/supabase-jsand validate at runtime against an allowlist of acceptable types:🔧 Proposed change
-import { createServerClient, type CookieOptions } from '@supabase/ssr'; +import { createServerClient, type CookieOptions } from '@supabase/ssr'; +import type { EmailOtpType } from '@supabase/supabase-js'; ... - const tokenHash = searchParams.get('token_hash'); - const type = searchParams.get('type') as - | 'magiclink' - | 'email' - | 'recovery' - | 'signup' - | null; + const tokenHash = searchParams.get('token_hash'); + const rawType = searchParams.get('type'); + const ALLOWED_OTP_TYPES: readonly EmailOtpType[] = [ + 'magiclink', + 'invite', + 'signup', + 'recovery', + 'email', + 'email_change', + ]; + const type: EmailOtpType | null = + rawType && (ALLOWED_OTP_TYPES as readonly string[]).includes(rawType) + ? (rawType as EmailOtpType) + : null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/auth/callback/route.ts` around lines 27 - 32, The TypeScript cast for the `type` extracted from `searchParams` omits the 'invite' variant and should align with supabase's EmailOtpType; import EmailOtpType from '@supabase/supabase-js', change the `type` declaration to string | null (or a looser type) and then validate it at runtime against an allowlist built from EmailOtpType (e.g., Object.values(EmailOtpType)) before calling supabase.auth.verifyOtp; ensure the validated value (e.g., `validatedType`) is narrowed to the EmailOtpType union for downstream use so 'invite' is accepted and the type checker no longer flags unreachable cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/api/auth/set-session/route.ts`:
- Around line 59-67: Add a short inline comment next to the error branch after
supabase.auth.setSession (the block that logs and returns NextResponse.json({
error: 'Session exchange failed' }, { status: 401 })) explaining that we
intentionally return a brand-new NextResponse rather than reusing the pre-built
response object to avoid leaking any partially-set cookies; reference
supabase.auth.setSession, the error path, and NextResponse.json so future
readers don't "optimize" by reusing the original response.
- Around line 17-37: The POST handler in route.ts should enforce an
origin/same-site check before converting tokens to session cookies: read
request.headers.get('origin') and request.headers.get('sec-fetch-site') inside
the POST function and verify origin equals process.env.NEXT_PUBLIC_APP_URL ||
process.env.APP_URL (or that sec-fetch-site === 'same-origin'); if the check
fails return a 403 JSON response and do not set cookies. Implement this check
early in the POST function (before parsing/validating tokens and before any
cookie-setting code) and use those header names and the POST function identifier
to locate where to add it.
In `@apps/web/src/app/auth/callback/hash/page.tsx`:
- Around line 80-92: The Suspense wrapper around HashBridge in HashBridgePage is
unnecessary because HashBridge does not suspend; remove the Suspense import and
the Suspense JSX wrapper (including the fallback) and return <HashBridge />
directly from the HashBridgePage component; update the HashBridgePage function
to render HashBridge without Suspense and delete any unused Suspense import to
keep imports clean.
In `@apps/web/src/lib/auth/process-invite.ts`:
- Around line 66-77: Replace the two-step update + select with a single chained
update that returns the updated row: call db.from('team_invitations').update({
status: 'accepted', accepted_at: new Date().toISOString() }).eq('team_id',
teamId).eq('email', emailLower).eq('status', 'pending').select('first_name,
last_name').maybeSingle() so you get the first_name/last_name in the same
round-trip and only set accepted_at when the row was previously pending.
- Around line 105-112: The current code loops over playersParam and calls
db.from('parent_player_links').upsert for each pid causing N round-trips;
instead build an array of rows from
playersParam.split(',').filter(Boolean).map(pid => ({ parent_user_id: user.id,
player_id: pid })) and call db.from('parent_player_links').upsert(rows, {
onConflict: 'parent_user_id,player_id', ignoreDuplicates: true }) once (ensure
you only call when rows.length > 0 and remove the for loop).
- Around line 43-47: The createClient call that constructs the service-role
Supabase client (createClient(..., serviceKey)) should be passed auth options to
disable session persistence; update the call that creates the SupabaseClient to
add a third argument: { auth: { persistSession: false, autoRefreshToken: false }
} so the service-role client does not attempt to persist or auto-refresh user
sessions (refer to the createClient invocation that uses serviceKey).
In `@docs/auth/magic-link-email-template.md`:
- Around line 24-31: The cast for the request query param variable named type
currently uses the union 'magiclink' | 'email' | 'recovery' | 'signup' | null
but omits the 'invite' EmailOtpType used by the invite template; update the cast
where type is extracted in the auth callback handler (the variable named type in
the callback route) to include 'invite' so it becomes 'magiclink' | 'email' |
'recovery' | 'signup' | 'invite' | null.
---
Outside diff comments:
In `@apps/web/src/app/auth/callback/route.ts`:
- Around line 27-32: The TypeScript cast for the `type` extracted from
`searchParams` omits the 'invite' variant and should align with supabase's
EmailOtpType; import EmailOtpType from '@supabase/supabase-js', change the
`type` declaration to string | null (or a looser type) and then validate it at
runtime against an allowlist built from EmailOtpType (e.g.,
Object.values(EmailOtpType)) before calling supabase.auth.verifyOtp; ensure the
validated value (e.g., `validatedType`) is narrowed to the EmailOtpType union
for downstream use so 'invite' is accepted and the type checker no longer flags
unreachable cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6e8745a5-0409-4bde-b4a7-c4dfe1f1f411
📒 Files selected for processing (7)
apps/web/src/app/(auth)/callback/page.tsxapps/web/src/app/api/auth/set-session/route.tsapps/web/src/app/auth/callback/hash/page.tsxapps/web/src/app/auth/callback/route.tsapps/web/src/components/auth/HashRedirect.tsxapps/web/src/lib/auth/process-invite.tsdocs/auth/magic-link-email-template.md
…nion - Add origin/same-site check on POST /api/auth/set-session to prevent cross-origin session fixation via token replay. - Comment the error path to explain why a fresh NextResponse is used instead of the pre-built response (avoids leaking partial cookies). - Remove unnecessary Suspense wrapper from HashBridge page. - Combine two-step update+select on team_invitations into a single round-trip that also guards on status='pending'. - Batch parent_player_links upsert into a single call instead of N. - Pass persistSession:false/autoRefreshToken:false to the service-role client in processInvite so it never tries to store or refresh sessions. - Widen the OTP type union in /auth/callback to include 'invite' and 'email_change' (imported as EmailOtpType) and validate at runtime against an allowlist so unknown types fall through to the hash bridge. https://claude.ai/code/session_01BWDx8ESjArdxZwWKj667Sd
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When signInWithOtp is called server-side with a service-role client,
Supabase's default {{ .ConfirmationURL }} email template falls back to
the implicit flow and delivers tokens in a URL hash fragment. The
server-side /auth/callback route can't see hash fragments, so it was
bouncing users to /login?error=auth_failed. Session cookies ended up
getting set eventually (via the client-side SDK auto-detecting the
hash on a later page), which is why a refresh "worked" — but the
initial navigation was broken.
Make /auth/callback tolerant of all three email-template shapes:
On the hash path, redirect to a new /auth/callback/hash client bridge
that POSTs the tokens to /api/auth/set-session, which calls
supabase.auth.setSession() and writes the session cookies atomically
on the response so middleware sees the session on the next request.
Also:
invitation acceptance, profile backfill, player/parent linking)
into lib/auth/process-invite.ts so both the server-exchange path
and the hash-exchange path run the same logic.
handlers.
original query string alongside the hash.
{{ .TokenHash }} so the cleanest (server-only) flow can be used
once the dashboard is configured.
https://claude.ai/code/session_01BWDx8ESjArdxZwWKj667Sd
Summary by CodeRabbit
Release Notes
New Features
Refactor
Documentation