feat(react): support custom redirect function via AuthProvider#175
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR refactors authentication redirect handling to move control from server-side routing to client-side React hooks. Core API types are unified into shared discriminated unions, client methods consistently request ChangesClient-side Redirect Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/client/client.ts (2)
76-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefault redirect behavior may not match the documented
@default true.This branch (and the equivalents in
signInCredentials,updateSession,signOut, and the React hooks) only performs navigation whenoptions?.redirect === true. Whenredirectis omitted it isundefined, so no redirect occurs — yetOptionsWithRedirectTo.redirectis documented as@default true. If the default is intended to redirect, the guard should beoptions?.redirect !== false. Please confirm the intended default.🤖 Prompt for 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. In `@packages/core/src/client/client.ts` at line 76, The redirect check currently only triggers when options?.redirect === true, so omitted redirects (undefined) are treated as false; update the guard in the client.ts branch (and the equivalent checks in signInCredentials, updateSession, signOut, and the React hooks) to treat undefined as “true” by changing the condition to options?.redirect !== false so redirect occurs by default; ensure every occurrence that currently checks for === true is replaced with !== false and run tests/verify navigation behavior after the change.
122-135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSession payload (PII) leaked into query string.
Same pattern as credentials:
searchParams: { ...options, redirect: false }spreads the fullsessionobject (e.g. user name/email) into the URL. The body already sendsuser/expires; onlyredirectTo/redirectbelong in search params.🔒️ Proposed fix
searchParams: { - ...options, + redirectTo: options.redirectTo, redirect: false, },🤖 Prompt for 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. In `@packages/core/src/client/client.ts` around lines 122 - 135, The session PATCH is leaking PII by spreading the entire options/session into searchParams; update the client.patch call in client.ts (the call to client.patch("/session")) to only include allowed query keys (e.g. redirect and redirectTo) in searchParams instead of spreading ...options or session. Ensure the request body still contains user and expires (the existing body object remains), remove or replace the spread in searchParams with an explicit object like { redirect: false, redirectTo: options?.redirectTo } (or the minimal allowed keys), and keep the X-CSRF-Token header intact.
🤖 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 `@packages/react/src/hooks.ts`:
- Around line 101-103: The current guard options?.redirect === true treats
undefined as false and therefore skips navigation even though redirect defaults
to true; change the condition to treat undefined as true (for example use
options?.redirect !== false or (options?.redirect ?? true) === true) where
performRedirect(redirect, value.signInURL) is called (symbols: options,
performRedirect, redirect, value.signInURL) in this hook and replicate the same
fix in the other three hooks so they honor the documented default; also align
the core client default-redirect logic (the root default-redirect concern) to
the same semantics so all callers behave consistently.
In `@packages/react/test/hooks.test.tsx`:
- Line 68: The test currently uses
expect(redirectMock).not.toHaveBeenCalledOnce(), which doesn't guarantee zero
calls; replace that assertion with expect(redirectMock).not.toHaveBeenCalled()
to precisely assert no redirect occurred; update the assertion in the test
referencing redirectMock in hooks.test.tsx (same test block) so it matches the
other tests' pattern.
---
Outside diff comments:
In `@packages/core/src/client/client.ts`:
- Line 76: The redirect check currently only triggers when options?.redirect ===
true, so omitted redirects (undefined) are treated as false; update the guard in
the client.ts branch (and the equivalent checks in signInCredentials,
updateSession, signOut, and the React hooks) to treat undefined as “true” by
changing the condition to options?.redirect !== false so redirect occurs by
default; ensure every occurrence that currently checks for === true is replaced
with !== false and run tests/verify navigation behavior after the change.
- Around line 122-135: The session PATCH is leaking PII by spreading the entire
options/session into searchParams; update the client.patch call in client.ts
(the call to client.patch("/session")) to only include allowed query keys (e.g.
redirect and redirectTo) in searchParams instead of spreading ...options or
session. Ensure the request body still contains user and expires (the existing
body object remains), remove or replace the spread in searchParams with an
explicit object like { redirect: false, redirectTo: options?.redirectTo } (or
the minimal allowed keys), and keep the X-CSRF-Token header intact.
🪄 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: 58f265c6-6c16-43b1-b8c3-8b82bff7c53c
📒 Files selected for processing (9)
packages/core/src/@types/api.tspackages/core/src/@types/utility.tspackages/core/src/actions/signInCredentials/signInCredentials.tspackages/core/src/client/client.tspackages/react/package.jsonpackages/react/src/@types/types.tspackages/react/src/context.tsxpackages/react/src/hooks.tspackages/react/test/hooks.test.tsx
Description
This pull request adds support for custom redirect handlers in authentication flows within the
@aura-stack/reactpackage.Previously, authentication actions relied on the default browser navigation behavior. With this update, developers can provide their own redirect implementation, allowing seamless integration with framework-specific navigation APIs such as:
This provides greater flexibility when handling authentication redirects while preserving the existing default behavior.
Key Changes
Usage