Fix team invite redirect being lost after email verification#457
Fix team invite redirect being lost after email verification#457AnthonyRonning merged 2 commits intomasterfrom
Conversation
- Add VerificationModal to team invite page so users can verify email without losing their place in the invite flow - Store post_auth_redirect in sessionStorage before OAuth redirects (GitHub/Google) so the invite URL survives the round-trip - After email verification (modal or link), redirect to the stored URL instead of always going to / - Read post_auth_redirect in OAuth callback to navigate to the correct page after authentication Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying maple with
|
| Latest commit: |
128ea40
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://834f7aad.maple-ca8.pages.dev |
| Branch Preview URL: | https://devin-1772724946-fix-team-in.maple-ca8.pages.dev |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes implement persistent post-auth redirect handling across the OAuth and email verification flows. When users initiate authentication, intended redirect paths are stored in sessionStorage before OAuth redirection, then retrieved and applied after authentication and verification complete. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser
participant SessionStorage
participant OAuth as OAuth Provider
participant Backend
User->>Browser: Click "Sign up with OAuth"<br/>(with next=/invite/123)
Browser->>SessionStorage: Store post_auth_redirect = "/invite/123"
Browser->>OAuth: Redirect to OAuth provider
User->>OAuth: Complete OAuth flow
OAuth->>Browser: Redirect to auth callback
Browser->>SessionStorage: Read post_auth_redirect
Browser->>Backend: Verify user session
Backend->>Browser: Return user data
Browser->>Browser: Navigate to /invite/123
Browser->>User: Render invite page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/routes/auth.$provider.callback.tsx (1)
65-66: Harden redirect consumption with an internal-path guard.Recommend validating
post_auth_redirectagain before navigation so stale/invalid storage values can’t break post-auth routing.Proposed hardening
const postAuthRedirect = sessionStorage.getItem("post_auth_redirect"); sessionStorage.removeItem("post_auth_redirect"); + const safePostAuthRedirect = + postAuthRedirect && postAuthRedirect.startsWith("/") && !postAuthRedirect.startsWith("//") + ? postAuthRedirect + : null; @@ - } else if (postAuthRedirect) { - navigate({ to: postAuthRedirect }); + } else if (safePostAuthRedirect) { + navigate({ to: safePostAuthRedirect }); } else { navigate({ to: "/" }); }Also applies to: 74-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/routes/auth`.$provider.callback.tsx around lines 65 - 66, The post-auth redirect consumption should validate that the sessionStorage value is a safe internal path before navigating: when reading post_auth_redirect into the postAuthRedirect variable (and the similar use at the other occurrence), check it's a non-empty string and matches an internal-path guard (e.g., begins with "/" and does not contain "//" or an absolute origin, or match your app's allowed path regex) and only then call navigate; still remove the sessionStorage item regardless to avoid reuse. Update the code around postAuthRedirect (and the repeated block) to perform this validation and skip/redirect to a safe default if validation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/routes/login.tsx`:
- Around line 142-144: The code stores a user-controlled query param `next` into
sessionStorage as "post_auth_redirect" without validation; update both places
where sessionStorage.setItem("post_auth_redirect", next) is called to validate
`next` first: allow only safe internal paths (e.g., ensure `next` is a non-empty
string that starts with "/" but not "//", does not include a full origin, and
resolves to the same origin if parsed via new URL), otherwise do not persist it
(or fall back to "/"). Locate the occurrences using the `next` variable and the
exact call sessionStorage.setItem("post_auth_redirect", next) in this file and
replace with the validation logic described.
In `@frontend/src/routes/signup.tsx`:
- Around line 153-155: The code stores the raw URL param `next` into
sessionStorage via sessionStorage.setItem("post_auth_redirect", next) without
validating it's an internal path; mirror the login logic by validating `next`
with the same internal-path check used elsewhere (e.g., the helper used in the
login flow) before calling sessionStorage.setItem, and apply this fix to both
occurrences around the sessionStorage.setItem lines (the block with if (next) {
sessionStorage.setItem("post_auth_redirect", next); } and the similar block at
lines ~198-200) so only safe internal redirects are persisted.
In `@frontend/src/routes/verify`.$code.tsx:
- Around line 25-36: The setTimeout callback contains async work (await
refetchUser() and navigate) which escapes the surrounding try/catch and can
produce unhandled rejections; extract the async flow into a named async helper
(e.g., handlePostAuthRedirect or similar) that performs await refetchUser(),
reads sessionStorage ("post_auth_redirect"), removes it, and calls navigate(...)
inside its own try/catch, then invoke it from setTimeout using a non-async arrow
(e.g., setTimeout(() => void handlePostAuthRedirect(), 2000)) or call
handlePostAuthRedirect().catch(err => {/* log/handle error */}) so all errors
are caught and logged.
---
Nitpick comments:
In `@frontend/src/routes/auth`.$provider.callback.tsx:
- Around line 65-66: The post-auth redirect consumption should validate that the
sessionStorage value is a safe internal path before navigating: when reading
post_auth_redirect into the postAuthRedirect variable (and the similar use at
the other occurrence), check it's a non-empty string and matches an
internal-path guard (e.g., begins with "/" and does not contain "//" or an
absolute origin, or match your app's allowed path regex) and only then call
navigate; still remove the sessionStorage item regardless to avoid reuse. Update
the code around postAuthRedirect (and the repeated block) to perform this
validation and skip/redirect to a safe default if validation fails.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53d93b35-cfb8-44c0-8b02-bbeb8c93632c
📒 Files selected for processing (6)
frontend/src/components/VerificationModal.tsxfrontend/src/routes/auth.$provider.callback.tsxfrontend/src/routes/login.tsxfrontend/src/routes/signup.tsxfrontend/src/routes/team.invite.$inviteId.tsxfrontend/src/routes/verify.$code.tsx
frontend/src/routes/login.tsx
Outdated
| if (next) { | ||
| sessionStorage.setItem("post_auth_redirect", next); | ||
| } |
There was a problem hiding this comment.
Validate next before persisting it as post_auth_redirect.
This value is user-controlled (query param). Persisting it without an internal-path check can produce unsafe or malformed redirects later.
Proposed fix
+ const isSafeRedirect = (value: string) => value.startsWith("/") && !value.startsWith("//");
+
- if (next) {
+ if (next && isSafeRedirect(next)) {
sessionStorage.setItem("post_auth_redirect", next);
}Also applies to: 187-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/routes/login.tsx` around lines 142 - 144, The code stores a
user-controlled query param `next` into sessionStorage as "post_auth_redirect"
without validation; update both places where
sessionStorage.setItem("post_auth_redirect", next) is called to validate `next`
first: allow only safe internal paths (e.g., ensure `next` is a non-empty string
that starts with "/" but not "//", does not include a full origin, and resolves
to the same origin if parsed via new URL), otherwise do not persist it (or fall
back to "/"). Locate the occurrences using the `next` variable and the exact
call sessionStorage.setItem("post_auth_redirect", next) in this file and replace
with the validation logic described.
frontend/src/routes/signup.tsx
Outdated
| if (next) { | ||
| sessionStorage.setItem("post_auth_redirect", next); | ||
| } |
There was a problem hiding this comment.
Apply the same internal-path validation for next before storing redirect state.
Same concern as login: next comes from URL search params and should be constrained before persisting as post_auth_redirect.
Proposed fix
+ const isSafeRedirect = (value: string) => value.startsWith("/") && !value.startsWith("//");
+
- if (next) {
+ if (next && isSafeRedirect(next)) {
sessionStorage.setItem("post_auth_redirect", next);
}Also applies to: 198-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/routes/signup.tsx` around lines 153 - 155, The code stores the
raw URL param `next` into sessionStorage via
sessionStorage.setItem("post_auth_redirect", next) without validating it's an
internal path; mirror the login logic by validating `next` with the same
internal-path check used elsewhere (e.g., the helper used in the login flow)
before calling sessionStorage.setItem, and apply this fix to both occurrences
around the sessionStorage.setItem lines (the block with if (next) {
sessionStorage.setItem("post_auth_redirect", next); } and the similar block at
lines ~198-200) so only safe internal redirects are persisted.
| setTimeout(async () => { | ||
| await refetchUser(); | ||
| navigate({ to: "/" }); | ||
|
|
||
| // Check for a pending redirect (e.g. team invite page) | ||
| const pendingRedirect = sessionStorage.getItem("post_auth_redirect"); | ||
| if (pendingRedirect) { | ||
| sessionStorage.removeItem("post_auth_redirect"); | ||
| navigate({ to: pendingRedirect }); | ||
| } else { | ||
| navigate({ to: "/" }); | ||
| } | ||
| }, 2000); |
There was a problem hiding this comment.
Avoid async side effects inside setTimeout here.
refetchUser()/redirect failures inside the timer bypass the outer try/catch, so failures become unhandled and the flow can silently stall.
Proposed fix
- // Do both refetch and navigation after a delay
- setTimeout(async () => {
- await refetchUser();
-
- // Check for a pending redirect (e.g. team invite page)
- const pendingRedirect = sessionStorage.getItem("post_auth_redirect");
- if (pendingRedirect) {
- sessionStorage.removeItem("post_auth_redirect");
- navigate({ to: pendingRedirect });
- } else {
- navigate({ to: "/" });
- }
- }, 2000);
+ await new Promise((resolve) => setTimeout(resolve, 2000));
+ await refetchUser();
+
+ const pendingRedirect = sessionStorage.getItem("post_auth_redirect");
+ if (pendingRedirect) {
+ sessionStorage.removeItem("post_auth_redirect");
+ navigate({ to: pendingRedirect });
+ } else {
+ navigate({ to: "/" });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/routes/verify`.$code.tsx around lines 25 - 36, The setTimeout
callback contains async work (await refetchUser() and navigate) which escapes
the surrounding try/catch and can produce unhandled rejections; extract the
async flow into a named async helper (e.g., handlePostAuthRedirect or similar)
that performs await refetchUser(), reads sessionStorage ("post_auth_redirect"),
removes it, and calls navigate(...) inside its own try/catch, then invoke it
from setTimeout using a non-async arrow (e.g., setTimeout(() => void
handlePostAuthRedirect(), 2000)) or call handlePostAuthRedirect().catch(err =>
{/* log/handle error */}) so all errors are caught and logged.
Validate that post_auth_redirect values start with '/' and not '//' to prevent open redirect attacks via user-controlled query params. Applied at both storage (login/signup) and consumption (callback, verification modal, verify link) points. Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com>
Fix team invite redirect lost after email verification
Summary
When a new user clicks a team invite link from email, they are redirected to sign up. After signing up, they should land on the team invite acceptance page — but this redirect was being lost in two ways:
VerificationModal, so after signup the email verification popup would appear on the index page (or not at all), and after verifying, the user was stuck on/with no way back to the invite page.nextparameter was never persisted tosessionStoragebefore redirecting to the OAuth provider, so it was lost entirely on the round-trip.Changes
team.invite.$inviteId.tsx: Stores the invite URL insessionStorage(post_auth_redirect) before redirecting to signup. Adds<VerificationModal />so users can verify their email without leaving the invite page.signup.tsx/login.tsx: StoresnextinsessionStorageaspost_auth_redirectbefore GitHub/Google OAuth web redirects.auth.$provider.callback.tsx: Readspost_auth_redirectfromsessionStorageafter OAuth callback and navigates there instead of always going to/.VerificationModal.tsx: After successful email verification, checks forpost_auth_redirectand navigates to it.verify.$code.tsx: After email verification via link, checks forpost_auth_redirectand navigates there instead of always going to/.Review & Testing Checklist for Human
/)VerificationModalplacement: It's only rendered in the "invite preview" return path — check whether it should also appear in the loading/error states of the invite pagepost_auth_redirect: If a user abandons an invite flow and later signs in normally, verify they aren't unexpectedly redirected to an old invite URLnavigate({ to: pendingRedirect })works with TanStack Router: The redirect value is a dynamic string fromsessionStorage— confirm TanStack Router handles paths like/team/invite/{uuid}correctly when passed as a stringNotes
post_auth_redirect— only GitHub and Google web flows. Apple web uses a popup (no page redirect), so the context is preserved. Tauri flows redirect to external browser and use deep links, which is a separate flow.sessionStoragekey is scoped to the browser tab/session, so it won't leak across tabs or persist after the session ends.Link to Devin session: https://app.devin.ai/sessions/3e6cfae8255e4610b40ae1e868bde04c
Requested by: @AnthonyRonning
Summary by CodeRabbit