Repository navigation
fix(share): auto-join drive after OAuth sign-in/up - #1344
Conversation
- Thread returnUrl through useOAuthSignIn so OAuth flows redirect back to /s/[token] after auth instead of /dashboard - Skip getting-started drive overwrite of returnUrl when destination is a share link, so new users still land on /s/[token] - DriveShareAccept now auto-accepts via useEffect on auth (matching PageShareAccept) - signin page passes nextPath as returnUrl to OAuth; signup page reads and forwards ?next param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR extends the authentication flow to support post-auth redirect URLs while preserving share-invitation links. It adds optional ChangesOAuth post-auth redirect flow with conditional share-invitation preservation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47950facbd
ℹ️ 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".
When authenticated but csrfToken is null, show loading state instead of null (blank page regression). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/auth/signup/SignUpClient.tsx`:
- Line 27: The "Already have an account?" link in SignUpClient is hardcoded to
'/auth/signin' and drops the passed returnUrl; update the link generation inside
SignUpClient (the code that renders the "Already have an account?" path) to
include the existing returnUrl when present (e.g., append as a query param or
build the route using returnUrl) so the redirect target is preserved when users
switch to Log in; ensure both occurrences around the current sign-in link use
the inviteToken/inviteContext logic if needed and fall back safely when
returnUrl is undefined.
In `@apps/web/src/app/s/`[token]/DriveShareAccept.tsx:
- Around line 24-26: The component DriveShareAccept currently exits the
useEffect and renders null when isAuthenticated is true but csrfToken is
missing, causing a blank UI; update the useEffect to only skip starting the
accept flow when the user is unauthenticated (i.e., if (!isAuthenticated)
return) and handle the authenticated+no-csrf state by rendering a
loading/placeholder instead of null (replace the null return at the render path
with a spinner or retry UI), and optionally trigger or retry CSRF retrieval when
csrfToken is absent so the accept logic (the effect) runs once csrfToken becomes
available.
- Around line 97-99: The sign-in redirect currently interpolates the raw token
into the next query (Button onClick →
router.push(`/auth/signin?next=/s/${token}`)), which can produce invalid URLs
for tokens containing reserved characters; change the push to URL-encode the
next value (e.g., use encodeURIComponent on the path or token) so the
constructed query becomes
router.push(`/auth/signin?next=${encodeURIComponent(`/s/${token}`)}`) — update
the Button onClick handler accordingly to ensure safe redirects.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33b25eb0-f420-4261-a45f-6527f2f9ebbb
📒 Files selected for processing (7)
apps/web/src/app/api/auth/apple/callback/route.tsapps/web/src/app/api/auth/google/callback/route.tsapps/web/src/app/auth/signin/page.tsxapps/web/src/app/auth/signup/SignUpClient.tsxapps/web/src/app/auth/signup/page.tsxapps/web/src/app/s/[token]/DriveShareAccept.tsxapps/web/src/hooks/useOAuthSignIn.ts
- encodeURIComponent the /s/[token] path in DriveShareAccept signin redirect - SignUpClient Log in link now preserves returnUrl as ?next= param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Spinner replaces disabled button during auth load; sign-in URL is now URL-encoded per CodeRabbit fix in 45023b4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
CI fix: Unit Tests failed on the existing
Fixed in 65bec36. |
New users registering via passkey from a share link were landing on /dashboard instead of back at /s/[token]. Add nextPath prop to PasskeySignupButton (mirrors PasskeyLoginButton) and wire it from SignUpClient. Also fix onEmailExists to preserve returnUrl. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Additional fix: passkey signup didn't thread Root cause: Fix (13684a5):
Auth method coverage after all fixes:
|
…ative iOS OAuth - DriveShareAccept: destructure csrfLoading/csrfError from useCSRFToken so a CSRF fetch failure shows the error state instead of an infinite spinner - useOAuthSignIn: add returnUrl to PostNativeAuthRedirectInput so native iOS auth uses the share link as the post-auth destination (invite-consumed drive still wins) - ios-google-auth / ios-apple-auth: pass returnUrl in request body and surface it in the result type so the hook can act on it - api/auth/google/native + api/auth/apple/native: accept returnUrl in the Zod schema, validate it with isSafeReturnUrl, and echo it back in the response - tests: add 4 new DriveShareAccept cases (CSRF error, happy path, non-ok, throws) and 5 useOAuthSignIn cases (returnUrl inclusion/omission in body, buildPostNativeAuthRedirect returnUrl precedence) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Additional hardening commit pushed ( CSRF error → infinite spinner (critical, found in self-review):
Missing test coverage (significant, found in self-review):
Native iOS
All 21 new/updated tests pass. |
Replace dynamic await import() with the static import that is already present via the getClientIP import from @/lib/auth. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(share): auto-join drive after OAuth sign-in/up - Thread returnUrl through useOAuthSignIn so OAuth flows redirect back to /s/[token] after auth instead of /dashboard - Skip getting-started drive overwrite of returnUrl when destination is a share link, so new users still land on /s/[token] - DriveShareAccept now auto-accepts via useEffect on auth (matching PageShareAccept) - signin page passes nextPath as returnUrl to OAuth; signup page reads and forwards ?next param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(share): show spinner while CSRF token loads When authenticated but csrfToken is null, show loading state instead of null (blank page regression). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(share): encode next param and preserve returnUrl on login link - encodeURIComponent the /s/[token] path in DriveShareAccept signin redirect - SignUpClient Log in link now preserves returnUrl as ?next= param Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(share): update DriveShareAccept tests for auto-accept Spinner replaces disabled button during auth load; sign-in URL is now URL-encoded per CodeRabbit fix in 45023b4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(share): thread nextPath through PasskeySignupButton New users registering via passkey from a share link were landing on /dashboard instead of back at /s/[token]. Add nextPath prop to PasskeySignupButton (mirrors PasskeyLoginButton) and wire it from SignUpClient. Also fix onEmailExists to preserve returnUrl. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(share): harden CSRF error handling and thread returnUrl through native iOS OAuth - DriveShareAccept: destructure csrfLoading/csrfError from useCSRFToken so a CSRF fetch failure shows the error state instead of an infinite spinner - useOAuthSignIn: add returnUrl to PostNativeAuthRedirectInput so native iOS auth uses the share link as the post-auth destination (invite-consumed drive still wins) - ios-google-auth / ios-apple-auth: pass returnUrl in request body and surface it in the result type so the hook can act on it - api/auth/google/native + api/auth/apple/native: accept returnUrl in the Zod schema, validate it with isSafeReturnUrl, and echo it back in the response - tests: add 4 new DriveShareAccept cases (CSRF error, happy path, non-ok, throws) and 5 useOAuthSignIn cases (returnUrl inclusion/omission in body, buildPostNativeAuthRedirect returnUrl precedence) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(share): use static import for isSafeReturnUrl in native routes Replace dynamic await import() with the static import that is already present via the getClientIP import from @/lib/auth. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Changes:
Test plan
🤖 Generated with Claude Code