Skip to content

Commit 8c62867

Browse files
fix(api-client): only auto-redirect to /login from /app/* (closes homepage redirect bug) (#53)
Root cause: src/api/index.ts had an inverted skip-list — only /login and /claim were excluded from the 401→/login auto-redirect. Any other public route (/, /pricing, /docs, /blog, /use-cases, /status, /incidents) could get bounced to /login if a stray api call from a hook, Suspense boundary, or cached service worker returned 401. Fix: invert the gate. Redirect only when on /app/*. On public routes, clear the stale token silently and throw the APIError to the caller without navigating — the page stays renderable for an anonymous visitor. +1 regression test pinning pathname stability on / when fetchMe returns 401. 92/95 api tests pass. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d0a209d commit 8c62867

2 files changed

Lines changed: 44 additions & 14 deletions

File tree

src/api/index.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,23 @@ describe('claim()', () => {
554554
})
555555
})
556556

557+
it('does NOT redirect on 401 from the marketing homepage (regression for "homepage auto-redirects to /login")', async () => {
558+
// Root cause of the homepage-redirect bug: the previous SKIP-list only
559+
// excluded /login + /claim. Any other public page (marketing /, /pricing,
560+
// /docs, /blog, /use-cases, /status, /incidents) would, on a 401 from a
561+
// stray api call, get bounced to /login. Fix: redirect only when in
562+
// /app/*. Pin pathname stability on / as the regression guard.
563+
window.history.pushState({}, '', '/')
564+
const m = installFetch()
565+
m.mockResolvedValueOnce(jsonResponse(
566+
{ error: 'unauthorized' },
567+
{ status: 401, statusText: 'Unauthorized' },
568+
))
569+
await expect(fetchMe()).rejects.toMatchObject({ status: 401 })
570+
expect(window.location.pathname).toBe('/')
571+
window.history.pushState({}, '', '/')
572+
})
573+
557574
it('does NOT redirect on 401 when the current path starts with /claim', async () => {
558575
// Navigate jsdom to /claim/abc via history.pushState so the auth-skip
559576
// prefix matches. We can't spy on window.location.replace in jsdom 24

src/api/index.ts

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,24 +73,33 @@ class APIError extends Error {
7373
}
7474
}
7575

76-
// Paths where a 401 should NOT trigger the auto-redirect to /login. The
77-
// LoginPage uses fetchMe() to verify a freshly-pasted PAT and wants to
78-
// surface the 401 inline; the ClaimPage handles its own auth flow.
79-
const AUTH_REDIRECT_SKIP_PREFIXES = ['/login', '/claim']
76+
// Paths where a 401 SHOULD auto-redirect to /login. Only the gated `/app/*`
77+
// subtree warrants kicking the user out — every other route (marketing `/`,
78+
// `/pricing`, `/docs`, `/blog`, `/use-cases`, `/status`, `/incidents`,
79+
// `/login`, `/claim`, …) must render even when the api rejects a stray
80+
// call. The previous SKIP-list approach was inverted: any public page that
81+
// happened to fire a 401-producing api call (stale NR ping, deferred
82+
// fetch, a re-mounted hook from cached navigation) would bounce the
83+
// visitor to /login. That was the root cause of "homepage automatically
84+
// redirects to /login" — fixed by gating the redirect to the path we
85+
// actually want to protect.
86+
const AUTH_REDIRECT_REQUIRED_PREFIXES = ['/app']
8087
const RETURN_TO_KEY = 'instanode.return_to'
8188

8289
/**
8390
* Central fetch wrapper. On a 401 response from the API the helper:
8491
* 1. clears the stored token,
8592
* 2. saves the current pathname+search under `instanode.return_to` so the
8693
* user can be sent back after re-login,
87-
* 3. redirects to `/login` via `window.location.replace` (so the back
88-
* button doesn't loop), and
94+
* 3. redirects to `/login` via `window.location.replace` ONLY if the
95+
* caller is already inside the gated `/app/*` subtree,
8996
* 4. still throws the `APIError` so callers see a rejected promise.
9097
*
91-
* The redirect is suppressed when the current path already starts with
92-
* `/login` or `/claim` — those pages render the 401 inline. The function
93-
* is also a no-op outside a browser environment (SSR / unit tests).
98+
* The redirect is intentionally limited to `/app/*`. On any public page
99+
* (marketing, pricing, docs, login, claim) we clear the token and surface
100+
* the error to the caller without navigation — the page stays renderable
101+
* for an anonymous visitor. The function is also a no-op outside a
102+
* browser environment (SSR / unit tests).
94103
*/
95104
async function call<T>(
96105
path: string,
@@ -118,15 +127,19 @@ async function call<T>(
118127
const body: any = ct.includes('application/json') ? await res.json().catch(() => null) : await res.text()
119128
if (!res.ok) {
120129
if (res.status === 401) {
121-
let onAuthPage = false
130+
let inGatedAppArea = false
122131
try {
123132
const p = location.pathname
124-
onAuthPage = AUTH_REDIRECT_SKIP_PREFIXES.some((prefix) => p.startsWith(prefix))
133+
inGatedAppArea = AUTH_REDIRECT_REQUIRED_PREFIXES.some(
134+
(prefix) => p === prefix || p.startsWith(prefix + '/'),
135+
)
125136
} catch {
126-
/* non-browser env (jsdom-less tests) — treat as not on auth page */
137+
/* non-browser env (jsdom-less tests) — treat as not in /app */
127138
}
128-
if (!onAuthPage) {
129-
clearToken()
139+
// Always clear the stale token so subsequent calls don't re-attach
140+
// it. Only navigate when we're already inside the gated area.
141+
clearToken()
142+
if (inGatedAppArea) {
130143
try {
131144
localStorage.setItem(RETURN_TO_KEY, location.pathname + location.search)
132145
} catch {

0 commit comments

Comments
 (0)