fix: Safari onboarding + general redirection #920
Conversation
WalkthroughSwitches redirect/navigation to router-based refresh flows in onboarding and OTP verification, adds redirecting UI state, removes dashboard revalidation in onboarding API route, and relaxes NextAuth session cookie options (sameSite=lax, secure only in production). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant O as Onboarding.tsx
participant API as POST /api/settings/onboarding
participant R as Next.js Router
U->>O: Submit onboarding form
O->>API: POST onboarding data
API-->>O: Response (isMemberOfOrganization)
alt isMemberOfOrganization == true
O->>O: set isRedirecting = true
O->>R: router.refresh()
else not a member
O->>O: Show UpgradeModal
U->>O: Close modal (upgrade complete)
O->>O: set isRedirecting = true
O->>R: router.refresh()
end
sequenceDiagram
autonumber
actor U as User
participant F as verify-otp/form.tsx
participant R as Next.js Router
U->>F: Submit OTP
F-->>F: onSuccess handler
F->>R: router.refresh()
F->>R: router.replace(next || "/dashboard")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/(org)/verify-otp/form.tsx (1)
14-22: Sanitize the next parameter to prevent open-redirects.next is user-controllable and passed directly to router.replace(). If it contains an absolute URL or protocol-relative path (//evil.com), this becomes an open-redirect vector. Restrict navigation to same-origin relative paths.
Apply this diff to harden navigation:
onSuccess: () => { - router.refresh(); - router.replace(next || "/dashboard"); + router.refresh(); + const safeNext = + typeof next === "string" && + next.startsWith("/") && // must be a path + !next.startsWith("//") // disallow protocol-relative + ? next + : "/dashboard"; + router.replace(safeNext); },Also applies to: 89-92
🧹 Nitpick comments (7)
apps/web/app/(org)/verify-otp/form.tsx (3)
89-92: Avoid redundant/competing navigations (refresh + replace).Calling router.refresh() immediately followed by router.replace() can trigger extra work and, in some cases, drop the refresh. If the intent is “ensure session cookie is visible to RSC before navigating,” queue replace on the next tick.
Apply this refinement:
- router.refresh(); - router.replace(safeNext); + router.refresh(); + // allow refresh to commit; then navigate + setTimeout(() => router.replace(safeNext), 0);
171-196: Improve Safari/iOS OTP UX with one-time-code autocomplete.Adding autoComplete="one-time-code" helps Safari and iOS Mail auto-fill the 6‑digit code.
Apply this small change to the input:
<input key={index.toString()} ref={(el) => (inputRefs.current[index] = el)} type="text" inputMode="numeric" pattern="[0-9]*" + autoComplete="one-time-code" maxLength={1} value={digit}
73-88: Minor: Provide clearer error surface for network failures.If the fetch to /api/auth/callback/email fails (network error/throws), the mutation will throw a TypeError before res.url is evaluated. Consider catching network failures to show a specific toast.
Example:
mutationFn: async () => { const otpCode = code.join(""); if (otpCode.length !== 6) throw "Please enter a complete 6-digit code"; - const res = await fetch(`...`); + let res: Response; + try { + res = await fetch(`...`); + } catch { + throw "Network error. Please check your connection and try again."; + }packages/database/auth/auth-options.tsx (1)
118-121: Optional hardening: explicitly set sameSite on CSRF cookie as well (if customized elsewhere).If you have customized csrfToken cookie elsewhere, align its sameSite/secure with sessionToken to avoid mixed contexts. If not customized, ignore; NextAuth defaults are typically fine.
apps/web/app/(org)/onboarding/Onboarding.tsx (3)
42-44: Add a safety fallback to avoid indefinite “Redirecting…” if server-side redirect doesn’t occur.If the refreshed /onboarding route doesn’t immediately redirect (e.g., cache race), the UI remains disabled. Consider a short fallback to navigate to the dashboard.
Apply this minimal fallback:
- router.refresh(); - setIsRedirecting(true); + router.refresh(); + setIsRedirecting(true); + // Fallback in case the refreshed route doesn't redirect promptly + setTimeout(() => { + // optional: if you have a preferred post-onboarding path, use it here + router.replace("/dashboard"); + }, 4000);If you prefer not to force navigation, alternatively re-enable the form after a timeout.
Also applies to: 103-105
85-95: Button state logic is solid; minor copy improvement.Current labels are clear. If you add the fallback above, consider switching label to “Finishing up…” during the grace window to better set expectations.
17-31: Optional: make onboardingRequest non-caching explicit (defensive).POSTs aren’t cached by default, but being explicit helps when proxies/CDNs are involved.
You can add: fetch("/api/settings/onboarding", { method: "POST", cache: "no-store", ... })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/web/app/(org)/onboarding/Onboarding.tsx(6 hunks)apps/web/app/(org)/verify-otp/form.tsx(1 hunks)apps/web/app/api/settings/onboarding/route.ts(0 hunks)packages/database/auth/auth-options.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/app/api/settings/onboarding/route.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (rust)
🔇 Additional comments (4)
apps/web/app/(org)/verify-otp/form.tsx (1)
102-140: Client-only rate limit is bypassable; align message on actual server policy.The 30s resend check is only client-side and message assumes a server throttle. Ensure the server also enforces this policy to avoid email abuse; if it already does, consider surfacing server-provided backoff seconds.
Manual check:
- Try rapid consecutive POSTs to the NextAuth email sign-in endpoint and verify the server returns a consistent “EmailSignin” error with a 30s enforced gap.
- Confirm Safari behaves the same as Chrome after this change.
packages/database/auth/auth-options.tsx (1)
118-121: Cookie policy change looks good for Safari; verify OAuth and magic-link flows end-to-end.Switching sameSite to "lax" and secure to production-only should mitigate Safari issues while keeping dev convenient.
Please verify across browsers/environments:
- Email magic link on Safari (macOS and iOS): code entry -> session established -> RSC sees session without a full reload.
- Google OAuth callback (top-level GET): with sameSite="lax", the session cookie should be sent; ensure no regression vs. previous "none".
- Staging/preview under HTTPS: confirm NODE_ENV is "production" so Secure is set.
apps/web/app/(org)/onboarding/Onboarding.tsx (2)
42-44: Router-based refresh + redirecting state is the right approach here.This aligns with App Router best practices and should help Safari observe the new session without a hard navigation.
39-44: Verify server-side gating truly redirects post-onboarding.The flow assumes that after router.refresh() the /onboarding route will detect completion and redirect server-side (middleware or loader). Please confirm that:
- The GET of /onboarding after completion redirects to /dashboard.
- The revalidation logic on apps/web/app/api/settings/onboarding/route.ts indeed invalidates /onboarding and not just /dashboard (per PR description).
Suggested manual test plan:
- Fresh user -> complete onboarding -> expect spinner “Redirecting…” then land on /dashboard without a full reload on Safari and Chrome.
- Open UpgradeModal path -> close modal -> same behavior as above.
- Rapid double-submit is benign (only one network request).
Also applies to: 98-105
This PR: Fixes auth signup on Safari + onboarding redirection improved.
Summary by CodeRabbit