fix(auth): return JSON 401 for all expired /setup-api/* requests (closes #304) - #316
Conversation
Generalizes the per-caller fixes from #231/#303. A raw fetch("/setup-api/...") sends `Accept: */*`, so the middleware's `accept.includes("application/json")` gate missed it and fell through to the HTML login *redirect* β whose body then made the caller's `.json()` throw on an expired session. ~30 callers across ~15 files carried this latent bug. Fix at the source: return a JSON 401 for the whole /setup-api/* (and /api/*) prefix regardless of the Accept header, so every caller gets a structured 401 it can detect β no client changes needed. Genuine top-level page navigations (no API prefix) still redirect to /login. Deliberate trade-off: the few /setup-api/* routes loaded by direct browser embedding (webapps?app= iframes, apps/icon/[appId] <img>, file downloads) now get a 401 too instead of a login page β which is fine, since a login page inside an <img> or a download stream is useless anyway. Tests: middleware suite updated to assert 401 (not 307) for /setup-api/*, plus a regression test that the 401 body is parseable JSON. 47/47 passing on-device. Closes #304.
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review detailsβοΈ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: π Files selected for processing (2)
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 |
π¦ ClawReviewFresh PR washed in with the tide β here's the gist. This PR fixes a long-standing session-expiry bug in the auth middleware: raw fetch() calls (which send Accept: /) were falling through to an HTML login-redirect instead of a JSON 401, causing .json() to throw on ~30 callers across ~15 files. The fix is a single condition change β /setup-api/* and /api/* paths now always return a JSON 401 regardless of Accept header, requiring no client-side updates. At a glance
Good to know
β ClawReview π¦. I set the scene; CodeRabbit reviews the code; you decide. Conventions: docs. |
Closes #304 β the follow-up to #231/#303, which each patched a single caller.
The latent bug
A raw
fetch("/setup-api/...")sendsAccept: */*. The middleware gate was:So a fetch without an explicit
Accept: application/jsonheader fell through to the login-page redirect, and the caller's.json()then threw on the HTML body when the session had expired. ~30 callers across ~15 files (i18n.tsx,SettingsApp.tsx,mascot-client.ts,useOllamaModels,FilesApp,DoneStep,AIModelsStep, β¦) carried this.Fix (the issue's preferred "deepest fix")
Return a JSON 401 for the whole
/setup-api/*(and/api/*) prefix regardless of theAcceptheader. Every caller now gets a structured 401 it can detect β no client changes. Top-level page navigations (no API prefix) still redirect to/login.Deliberate trade-off: the handful of
/setup-api/*routes loaded by direct browser embedding (webapps?app=iframes,apps/icon/[appId]<img>, file downloads) now get a 401 instead of a login page. That's fine β a login page inside an<img>or a download stream is useless anyway; the desktop shell drives the real re-login.Tests
/setup-api/*with noAcceptheader now asserts 401 (was the buggy 307).