From 140686415790be9352f340d60c8f1bc41cec41f3 Mon Sep 17 00:00:00 2001 From: liutingqiu Date: Fri, 2 Oct 2026 07:09:36 +0800 Subject: [PATCH] fix(web): redirect on session expiry without reloading the page The api interceptor assigned window.location.href when a token refresh failed, which reloads the whole document and discards all React and store state. It also sent users to / instead of /dashboard. The service now clears the store and calls an injected handler, and the router registers that handler to navigate declaratively with . Also polyfill localStorage in the test setup so the store-backed suites can run. --- src/router/index.tsx | 33 +++++- src/services/__tests__/api.test.ts | 153 ++++++++++++++++++++++++- src/services/api.ts | 43 +++++-- src/stores/__tests__/app.store.test.ts | 88 ++++++++++++++ src/stores/app.store.ts | 15 ++- src/test/setup.ts | 48 ++++++++ 6 files changed, 369 insertions(+), 11 deletions(-) create mode 100644 src/stores/__tests__/app.store.test.ts diff --git a/src/router/index.tsx b/src/router/index.tsx index 4b909b2..c0565ef 100644 --- a/src/router/index.tsx +++ b/src/router/index.tsx @@ -1,11 +1,13 @@ import { createBrowserRouter, RouterProvider, Navigate } from 'react-router-dom' -import { lazy, Suspense } from 'react' +import { lazy, Suspense, useEffect } from 'react' import type { ReactNode } from 'react' import { Layout } from '../components/layout/Layout' import { Spinner } from '../components/ui/Spinner' import { useRoleStore } from '../stores/role.store' import type { UserRole } from '../stores/role.store' import { useWallet } from '../hooks/useWallet' +import { useAppStore } from '../stores/app.store' +import { setSessionExpiredHandler } from '../services/api' // Route pages are code-split so heavy per-page dependencies (e.g. the Stellar // SDK pulled in by the dashboard/vouch flows) load on demand instead of @@ -55,6 +57,7 @@ function RoleGuard({ function page(node: ReactNode) { return ( + @@ -68,6 +71,26 @@ function page(node: ReactNode) { ) } +/** + * Sends the user to /dashboard with a client-side redirect once the API layer + * reports that the session is unrecoverable. + * + * `/dashboard` is the role dispatcher: it reads the role store and forwards to + * the correct surface (or to `/` when no role is selected), which is the right + * entry point for a signed-out session. + * + * This exists so `src/services/api.ts` never has to assign `window.location`: + * a full document reload discards all React and store state. The service calls + * a handler that this component registers, and React Router performs the + * navigation. + */ +function RedirectOnSessionExpiry() { + const sessionExpired = useAppStore((s) => s.sessionExpired) + + if (!sessionExpired) return null + return +} + const router = createBrowserRouter([ { path: '/', @@ -136,5 +159,13 @@ const router = createBrowserRouter([ ]) export function Router() { + useEffect(() => { + // Register the redirect once, at the top of the app: the API layer has no + // router context of its own, so this is how it hands the "session is over" + // signal back to React Router. + setSessionExpiredHandler(() => useAppStore.getState().setSessionExpired(true)) + return () => setSessionExpiredHandler(null) + }, []) + return } diff --git a/src/services/__tests__/api.test.ts b/src/services/__tests__/api.test.ts index e808729..32a5f48 100644 --- a/src/services/__tests__/api.test.ts +++ b/src/services/__tests__/api.test.ts @@ -6,11 +6,64 @@ import type { AxiosResponse, InternalAxiosRequestConfig, } from 'axios' -import { api } from '../api' +import { api, setSessionExpiredHandler } from '../api' import { useUserStore } from '../../stores/user.store' const originalAdapter = api.defaults.adapter +/** + * Installs a working in-memory `localStorage` for the duration of a test. + * + * jsdom provides no `localStorage` here, and Node 24+ shadows it with a + * getter-only global accessor that resolves to `undefined` unless the process + * was started with `--localstorage-file`. Two consequences: + * - `vi.stubGlobal` cannot replace it (assignment to an accessor with no + * setter is silently ignored), and + * - zustand reads `window.localStorage` specifically (see `persistImpl` in + * zustand/middleware), not `globalThis.localStorage`, so `window` must be + * patched too. + * + * Returns a restore function for `afterEach`. + */ +function installMemoryStorage(): () => void { + const makeStorage = () => { + const store = new Map() + return { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => void store.set(key, String(value)), + removeItem: (key: string) => void store.delete(key), + clear: () => store.clear(), + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } + } + + const targets: object[] = [] + const win = (globalThis as { window?: object }).window + if (win) targets.push(win) + targets.push(globalThis) + + const restores = targets.map((target) => { + const original = Object.getOwnPropertyDescriptor(target, 'localStorage') + Object.defineProperty(target, 'localStorage', { + configurable: true, + writable: true, + value: makeStorage(), + }) + return () => { + if (original) { + Object.defineProperty(target, 'localStorage', original) + } else { + Reflect.deleteProperty(target, 'localStorage') + } + } + }) + + return () => restores.forEach((restore) => restore()) +} + function resetTokens() { useUserStore.setState({ accessToken: '', refreshToken: '', isAuthenticated: false }) } @@ -110,3 +163,101 @@ describe('API response interceptor — refresh keeps the store in sync', () => { expect(useUserStore.getState().refreshToken).toBe('refresh-3') }) }) + +describe('API response interceptor — unrecoverable session', () => { + let restoreStorage: () => void + + beforeEach(() => { + // This suite reaches `clearTokens()`, which writes to localStorage. The + // environment does not provide a usable one (see installMemoryStorage). + restoreStorage = installMemoryStorage() + localStorage.clear() + resetTokens() + }) + + afterEach(() => { + setSessionExpiredHandler(null) + restoreStorage() + vi.restoreAllMocks() + }) + + it('clears the store and asks the router to redirect when there is no session to refresh', async () => { + // No refresh token and no access token: nothing can be recovered. + const onSessionExpired = vi.fn() + setSessionExpiredHandler(onSessionExpired) + useUserStore.setState({ accessToken: '', refreshToken: '', isAuthenticated: false }) + + const rejected = api.interceptors.response.handlers?.[0]?.rejected + const error = { + config: { headers: {} }, + response: { status: 401 }, + } as unknown as AxiosError + + await expect(rejected!(error)).rejects.toBe(error) + + expect(onSessionExpired).toHaveBeenCalledTimes(1) + expect(useUserStore.getState().isAuthenticated).toBe(false) + // The in-memory store must be cleared, not just flagged unauthenticated. + expect(useUserStore.getState().accessToken).toBe('') + expect(localStorage.getItem('accessToken')).toBeNull() + expect(localStorage.getItem('refreshToken')).toBeNull() + }) + + it('clears the store and redirects when a refresh attempt fails', async () => { + const onSessionExpired = vi.fn() + setSessionExpiredHandler(onSessionExpired) + useUserStore.getState().setTokens('stale-access', 'stale-refresh') + // Refresh fails, so recovery is impossible from here. + vi.spyOn(axios, 'post').mockRejectedValue(new Error('refresh rejected')) + + const rejected = api.interceptors.response.handlers?.[0]?.rejected + const error = { + config: { headers: {} }, + response: { status: 401 }, + } as unknown as AxiosError + + await expect(rejected!(error)).rejects.toBeTruthy() + + expect(onSessionExpired).toHaveBeenCalledTimes(1) + expect(useUserStore.getState().accessToken).toBe('') + expect(useUserStore.getState().refreshToken).toBe('') + expect(localStorage.getItem('accessToken')).toBeNull() + }) + + it('clears the store but does not redirect an existing session that cannot be refreshed', async () => { + // An expired session with an access token but no refresh token: the caller + // should surface the 401 rather than having the page torn down underneath + // the user. This preserves the behaviour the original guard documented. + const onSessionExpired = vi.fn() + setSessionExpiredHandler(onSessionExpired) + useUserStore.setState({ + accessToken: 'expired-access', + refreshToken: '', + isAuthenticated: true, + }) + + const rejected = api.interceptors.response.handlers?.[0]?.rejected + const error = { + config: { headers: {} }, + response: { status: 401 }, + } as unknown as AxiosError + + await rejected!(error).catch(() => undefined) + + expect(onSessionExpired).not.toHaveBeenCalled() + expect(useUserStore.getState().accessToken).toBe('') + }) + + it('does nothing when no handler has been registered', async () => { + // Guards against the service hard-depending on a mounted router. + setSessionExpiredHandler(null) + + const rejected = api.interceptors.response.handlers?.[0]?.rejected + const error = { + config: { headers: {} }, + response: { status: 401 }, + } as unknown as AxiosError + + await expect(rejected!(error)).rejects.toBe(error) + }) +}) diff --git a/src/services/api.ts b/src/services/api.ts index ba7a680..4afa24d 100644 --- a/src/services/api.ts +++ b/src/services/api.ts @@ -11,6 +11,33 @@ interface FailedRequest { let isRefreshing = false let failedQueue: FailedRequest[] = [] +/** + * Called when the session can no longer be recovered and the user has to be + * sent back through the login flow. + * + * This is injected rather than performed here because `api` is a plain module: + * it cannot call `useNavigate`, and assigning `window.location` would trigger a + * full document reload that throws away all React and store state. The router + * registers the real handler at startup (see `setSessionExpiredHandler` in + * `src/router/index.tsx`), which navigates declaratively instead. + */ +let sessionExpiredHandler: (() => void) | null = null + +export function setSessionExpiredHandler(handler: (() => void) | null): void { + sessionExpiredHandler = handler +} + +function notifySessionExpired(): void { + sessionExpiredHandler?.() +} + +function endSession(): void { + // Clear the store first so in-memory auth state and its persisted copy are + // gone before anything navigates away from the current route. + useUserStore.getState().clearTokens() + notifySessionExpired() +} + function processQueue(error: unknown, token: string | null = null) { failedQueue.forEach((prom) => { if (error) { @@ -65,13 +92,14 @@ api.interceptors.response.use( const { accessToken: existingAccessToken, refreshToken } = useUserStore.getState() if (!refreshToken) { isRefreshing = false - useUserStore.getState().clearTokens() - // Only force-redirect users who were never authenticated. - // If a token existed (e.g. expired session with no refresh - // token), let the caller surface the error instead of - // tearing the page down. + // Only sign users out when there was no authenticated session to + // preserve. If a token existed (e.g. an expired session with no refresh + // token), let the caller surface the error instead of tearing the page + // down. if (!existingAccessToken) { - window.location.href = '/' + endSession() + } else { + useUserStore.getState().clearTokens() } return Promise.reject(error) } @@ -91,8 +119,7 @@ api.interceptors.response.use( return api(originalRequest) } catch (refreshError) { processQueue(refreshError, null) - useUserStore.getState().clearTokens() - window.location.href = '/' + endSession() return Promise.reject(refreshError) } finally { isRefreshing = false diff --git a/src/stores/__tests__/app.store.test.ts b/src/stores/__tests__/app.store.test.ts new file mode 100644 index 0000000..3be5e92 --- /dev/null +++ b/src/stores/__tests__/app.store.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { useAppStore } from '../app.store' + +/** + * Installs a working in-memory `localStorage` for the duration of a test. + * + * jsdom provides no `localStorage` here, and Node 24+ shadows it with a + * getter-only global accessor that resolves to `undefined` unless the process + * was started with `--localstorage-file`. Two consequences: + * - `vi.stubGlobal` cannot replace it (assignment to an accessor with no + * setter is silently ignored), and + * - zustand reads `window.localStorage` specifically (see `persistImpl` in + * zustand/middleware), not `globalThis.localStorage`, so `window` must be + * patched too. + */ +function installMemoryStorage(): () => void { + const makeStorage = () => { + const store = new Map() + return { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => void store.set(key, String(value)), + removeItem: (key: string) => void store.delete(key), + clear: () => store.clear(), + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } + } + + const targets: object[] = [] + const win = (globalThis as { window?: object }).window + if (win) targets.push(win) + targets.push(globalThis) + + const restores = targets.map((target) => { + const original = Object.getOwnPropertyDescriptor(target, 'localStorage') + Object.defineProperty(target, 'localStorage', { + configurable: true, + writable: true, + value: makeStorage(), + }) + return () => { + if (original) { + Object.defineProperty(target, 'localStorage', original) + } else { + Reflect.deleteProperty(target, 'localStorage') + } + } + }) + + return () => restores.forEach((restore) => restore()) +} + +describe('useAppStore session expiry signal', () => { + let restoreStorage: () => void + + beforeEach(() => { + restoreStorage = installMemoryStorage() + useAppStore.getState().setSessionExpired(false) + }) + + afterEach(() => { + restoreStorage() + }) + + it('starts without a pending session-expiry redirect', () => { + expect(useAppStore.getState().sessionExpired).toBe(false) + }) + + it('setSessionExpired raises and clears the redirect signal', () => { + useAppStore.getState().setSessionExpired(true) + expect(useAppStore.getState().sessionExpired).toBe(true) + + useAppStore.getState().setSessionExpired(false) + expect(useAppStore.getState().sessionExpired).toBe(false) + }) + + it('does not persist the transient redirect signal across reloads', () => { + useAppStore.getState().setSessionExpired(true) + + // A pending redirect must not survive a reload, otherwise a later visit + // would bounce the user straight back to the dashboard. + const raw = localStorage.getItem('stepfi-app') + const persisted = raw ? JSON.parse(raw) : { state: {} } + expect(persisted.state).not.toHaveProperty('sessionExpired') + }) +}) diff --git a/src/stores/app.store.ts b/src/stores/app.store.ts index 45b4291..0fbb3b7 100644 --- a/src/stores/app.store.ts +++ b/src/stores/app.store.ts @@ -5,8 +5,14 @@ interface AppStore { mobileMenuOpen: boolean onboardingComplete: boolean theme: 'dark' | 'light' + /** + * Set when a token refresh failed and the session cannot be recovered. The + * router watches this to redirect the user without a full page reload. + */ + sessionExpired: boolean setMobileMenuOpen: (open: boolean) => void setOnboardingComplete: (complete: boolean) => void + setSessionExpired: (expired: boolean) => void toggleTheme: () => void } @@ -16,15 +22,22 @@ export const useAppStore = create()( mobileMenuOpen: false, onboardingComplete: false, theme: 'dark', + sessionExpired: false, setMobileMenuOpen: (mobileMenuOpen) => set({ mobileMenuOpen }), setOnboardingComplete: (onboardingComplete) => set({ onboardingComplete }), + setSessionExpired: (sessionExpired) => set({ sessionExpired }), toggleTheme: () => set((state) => ({ theme: state.theme === 'dark' ? 'light' : 'dark', })), }), - { name: 'stepfi-app' } + { + name: 'stepfi-app', + // A transient navigation signal must not survive a reload, or a later + // visit would bounce the user straight back to the dashboard. + partialize: (state) => ({ theme: state.theme }), + } ) ) diff --git a/src/test/setup.ts b/src/test/setup.ts index c44951a..9481fcc 100644 --- a/src/test/setup.ts +++ b/src/test/setup.ts @@ -1 +1,49 @@ import '@testing-library/jest-dom' + +/** + * Provide `localStorage` for the test environment. + * + * jsdom under this Node version leaves `window.localStorage` undefined, and + * Node 24+ shadows the global with a getter-only accessor that resolves to + * `undefined` unless the process was started with `--localstorage-file`. Two + * consequences that break tests before they can assert anything: + * - `vi.stubGlobal('localStorage', ...)` cannot replace it, because assigning + * to an accessor with no setter is silently ignored; and + * - zustand's persist middleware reads `window.localStorage` once, at module + * load (`createJSONStorage(() => window.localStorage)` in + * zustand/middleware), and keeps that reference forever. + * + * So the polyfill has to be installed here — in the setup file, which runs + * before any test module is imported — rather than per test. + */ +function installLocalStoragePolyfill(): void { + const store = new Map() + const storage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => { + store.set(key, String(value)) + }, + removeItem: (key: string) => { + store.delete(key) + }, + clear: () => { + store.clear() + }, + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } + + for (const target of [window, globalThis] as object[]) { + Object.defineProperty(target, 'localStorage', { + configurable: true, + writable: true, + value: storage, + }) + } +} + +if (typeof window !== 'undefined' && !window.localStorage) { + installLocalStoragePolyfill() +}