From a209d715892862bb6d48084df0c84a10b8d56bf1 Mon Sep 17 00:00:00 2001 From: Sergey Krylov Date: Mon, 21 Sep 2026 21:09:23 +0300 Subject: [PATCH] fix(auth): isolate session races and financial caches TD-001 and TD-002: serialize cookie writes, ignore stale responses and reset financial queries on identity changes. Backend cookie lifecycle regression: ec5780d. --- src/features/auth/model/auth.test.tsx | 162 +++++++++++++++++++++++++- src/features/auth/model/auth.tsx | 89 +++++++++----- src/shared/api/client.test.ts | 29 +++++ src/shared/api/client.ts | 19 ++- 4 files changed, 261 insertions(+), 38 deletions(-) diff --git a/src/features/auth/model/auth.test.tsx b/src/features/auth/model/auth.test.tsx index ecc997d..e349958 100644 --- a/src/features/auth/model/auth.test.tsx +++ b/src/features/auth/model/auth.test.tsx @@ -3,7 +3,7 @@ import { act, renderHook, waitFor } from "@testing-library/react"; import { HttpResponse, http } from "msw"; import type { FC, PropsWithChildren } from "react"; import { describe, expect, it } from "vitest"; -import { apiClient, apiQueryClient } from "@/shared/api"; +import { apiClient, apiQueryClient, useAuthenticatedQuery } from "@/shared/api"; import { createTestQueryClient } from "@/test/query"; import { server } from "@/test/server"; import { AuthProvider, useAuth } from "./auth"; @@ -167,4 +167,164 @@ describe("AuthProvider", () => { expect(queryClient.getQueryData(portfolioKey)).toBeUndefined(); expect(queryClient.getQueryData(bankCardsKey)).toBeUndefined(); }); + it.each([ + [false, false], + [true, false], + [false, true], + [true, true], + ])( + "serializes late refresh (failure=%s, login=%s)", + async (failure, loginAfterLogout) => { + let release!: () => void; + const pending = new Promise((resolve) => { + release = resolve; + }); + let calls = 0; + const order: string[] = []; + server.use( + http.post("*/api/v1/auth/refresh", async () => { + if (++calls > 1) { + order.push("refresh-start"); + await pending; + order.push("refresh-cookie"); + } + return failure && calls > 1 + ? HttpResponse.error() + : HttpResponse.json({ + accessToken: "a", + user: { id: "a", email: "a@example.com" }, + }); + }), + http.post("*/api/v1/auth/logout", () => { + order.push("logout-cookie"); + return new HttpResponse(null, { status: 204 }); + }), + http.post("*/api/v1/auth/login", () => { + order.push("login-cookie"); + return HttpResponse.json({ + accessToken: "b", + user: { id: "b", email: "b@example.com" }, + }); + }), + http.get("*/api/v1/brokerage-accounts", () => + HttpResponse.json({ code: "UNAUTHORIZED" }, { status: 401 }), + ), + ); + const queryClient = createTestQueryClient(); + const wrapper: FC = ({ children }) => ( + + {children} + + ); + const { result } = renderHook(() => useAuth(), { wrapper }); + await waitFor(() => expect(result.current.ready).toBe(true)); + const oldRequest = apiClient + .GET("/api/v1/brokerage-accounts") + .catch(() => null); + await waitFor(() => expect(order).toContain("refresh-start")); + let logout!: Promise; + act(() => { + logout = result.current.logout(); + }); + await waitFor(() => expect(result.current.user).toBeNull()); + let login!: Promise; + act(() => { + login = loginAfterLogout + ? result.current.login("b@example.com", "password") + : Promise.resolve(); + }); + await act(async () => { + release(); + await Promise.all([oldRequest, logout, login]); + }); + expect(result.current.user?.id ?? null).toBe( + loginAfterLogout ? "b" : null, + ); + expect(order).toEqual([ + "refresh-start", + "refresh-cookie", + "logout-cookie", + ...(loginAfterLogout ? ["login-cookie"] : []), + ]); + }, + ); + + it("preserves same-user cache and removes inactive and pending data on refresh identity change", async () => { + let identity = "a"; + server.use( + http.post("*/api/v1/auth/refresh", () => + HttpResponse.json({ + accessToken: identity, + user: { id: identity, email: `${identity}@example.com` }, + }), + ), + ); + const queryClient = createTestQueryClient(); + const wrapper: FC = ({ children }) => ( + + {children} + + ); + const { result } = renderHook(() => useAuth(), { wrapper }); + await waitFor(() => expect(result.current.ready).toBe(true)); + queryClient.setQueryData(bankCardsKey, [{ id: "a-card" }]); + await act(() => result.current.refresh()); + expect(queryClient.getQueryData(bankCardsKey)).toEqual([{ id: "a-card" }]); + let release!: (value: string) => void; + const pending = queryClient + .fetchQuery({ + queryKey: ["pending-finance"], + queryFn: () => + new Promise((resolve) => { + release = resolve; + }), + }) + .catch(() => null); + identity = "b"; + await act(() => result.current.refresh()); + release("a-data"); + await pending; + expect(result.current.user?.id).toBe("b"); + expect(queryClient.getQueryData(bankCardsKey)).toBeUndefined(); + expect(queryClient.getQueryData(["pending-finance"])).toBeUndefined(); + }); + it("replaces an observed financial query when refresh changes identity", async () => { + let identity = "a"; + server.use( + http.post("*/api/v1/auth/refresh", () => + HttpResponse.json({ + accessToken: identity, + user: { id: identity, email: `${identity}@example.com` }, + }), + ), + http.get("*/api/v1/bank-cards", () => + HttpResponse.json([{ id: `${identity}-card` }]), + ), + ); + const queryClient = createTestQueryClient(); + const wrapper: FC = (props) => ( + + {props.children} + + ); + const { result } = renderHook( + () => ({ + auth: useAuth(), + cards: useAuthenticatedQuery( + apiQueryClient.queryOptions("get", "/api/v1/bank-cards", { + params: { query: { includeArchived: "false" } }, + }), + ), + }), + { wrapper }, + ); + await waitFor(() => + expect(result.current.cards.data?.[0].id).toBe("a-card"), + ); + identity = "b"; + await act(() => result.current.auth.refresh()); + await waitFor(() => + expect(result.current.cards.data?.[0].id).toBe("b-card"), + ); + }); }); diff --git a/src/features/auth/model/auth.tsx b/src/features/auth/model/auth.tsx index 6bd700c..16d13ca 100644 --- a/src/features/auth/model/auth.tsx +++ b/src/features/auth/model/auth.tsx @@ -47,49 +47,70 @@ const AuthProvider: FC = (props) => { const accessTokenRef = useRef(null); const [ready, setReady] = useState(false); + const generation = useRef(0); + const userRef = useRef(null); + const authQueue = useRef>(Promise.resolve()); const refreshPromise = useRef | null>(null); - const applyAuthentication = useCallback((authentication: Authentication) => { - setUser(authentication.user); - setAccessToken(authentication.accessToken); - accessTokenRef.current = authentication.accessToken; + // Cookie-writing requests share ordering, including across tabs with Web Locks. + const serialize = useCallback((run: () => Promise): Promise => { + const next = authQueue.current.then(async () => + "locks" in navigator + ? navigator.locks.request("wealth-ledger-refresh", run) + : run(), + ); + authQueue.current = next.catch(() => undefined); + return next; }, []); + const applyAuthentication = useCallback( + (authentication: Authentication) => { + const changedIdentity = + userRef.current && userRef.current.id !== authentication.user.id; + accessTokenRef.current = authentication.accessToken; + if (changedIdentity) { + generation.current += 1; + queryClient.removeQueries({ type: "inactive" }); + queryClient.getMutationCache().clear(); + void queryClient.resetQueries(); + } + userRef.current = authentication.user; + setUser(authentication.user); + setAccessToken(authentication.accessToken); + }, + [queryClient], + ); const clearSession = useCallback(() => { + generation.current += 1; + userRef.current = null; setUser(null); setAccessToken(null); accessTokenRef.current = null; queryClient.clear(); }, [queryClient]); - const refresh = useCallback(async (): Promise => { + const refresh = useCallback((): Promise => { if (refreshPromise.current) return refreshPromise.current; - refreshPromise.current = (async () => { - const run = async () => { - try { - const { data } = await apiClient.POST("/api/v1/auth/refresh"); - if (!data) { - clearSession(); - return false; - } - applyAuthentication(data as Authentication); - return true; - } catch { + const started = generation.current; + const pending = serialize(async () => { + if (started !== generation.current) return false; + try { + const { data } = await apiClient.POST("/api/v1/auth/refresh"); + if (started !== generation.current) return false; + if (!data) { clearSession(); return false; } - }; - try { - return "locks" in navigator - ? navigator.locks.request("wealth-ledger-refresh", run) - : run(); + applyAuthentication(data); + return true; } catch { - clearSession(); + if (started === generation.current) clearSession(); return false; } - })().finally(() => { - refreshPromise.current = null; + }).finally(() => { + if (refreshPromise.current === pending) refreshPromise.current = null; }); - return refreshPromise.current; - }, [applyAuthentication, clearSession]); + refreshPromise.current = pending; + return pending; + }, [applyAuthentication, clearSession, serialize]); useEffect(() => { void refresh().finally(() => setReady(true)); @@ -100,21 +121,25 @@ const AuthProvider: FC = (props) => { email: string, password: string, ) { - const authentication = await request(path, email, password); - applyAuthentication(authentication); + clearSession(); + const started = generation.current; + await serialize(async () => { + const authentication = await request(path, email, password); + if (started === generation.current) applyAuthentication(authentication); + }); } async function logout() { - try { + clearSession(); + await serialize(async () => { await apiClient.POST("/api/v1/auth/logout"); - } finally { - clearSession(); - } + }); } useEffect(() => { configureAuthClient({ accessToken: () => accessTokenRef.current, + generation: () => generation.current, refresh, clear: clearSession, }); diff --git a/src/shared/api/client.test.ts b/src/shared/api/client.test.ts index fa83613..e679c1a 100644 --- a/src/shared/api/client.test.ts +++ b/src/shared/api/client.test.ts @@ -64,4 +64,33 @@ describe("apiClient authentication", () => { expect(authorization).toBeNull(); }); + it("discards a retried request response after another session takes over", async () => { + let generation = 0; + let calls = 0; + let release!: () => void; + const pending = new Promise((resolve) => { + release = resolve; + }); + server.use( + http.get("*/api/v1/brokerage-accounts", async () => { + if (++calls === 1) + return HttpResponse.json({ code: "UNAUTHORIZED" }, { status: 401 }); + await pending; + return HttpResponse.json([{ id: "previous-user-account" }]); + }), + ); + configureAuthClient({ + accessToken: () => "synthetic", + generation: () => generation, + refresh: async () => true, + clear: vi.fn(), + }); + const response = expect( + apiClient.GET("/api/v1/brokerage-accounts"), + ).rejects.toMatchObject({ name: "AbortError" }); + await vi.waitFor(() => expect(calls).toBe(2)); + generation += 1; + release(); + await response; + }); }); diff --git a/src/shared/api/client.ts b/src/shared/api/client.ts index cb87d40..d73ea56 100644 --- a/src/shared/api/client.ts +++ b/src/shared/api/client.ts @@ -8,6 +8,7 @@ const HTTP_UNAUTHORIZED = 401; const apiBaseUrl = import.meta.env.VITE_API_BASE_URL ?? "http://localhost:3000"; type AuthHandlers = { + generation?: () => number; accessToken: () => string | null; refresh: () => Promise; clear: () => void; @@ -41,7 +42,10 @@ async function authenticatedFetch(request: Request): Promise { const isAuthRequest = new URL(request.url).pathname.startsWith( "/api/v1/auth/", ); - const token = auth?.accessToken(); + const session = auth; + const generation = session?.generation?.(); + const isCurrent = () => generation === auth?.generation?.(); + const token = session?.accessToken(); const withToken = (source: Request, value: string) => { const headers = new Headers(source.headers); headers.set("Authorization", `Bearer ${value}`); @@ -51,6 +55,8 @@ async function authenticatedFetch(request: Request): Promise { token && !isAuthRequest ? withToken(request, token) : request; const retryRequest = authorized.clone(); const response = await fetch(authorized); + if (!isAuthRequest && !isCurrent()) + throw new DOMException("Session changed", "AbortError"); if ( response.status !== HTTP_UNAUTHORIZED || isAuthRequest || @@ -63,14 +69,17 @@ async function authenticatedFetch(request: Request): Promise { )?.code !== "UNAUTHORIZED" ) return response; + if (!isCurrent()) throw new DOMException("Session changed", "AbortError"); if (!(await auth.refresh())) { - auth.clear(); + if (isCurrent()) auth.clear(); return response; } + if (!isCurrent()) throw new DOMException("Session changed", "AbortError"); const refreshedToken = auth.accessToken(); - return refreshedToken - ? fetch(withToken(retryRequest, refreshedToken)) - : response; + if (!refreshedToken) return response; + const retried = await fetch(withToken(retryRequest, refreshedToken)); + if (!isCurrent()) throw new DOMException("Session changed", "AbortError"); + return retried; } const apiClient = createClient({