Conversation
Auth-gated routes showed page-level errors (e.g. "Ukjent bruker") instead of redirecting to login. The old Laravel+SPA app had a RedirectToLogin component that triggered on NotAuthedError. This was lost in the TanStack Start migration. Add a QueryCache onError handler that detects auth errors and redirects to SAML login with the current path as returnTo.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughThe root route imports QueryCache and adds AUTH_ERROR_MESSAGE. QueryClient is initialized with a QueryCache onError handler that detects the auth error message and redirects the browser to the SAML login with a return-to parameter before any retry logic runs. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (client)
participant App as React App (QueryClient/QueryCache)
participant SAML as SAML Login (Auth Server)
Browser->>App: Execute query via QueryClient
App->>App: QueryCache.onError receives error
alt error.message == AUTH_ERROR_MESSAGE
App->>Browser: window.location.assign(samlLogin?return_to=currentUrl)
Browser->>SAML: Redirect to SAML login
else other error
App->>App: Normal error handling / retries
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/routes/__root.tsx (1)
32-37: Consider a one-shot guard against concurrent auth-error redirects.QueryCache global callbacks run once per cache entry. If multiple queries with distinct keys all fail with
AUTH_ERROR_MESSAGEin the same tick,window.location.assignis called once per failing query key. The first navigation makes the rest harmless, but a simple flag makes the intent explicit and avoids any theoretical double-navigation side effects.♻️ Proposed guard
+let redirectingToLogin = false const queryClient = new QueryClient({ queryCache: new QueryCache({ onError(error) { if (error instanceof Error && error.message === AUTH_ERROR_MESSAGE) { + if (redirectingToLogin) return + redirectingToLogin = true const returnTo = encodeURIComponent(window.location.pathname) window.location.assign(`/intern/api/saml2/login?returnTo=${returnTo}`) } }, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/routes/__root.tsx` around lines 32 - 37, The onError global callback handling AUTH_ERROR_MESSAGE should use a one-shot guard to prevent concurrent redirects: add a module-level boolean (e.g., didAuthRedirect or isAuthRedirecting) checked at the top of the onError handler and return early if already set, set it true immediately before calling window.location.assign(`/intern/api/saml2/login?returnTo=${returnTo}`), and keep the existing check for error instanceof Error && error.message === AUTH_ERROR_MESSAGE; reference the onError callback, AUTH_ERROR_MESSAGE, and window.location.assign to locate where to add the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/routes/__root.tsx`:
- Line 34: The returnTo value currently uses window.location.pathname which
drops query params; change the construction of returnTo in __root.tsx to include
both pathname and search (e.g., window.location.pathname +
window.location.search) before calling encodeURIComponent so users are returned
to the exact URL (including ?...); update the declaration that defines returnTo
accordingly.
---
Nitpick comments:
In `@app/src/routes/__root.tsx`:
- Around line 32-37: The onError global callback handling AUTH_ERROR_MESSAGE
should use a one-shot guard to prevent concurrent redirects: add a module-level
boolean (e.g., didAuthRedirect or isAuthRedirecting) checked at the top of the
onError handler and return early if already set, set it true immediately before
calling window.location.assign(`/intern/api/saml2/login?returnTo=${returnTo}`),
and keep the existing check for error instanceof Error && error.message ===
AUTH_ERROR_MESSAGE; reference the onError callback, AUTH_ERROR_MESSAGE, and
window.location.assign to locate where to add the guard.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
QueryCacheonErrorhandler that detects auth errors and redirects to SAML login with the current path asreturnToRedirectToLogincomponent triggered onNotAuthedError