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.
This commit is contained in:
@@ -3,7 +3,7 @@ import { act, renderHook, waitFor } from "@testing-library/react";
|
|||||||
import { HttpResponse, http } from "msw";
|
import { HttpResponse, http } from "msw";
|
||||||
import type { FC, PropsWithChildren } from "react";
|
import type { FC, PropsWithChildren } from "react";
|
||||||
import { describe, expect, it } from "vitest";
|
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 { createTestQueryClient } from "@/test/query";
|
||||||
import { server } from "@/test/server";
|
import { server } from "@/test/server";
|
||||||
import { AuthProvider, useAuth } from "./auth";
|
import { AuthProvider, useAuth } from "./auth";
|
||||||
@@ -167,4 +167,164 @@ describe("AuthProvider", () => {
|
|||||||
expect(queryClient.getQueryData(portfolioKey)).toBeUndefined();
|
expect(queryClient.getQueryData(portfolioKey)).toBeUndefined();
|
||||||
expect(queryClient.getQueryData(bankCardsKey)).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<void>((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<PropsWithChildren> = ({ children }) => (
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<AuthProvider>{children}</AuthProvider>
|
||||||
|
</QueryClientProvider>
|
||||||
|
);
|
||||||
|
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<void>;
|
||||||
|
act(() => {
|
||||||
|
logout = result.current.logout();
|
||||||
|
});
|
||||||
|
await waitFor(() => expect(result.current.user).toBeNull());
|
||||||
|
let login!: Promise<void>;
|
||||||
|
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<PropsWithChildren> = ({ children }) => (
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<AuthProvider>{children}</AuthProvider>
|
||||||
|
</QueryClientProvider>
|
||||||
|
);
|
||||||
|
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<string>((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<PropsWithChildren> = (props) => (
|
||||||
|
<QueryClientProvider client={queryClient}>
|
||||||
|
<AuthProvider>{props.children}</AuthProvider>
|
||||||
|
</QueryClientProvider>
|
||||||
|
);
|
||||||
|
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"),
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -47,49 +47,70 @@ const AuthProvider: FC<PropsWithChildren> = (props) => {
|
|||||||
const accessTokenRef = useRef<string | null>(null);
|
const accessTokenRef = useRef<string | null>(null);
|
||||||
const [ready, setReady] = useState(false);
|
const [ready, setReady] = useState(false);
|
||||||
|
|
||||||
|
const generation = useRef(0);
|
||||||
|
const userRef = useRef<User | null>(null);
|
||||||
|
const authQueue = useRef<Promise<unknown>>(Promise.resolve());
|
||||||
const refreshPromise = useRef<Promise<boolean> | null>(null);
|
const refreshPromise = useRef<Promise<boolean> | null>(null);
|
||||||
const applyAuthentication = useCallback((authentication: Authentication) => {
|
// Cookie-writing requests share ordering, including across tabs with Web Locks.
|
||||||
setUser(authentication.user);
|
const serialize = useCallback(<T,>(run: () => Promise<T>): Promise<T> => {
|
||||||
setAccessToken(authentication.accessToken);
|
const next = authQueue.current.then(async () =>
|
||||||
accessTokenRef.current = authentication.accessToken;
|
"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(() => {
|
const clearSession = useCallback(() => {
|
||||||
|
generation.current += 1;
|
||||||
|
userRef.current = null;
|
||||||
setUser(null);
|
setUser(null);
|
||||||
setAccessToken(null);
|
setAccessToken(null);
|
||||||
accessTokenRef.current = null;
|
accessTokenRef.current = null;
|
||||||
queryClient.clear();
|
queryClient.clear();
|
||||||
}, [queryClient]);
|
}, [queryClient]);
|
||||||
|
|
||||||
const refresh = useCallback(async (): Promise<boolean> => {
|
const refresh = useCallback((): Promise<boolean> => {
|
||||||
if (refreshPromise.current) return refreshPromise.current;
|
if (refreshPromise.current) return refreshPromise.current;
|
||||||
refreshPromise.current = (async () => {
|
const started = generation.current;
|
||||||
const run = async () => {
|
const pending = serialize(async () => {
|
||||||
try {
|
if (started !== generation.current) return false;
|
||||||
const { data } = await apiClient.POST("/api/v1/auth/refresh");
|
try {
|
||||||
if (!data) {
|
const { data } = await apiClient.POST("/api/v1/auth/refresh");
|
||||||
clearSession();
|
if (started !== generation.current) return false;
|
||||||
return false;
|
if (!data) {
|
||||||
}
|
|
||||||
applyAuthentication(data as Authentication);
|
|
||||||
return true;
|
|
||||||
} catch {
|
|
||||||
clearSession();
|
clearSession();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
};
|
applyAuthentication(data);
|
||||||
try {
|
return true;
|
||||||
return "locks" in navigator
|
|
||||||
? navigator.locks.request("wealth-ledger-refresh", run)
|
|
||||||
: run();
|
|
||||||
} catch {
|
} catch {
|
||||||
clearSession();
|
if (started === generation.current) clearSession();
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
})().finally(() => {
|
}).finally(() => {
|
||||||
refreshPromise.current = null;
|
if (refreshPromise.current === pending) refreshPromise.current = null;
|
||||||
});
|
});
|
||||||
return refreshPromise.current;
|
refreshPromise.current = pending;
|
||||||
}, [applyAuthentication, clearSession]);
|
return pending;
|
||||||
|
}, [applyAuthentication, clearSession, serialize]);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
void refresh().finally(() => setReady(true));
|
void refresh().finally(() => setReady(true));
|
||||||
@@ -100,21 +121,25 @@ const AuthProvider: FC<PropsWithChildren> = (props) => {
|
|||||||
email: string,
|
email: string,
|
||||||
password: string,
|
password: string,
|
||||||
) {
|
) {
|
||||||
const authentication = await request(path, email, password);
|
clearSession();
|
||||||
applyAuthentication(authentication);
|
const started = generation.current;
|
||||||
|
await serialize(async () => {
|
||||||
|
const authentication = await request(path, email, password);
|
||||||
|
if (started === generation.current) applyAuthentication(authentication);
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
async function logout() {
|
async function logout() {
|
||||||
try {
|
clearSession();
|
||||||
|
await serialize(async () => {
|
||||||
await apiClient.POST("/api/v1/auth/logout");
|
await apiClient.POST("/api/v1/auth/logout");
|
||||||
} finally {
|
});
|
||||||
clearSession();
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
configureAuthClient({
|
configureAuthClient({
|
||||||
accessToken: () => accessTokenRef.current,
|
accessToken: () => accessTokenRef.current,
|
||||||
|
generation: () => generation.current,
|
||||||
refresh,
|
refresh,
|
||||||
clear: clearSession,
|
clear: clearSession,
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -64,4 +64,33 @@ describe("apiClient authentication", () => {
|
|||||||
|
|
||||||
expect(authorization).toBeNull();
|
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<void>((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;
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ const HTTP_UNAUTHORIZED = 401;
|
|||||||
const apiBaseUrl = import.meta.env.VITE_API_BASE_URL ?? "http://localhost:3000";
|
const apiBaseUrl = import.meta.env.VITE_API_BASE_URL ?? "http://localhost:3000";
|
||||||
|
|
||||||
type AuthHandlers = {
|
type AuthHandlers = {
|
||||||
|
generation?: () => number;
|
||||||
accessToken: () => string | null;
|
accessToken: () => string | null;
|
||||||
refresh: () => Promise<boolean>;
|
refresh: () => Promise<boolean>;
|
||||||
clear: () => void;
|
clear: () => void;
|
||||||
@@ -41,7 +42,10 @@ async function authenticatedFetch(request: Request): Promise<Response> {
|
|||||||
const isAuthRequest = new URL(request.url).pathname.startsWith(
|
const isAuthRequest = new URL(request.url).pathname.startsWith(
|
||||||
"/api/v1/auth/",
|
"/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 withToken = (source: Request, value: string) => {
|
||||||
const headers = new Headers(source.headers);
|
const headers = new Headers(source.headers);
|
||||||
headers.set("Authorization", `Bearer ${value}`);
|
headers.set("Authorization", `Bearer ${value}`);
|
||||||
@@ -51,6 +55,8 @@ async function authenticatedFetch(request: Request): Promise<Response> {
|
|||||||
token && !isAuthRequest ? withToken(request, token) : request;
|
token && !isAuthRequest ? withToken(request, token) : request;
|
||||||
const retryRequest = authorized.clone();
|
const retryRequest = authorized.clone();
|
||||||
const response = await fetch(authorized);
|
const response = await fetch(authorized);
|
||||||
|
if (!isAuthRequest && !isCurrent())
|
||||||
|
throw new DOMException("Session changed", "AbortError");
|
||||||
if (
|
if (
|
||||||
response.status !== HTTP_UNAUTHORIZED ||
|
response.status !== HTTP_UNAUTHORIZED ||
|
||||||
isAuthRequest ||
|
isAuthRequest ||
|
||||||
@@ -63,14 +69,17 @@ async function authenticatedFetch(request: Request): Promise<Response> {
|
|||||||
)?.code !== "UNAUTHORIZED"
|
)?.code !== "UNAUTHORIZED"
|
||||||
)
|
)
|
||||||
return response;
|
return response;
|
||||||
|
if (!isCurrent()) throw new DOMException("Session changed", "AbortError");
|
||||||
if (!(await auth.refresh())) {
|
if (!(await auth.refresh())) {
|
||||||
auth.clear();
|
if (isCurrent()) auth.clear();
|
||||||
return response;
|
return response;
|
||||||
}
|
}
|
||||||
|
if (!isCurrent()) throw new DOMException("Session changed", "AbortError");
|
||||||
const refreshedToken = auth.accessToken();
|
const refreshedToken = auth.accessToken();
|
||||||
return refreshedToken
|
if (!refreshedToken) return response;
|
||||||
? fetch(withToken(retryRequest, refreshedToken))
|
const retried = await fetch(withToken(retryRequest, refreshedToken));
|
||||||
: response;
|
if (!isCurrent()) throw new DOMException("Session changed", "AbortError");
|
||||||
|
return retried;
|
||||||
}
|
}
|
||||||
|
|
||||||
const apiClient = createClient<paths>({
|
const apiClient = createClient<paths>({
|
||||||
|
|||||||
Reference in New Issue
Block a user