fix: change browsers fetch to get 401s and redirect#1826
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a client-side fetch interceptor in the root provider that detects HTTP 401 responses, parses the response body for a redirect URL (three field name variants), validates it for safe relative or same-origin targets with a /login fallback, navigates the browser when valid, and returns the original Response. Changes401 Redirect Handling
sequenceDiagram
participant Browser
participant FetchWrapper
participant Server
Browser->>FetchWrapper: window.fetch(request)
FetchWrapper->>Server: network request
Server-->>FetchWrapper: 401 response with JSON { redirect_url / redirectUrl / redirect }
FetchWrapper->>FetchWrapper: clone() → parse JSON → validate redirect
FetchWrapper->>Browser: window.location.href = validatedRedirect (if valid)
FetchWrapper-->>Browser: return original Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 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 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
🤖 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 `@frontend/app/providers.tsx`:
- Around line 15-18: The code sets window.location.href directly from the
untrusted redirectUrl variable, creating an open-redirect risk; fix by
validating redirectUrl before assigning it to window.location.href in the same
block where redirectUrl is determined: allow only relative paths (e.g., strings
that start with "/" but not "//") or same-origin absolute URLs (use the URL
constructor with location.origin as base and verify url.origin ===
location.origin), and fall back to a safe default like "/" or "/login" if
validation fails; replace the direct assignment to window.location.href with the
validated URL.
- Around line 6-10: The module-level replacement of window.fetch in
providers.tsx is re-applied on each HMR causing nested wrappers; modify the
logic that captures originalFetch and assigns window.fetch to first check for a
marker (e.g., a Symbol or reserved property) on window.fetch to skip re-wrapping
if already patched, store the true original fetch once (use originalFetch
variable only when marker is absent), set the marker on the patched function,
and keep the existing 401 handling behavior in the patched fetch wrapper so
subsequent hot reloads do not wrap it again.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42043fd4-6b82-4631-9658-aae1c8a63f5c
📒 Files selected for processing (1)
frontend/app/providers.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This pull request introduces a global override for
window.fetchin the browser to handle authentication errors more gracefully. Now, if a fetch request receives a 401 Unauthorized response with a redirect URL in the response payload, the client will automatically redirect the user to that URL.Authentication and error handling improvements:
window.fetchinproviders.tsxto detect 401 responses, parse the response body for possible redirect URLs (redirect_url,redirectUrl, orredirect), and redirect the browser if a URL is found. Errors during parsing are logged to the console.Summary by CodeRabbit