From 0b5f03d99f7667d44be540b62f5eae62ca85dde5 Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Sun, 6 Sep 2026 11:04:18 +0300 Subject: [PATCH 1/3] fix: don't leave a spent form behind the back button Every post-save, cancel, and delete navigation used push instead of replace, so Back after finishing something returned to the form you just submitted or abandoned (worst case in the attach-partner detour, which left two dead entries). --- .../src/components/mapper/MappingEditorToolbar.tsx | 2 +- .../src/pages/aggregations/NewAggregationPage.tsx | 4 ++-- .../src/pages/api-gateways/ApiGatewayNewPage.tsx | 2 +- .../ClientApp/src/pages/api-gateways/ApiGatewayPage.tsx | 2 +- .../src/pages/api-gateways/AttachPartnerPage.tsx | 8 ++++++-- .../src/pages/api-gateways/EditAttachmentPage.tsx | 4 ++-- .../src/pages/api-gateways/NewGatewaySubscriptionPage.tsx | 2 +- .../src/pages/bus-gateways/BusGatewayNewPage.tsx | 2 +- .../ClientApp/src/pages/bus-gateways/BusGatewayPage.tsx | 2 +- .../ClientApp/src/pages/exchanges/ExchangeNewPage.tsx | 4 ++-- .../src/pages/global-values/GlobalValueSetPage.tsx | 2 +- .../src/pages/information-types/InformationTypePage.tsx | 2 +- .../ClientApp/src/pages/notifiers/NotifierPage.tsx | 2 +- .../ClientApp/src/pages/partners/PartnerPage.tsx | 2 +- .../src/pages/retry-policies/RetryPolicyPage.tsx | 2 +- .../src/pages/scheduled-jobs/NewScheduledJobPage.tsx | 4 ++-- .../src/pages/subscriptions/SubscriptionPage.tsx | 2 +- SW.Bitween.Web/ClientApp/src/pages/team/RoleEditor.tsx | 6 +++--- .../ClientApp/src/pages/work-groups/WorkGroupPage.tsx | 2 +- 19 files changed, 30 insertions(+), 26 deletions(-) diff --git a/SW.Bitween.Web/ClientApp/src/components/mapper/MappingEditorToolbar.tsx b/SW.Bitween.Web/ClientApp/src/components/mapper/MappingEditorToolbar.tsx index 4613ea6c..fe4c725b 100644 --- a/SW.Bitween.Web/ClientApp/src/components/mapper/MappingEditorToolbar.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/mapper/MappingEditorToolbar.tsx @@ -97,7 +97,7 @@ const MappingEditorToolbar: React.FC = ({
{/* Back */} + + + +
- + @@ -356,7 +356,7 @@ export function RoleEditor() { onConfirm={async () => { await api.deleteRole(id!); void queryClient.invalidateQueries({ queryKey: keys.roles.all }); - navigate("/team/roles"); + navigate("/team/roles", { replace: true }); }} onClose={() => setConfirmingDelete(false)} /> diff --git a/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupPage.tsx index 20c2347f..d2cb7dc5 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/work-groups/WorkGroupPage.tsx @@ -146,7 +146,7 @@ export function WorkGroupPage() { onConfirm={async () => { await api.deleteWorkGroup(groupId); void queryClient.invalidateQueries({ queryKey: keys.workGroups.all }); - navigate("/work-groups"); + navigate("/work-groups", { replace: true }); }} onClose={() => setDeleting(false)} /> From ddb324f2f2e909e8b1c4cb66e44474161aa03336 Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Sun, 6 Sep 2026 11:04:31 +0300 Subject: [PATCH 2/3] fix: end the session locally before calling the server Sign out awaited the logout request before clearing local state, so a failed or slow request left the user looking signed in (with the token already gone), and a normal sign-out felt slow waiting on the round trip. Also covers two related gaps: nothing reacted to a session that ended in another tab or expired server-side (both left the app rendering as signed in until a manual refresh), and a sign-in right after a sign-out could get wiped by a late-arriving Clear-Site-Data. - logout() clears the token first and swallows the request's own failure instead of throwing through it - signOut() clears session state before calling the server, not after - a storage listener ends the session here when it ends in another tab - request() reports a proven-dead session to a listener instead of only throwing to whichever caller happened to be asking - login() waits for any in-flight logout to settle first - removes the idle-timeout's retry/reload workaround, now unreachable --- .../ClientApp/src/api/http/request.ts | 20 ++++++- .../ClientApp/src/api/http/session.ts | 37 +++++++++++-- SW.Bitween.Web/ClientApp/src/api/index.ts | 1 + .../ClientApp/src/auth/SessionContext.tsx | 54 +++++++++++++++++-- .../ClientApp/src/auth/useIdleLogout.ts | 20 ++----- 5 files changed, 106 insertions(+), 26 deletions(-) diff --git a/SW.Bitween.Web/ClientApp/src/api/http/request.ts b/SW.Bitween.Web/ClientApp/src/api/http/request.ts index 67214946..e241d5c0 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/request.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/request.ts @@ -7,7 +7,22 @@ import { ApiRequestError } from "../types"; export const API_BASE = "/api"; /** The Jwt is kept in localStorage; the refresh token is an HttpOnly cookie JS never sees. */ -const TOKEN_KEY = "access_token"; +export const TOKEN_KEY = "access_token"; + +/** + * Told when a request proves the session is over — the Jwt was refused and no + * refresh cookie was left to replace it. + * + * A callback rather than a hook because this is plain module code that cannot + * reach React state. `SessionProvider` registers itself on mount; with nothing + * registered, the `UNAUTHENTICATED` error below is all that happens, which is + * what used to leave a dead session rendering the entire app until somebody + * pressed refresh. + */ +let sessionEndedListener: (() => void) | null = null; +export const onSessionEnded = (listener: (() => void) | null): void => { + sessionEndedListener = listener; +}; export const getToken = (): string | null => localStorage.getItem(TOKEN_KEY); export const setToken = (jwt: string): void => localStorage.setItem(TOKEN_KEY, jwt); @@ -117,6 +132,9 @@ export async function request(path: string, opts: RequestOptions = {}): Promi const refreshed = await silentRefresh(); if (refreshed) return request(path, { ...opts, _retried: true }); clearToken(); + // Tell the app, not only the caller. Every page with a read in flight is about + // to render its own small error, and a page's error state cannot end a session. + sessionEndedListener?.(); throw new ApiRequestError("UNAUTHENTICATED", "Your session has ended. Please sign in again."); } diff --git a/SW.Bitween.Web/ClientApp/src/api/http/session.ts b/SW.Bitween.Web/ClientApp/src/api/http/session.ts index 88076776..5261e390 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/session.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/session.ts @@ -42,6 +42,18 @@ const buildSession = (profile: Profile): Session => { const loadSession = async (): Promise => buildSession(await get("/accounts/profile")); +/** + * The most recent sign-out's server call, which every sign-in waits for. + * + * The logout response carries `Clear-Site-Data: "cookies", "storage"`. Arriving + * *after* a fresh sign-in it wipes that sign-in's Jwt and refresh cookie, and the + * user is thrown back to the page they just left. The two can genuinely overlap + * now that signing out no longer blocks the UI: the login page appears at once, + * and a password manager can fill and submit it before the logout lands. Awaiting + * a promise that has already settled costs nothing, so this is never cleared. + */ +let lastLogout: Promise | null = null; + export const sessionMethods = { async getSession(): Promise { // No stored Jwt → anonymous; don't probe the backend (an expired token still @@ -57,6 +69,7 @@ export const sessionMethods = { }, async login(email: string, password: string): Promise { + await lastLogout; const { jwt } = await post("/accounts/login", { Username: email, Password: password, @@ -66,6 +79,7 @@ export const sessionMethods = { }, async loginWithMicrosoft(): Promise { + await lastLogout; const cfg = await getAppConfig(); if (!cfg.msalClientId) throw new ApiRequestError("MS_NOT_CONFIGURED", "Microsoft sign-in isn't configured."); @@ -94,11 +108,24 @@ export const sessionMethods = { }, async logout(): Promise { - try { - await post("/accounts/logout"); - } finally { - clearToken(); - } + // Cleared first and unconditionally. This used to run in a `finally`, which + // deleted the Jwt and then let the error through — the worst pairing, because + // the caller aborted before it could end the session and the app carried on + // rendering as if signed in. Removing the key here is also what wakes the + // other tabs (see the `storage` listener in SessionContext). + clearToken(); + lastLogout = (async () => { + try { + await post("/accounts/logout"); + } catch { + // Swallowed on purpose: signing out must not depend on the server + // answering. Only the server can invalidate the refresh cookie — it is + // HttpOnly, so JS cannot touch it — so if this never lands the cookie + // outlives the sign-out. This browser has no Jwt to pair with it, and the + // next sign-in replaces it. + } + })(); + return lastLogout; }, async updateProfile(changes: { displayName: string }): Promise { diff --git a/SW.Bitween.Web/ClientApp/src/api/index.ts b/SW.Bitween.Web/ClientApp/src/api/index.ts index 31dadb7d..5e8fa302 100644 --- a/SW.Bitween.Web/ClientApp/src/api/index.ts +++ b/SW.Bitween.Web/ClientApp/src/api/index.ts @@ -12,4 +12,5 @@ export const api: ApiClient = httpClient; export { getAppConfig, resetAppConfig } from "./http/appConfig"; export type { AppConfig } from "./http/appConfig"; export { referencesGlobal, referencesPartnerProp } from "./http/references"; +export { TOKEN_KEY, onSessionEnded } from "./http/request"; export * from "./types"; diff --git a/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx b/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx index 8f49f237..0014d6b8 100644 --- a/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx +++ b/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx @@ -8,7 +8,7 @@ import { type ReactNode, } from "react"; import { useQueryClient } from "@tanstack/react-query"; -import { api, type PermissionKey, type Session } from "../api"; +import { api, TOKEN_KEY, onSessionEnded, type PermissionKey, type Session } from "../api"; import { useIdleLogout } from "./useIdleLogout"; interface SessionContextValue { @@ -67,12 +67,58 @@ export function SessionProvider({ children }: { children: ReactNode }) { setSession(await api.getSession()); }, []); - const signOut = useCallback(async () => { - await api.logout(); - queryClient.clear(); + /** Ends the session locally, then tells the server. Never the other way round. */ + const endSession = useCallback(() => { setSession(null); + queryClient.clear(); }, [queryClient]); + const signOut = useCallback(async () => { + // The session ends here, whatever the server says. `await api.logout()` used to + // come first, so any failure — a backend restart, a blip, an unreachable host — + // threw before the two lines below ran: signed out everywhere except the one + // place that decides what you see, which then needed a manual refresh. Ending it + // first also means the login page appears immediately rather than after the + // round trip, which is what made signing out feel slow. + endSession(); + await api.logout(); + }, [endSession]); + + /** + * Ends the session here when it ends in another tab. + * + * Both credentials are shared by every tab of this origin — the Jwt in + * localStorage and the refresh cookie — so a sign-out in one tab really has ended + * the session in all of them; the others simply never found out and kept rendering + * the whole app over reads that could only fail. `storage` fires only in the + * *other* tabs, which is exactly the audience. A null key means the entire store + * was cleared, which the logout response's `Clear-Site-Data` does, so that counts + * too; a key with a new value is another tab signing *in*, which doesn't. + */ + useEffect(() => { + const onStorage = (e: StorageEvent) => { + if (e.key !== null && e.key !== TOKEN_KEY) return; + if (e.newValue) return; + endSession(); + }; + window.addEventListener("storage", onStorage); + return () => window.removeEventListener("storage", onStorage); + }, [endSession]); + + /** + * Ends the session here when a request proves it is already over. + * + * Covers what no `storage` event can: a Jwt and refresh cookie that both expired + * while a tab sat open, or an administrator disabling the account. `request()` + * already knew — it writes "Your session has ended" — but it could only throw that + * at whichever page happened to be asking, and a page's error state cannot sign + * anyone out. + */ + useEffect(() => { + onSessionEnded(endSession); + return () => onSessionEnded(null); + }, [endSession]); + // Finding #4 in the 9USRCraft pen test: an authenticated session stayed usable // indefinitely. Signs out after 30 minutes with no activity in any tab. useIdleLogout(!!session, signOut); diff --git a/SW.Bitween.Web/ClientApp/src/auth/useIdleLogout.ts b/SW.Bitween.Web/ClientApp/src/auth/useIdleLogout.ts index ba1130e3..53e60537 100644 --- a/SW.Bitween.Web/ClientApp/src/auth/useIdleLogout.ts +++ b/SW.Bitween.Web/ClientApp/src/auth/useIdleLogout.ts @@ -22,8 +22,6 @@ const writeSharedActivity = (ts: number) => { } }; -const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); - /** * Signs the user out after a period with no activity in any tab. Runs the normal * sign-out path, so an idle session ends exactly like clicking Sign out. @@ -67,20 +65,10 @@ export function useIdleLogout( window.clearInterval(interval); ACTIVITY_EVENTS.forEach((e) => window.removeEventListener(e, markActivity)); - try { - await signOutRef.current(); - } catch { - // Only the server call invalidates the refresh-token cookie (it is HttpOnly, - // so JS cannot clear it). Retry once before giving up. - await sleep(2000); - try { - await signOutRef.current(); - } catch { - // api.logout() clears the stored Jwt in a finally, so it is already gone by - // now; reloading is what drops the in-memory session and shows the login page. - window.location.reload(); - } - } + // No retry, and nothing to reload around: `signOut` ends the session before it + // calls the server and swallows a failed call, so this cannot leave the user + // looking at the app. It used to need both. + await signOutRef.current(); }, CHECK_INTERVAL_MS); return () => { From b664145da03bad3795745a7b8defdfacdd16d6bb Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Sun, 6 Sep 2026 11:46:08 +0300 Subject: [PATCH 3/3] fix: cancel a pending sign-out instead of waiting for it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making login() await the in-flight logout guarded the Clear-Site-Data race but introduced a worse failure: a logout that hangs rather than fails would block signing back in for as long as it hung. Nothing in the response is worth waiting for — the request was already sent, so the server deletes the refresh token either way — so a sign-in now aborts it, and the header we don't want never arrives. Also discards session reads that resolve after the session ended. A held /accounts/profile returns 200 with a Jwt captured before a sign-out, so its result was applied on arrival and flashed the app back over a dead session until the 401 handler caught up. Adds e2e/sign-out.spec.ts covering both races plus the failure modes from the previous commit. --- SW.Bitween.Web/ClientApp/e2e/sign-out.spec.ts | 151 ++++++++++++++++++ .../ClientApp/src/api/http/request.ts | 3 + .../ClientApp/src/api/http/session.ts | 55 ++++--- .../ClientApp/src/auth/SessionContext.tsx | 21 ++- 4 files changed, 205 insertions(+), 25 deletions(-) create mode 100644 SW.Bitween.Web/ClientApp/e2e/sign-out.spec.ts diff --git a/SW.Bitween.Web/ClientApp/e2e/sign-out.spec.ts b/SW.Bitween.Web/ClientApp/e2e/sign-out.spec.ts new file mode 100644 index 00000000..60b11b84 --- /dev/null +++ b/SW.Bitween.Web/ClientApp/e2e/sign-out.spec.ts @@ -0,0 +1,151 @@ +import { expect, test, type Page } from "@playwright/test"; +import { ADMIN_EMAIL, ADMIN_PASSWORD } from "./helpers"; + +/** + * Signing out, and the four ways it used to go wrong. + * + * The session lives in four places — a row in the database, the HttpOnly refresh + * cookie, the Jwt in localStorage, and React's own copy. Only the last one decides + * what you see, and it used to be the one thing a sign-out could fail to clear: + * `signOut` awaited the server first, so any failure threw before the session was + * ended and left the whole app on screen with the Jwt already deleted. Every test + * here is a way that could happen. + */ + +const submitCredentials = async (page: Page) => { + await page.fill("#login-email", ADMIN_EMAIL); + await page.fill("#login-password", ADMIN_PASSWORD); + await page.getByRole("button", { name: /^Sign in$/ }).click(); +}; + +async function signIn(page: Page) { + await page.goto("login"); + await submitCredentials(page); + await page.waitForURL((u) => !u.pathname.endsWith("/login"), { timeout: 15000 }); +} + +const signOut = async (page: Page) => { + await page.getByRole("button", { name: "Account menu" }).click(); + await page.getByRole("button", { name: /Sign out/ }).click(); +}; + +/** The signed-in shell: present only while React holds a session. */ +const shell = (page: Page) => page.getByRole("button", { name: "Account menu" }); + +test("the login page appears without waiting for the server", async ({ page }) => { + await signIn(page); + await page.route("**/api/accounts/logout", async (r) => { + await new Promise((res) => setTimeout(res, 3000)); + await r.continue(); + }); + const started = Date.now(); + await signOut(page); + // Held for 3s on purpose: the session ends locally first, so nothing waits on it. + await page.waitForURL(/\/login/, { timeout: 2500 }); + expect(Date.now() - started).toBeLessThan(2500); +}); + +test("a refused sign-out still signs you out", async ({ page }) => { + await signIn(page); + await page.route("**/api/accounts/logout", (r) => r.fulfill({ status: 500, body: "boom" })); + await signOut(page); + await page.waitForURL(/\/login/, { timeout: 5000 }); + expect(await page.evaluate(() => localStorage.getItem("access_token"))).toBeNull(); +}); + +test("an unreachable backend still signs you out", async ({ page }) => { + await signIn(page); + await page.route("**/api/accounts/logout", (r) => r.abort("connectionrefused")); + await signOut(page); + await page.waitForURL(/\/login/, { timeout: 5000 }); +}); + +test("signing out in one tab ends the session in the other", async ({ page, context }) => { + await signIn(page); + const other = await context.newPage(); + await other.goto("https://localhost:7155/partners"); + await other.waitForTimeout(2000); + expect(await shell(other).count()).toBe(1); + + await signOut(page); + await page.waitForURL(/\/login/, { timeout: 5000 }); + // Both credentials are shared by every tab, so the session really has ended here + // too — this tab just never used to find out until someone pressed refresh. + await other.waitForURL(/\/login/, { timeout: 8000 }); +}); + +test("a sign-out that never answers does not block signing back in", async ({ page }) => { + await signIn(page); + await page.route("**/api/accounts/logout", () => { + /* never fulfilled: the request hangs rather than failing */ + }); + await signOut(page); + await page.waitForURL(/\/login/, { timeout: 5000 }); + + // A sign-in cancels the pending sign-out rather than waiting for it. Waiting was + // the obvious guard against the race in the next test, and would have deadlocked + // here for as long as the request hung. + await submitCredentials(page); + await page.waitForURL((u) => !u.pathname.endsWith("/login"), { timeout: 8000 }); + expect(await shell(page).count()).toBe(1); +}); + +test("a slow sign-out response cannot wipe the session that replaced it", async ({ page }) => { + await signIn(page); + // The logout response carries `Clear-Site-Data: "cookies", "storage"`, which the + // browser applies to the whole origin whenever it lands — including over a newer + // sign-in. Cancelling the request means the response never arrives. + await page.route("**/api/accounts/logout", async (r) => { + await new Promise((res) => setTimeout(res, 4000)); + await r.continue(); + }); + await signOut(page); + await page.waitForURL(/\/login/, { timeout: 5000 }); + await submitCredentials(page); + await page.waitForURL((u) => !u.pathname.endsWith("/login"), { timeout: 10000 }); + + await page.waitForTimeout(5000); // outlast the delayed response + expect(page.url()).not.toContain("/login"); + expect(await page.evaluate(() => localStorage.getItem("access_token"))).not.toBeNull(); + await page.goto("partners"); + await page.waitForTimeout(1500); + expect(await shell(page).count()).toBe(1); +}); + +test("a slow session read cannot flash the app back after a sign-out", async ({ page, context }) => { + await signIn(page); + + // Held open so it lands after the sign-out below. It returns 200 — the Jwt went + // out before the sign-out and is still valid — so its result must be discarded on + // arrival rather than trusted, or the app appears again over a dead session. + await page.route("**/api/accounts/profile", async (r) => { + await new Promise((res) => setTimeout(res, 4000)); + await r.continue(); + }); + const reloading = page.reload(); + await page.waitForTimeout(800); + + const other = await context.newPage(); + await other.goto("https://localhost:7155/partners"); + await other.waitForTimeout(1500); + await signOut(other); + await other.waitForURL(/\/login/, { timeout: 8000 }); + + let everSignedIn = false; + for (let i = 0; i < 60; i++) { + if (await shell(page).count()) everSignedIn = true; + await page.waitForTimeout(100); + } + await reloading.catch(() => {}); + expect(everSignedIn).toBe(false); + expect(page.url()).toContain("/login"); +}); + +test("a session that ended server-side does not need a refresh", async ({ page }) => { + await signIn(page); + // Exactly what a sign-out elsewhere leaves behind: no cookie, a useless Jwt. + await page.context().clearCookies(); + await page.evaluate(() => localStorage.setItem("access_token", "dead")); + await page.getByRole("link", { name: /^Partners$/ }).click(); + await page.waitForURL(/\/login/, { timeout: 10000 }); +}); diff --git a/SW.Bitween.Web/ClientApp/src/api/http/request.ts b/SW.Bitween.Web/ClientApp/src/api/http/request.ts index e241d5c0..415ef585 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/request.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/request.ts @@ -103,6 +103,8 @@ export interface RequestOptions { body?: unknown; /** Internal: prevents the 401 → refresh → retry loop from recursing. */ _retried?: boolean; + /** Lets a caller cancel the request — only `logout` needs this, and says why. */ + signal?: AbortSignal; } /** @@ -126,6 +128,7 @@ export async function request(path: string, opts: RequestOptions = {}): Promi ...(token ? { Authorization: `Bearer ${token}` } : {}), }, body: sendJson ? JSON.stringify(opts.body ?? {}) : undefined, + ...(opts.signal ? { signal: opts.signal } : {}), }); if (res.status === 401 && !opts._retried) { diff --git a/SW.Bitween.Web/ClientApp/src/api/http/session.ts b/SW.Bitween.Web/ClientApp/src/api/http/session.ts index 5261e390..7c4c4378 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/session.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/session.ts @@ -1,6 +1,6 @@ import { ApiRequestError, type Session, type User } from "../types"; import { getAppConfig } from "./appConfig"; -import { clearToken, get, getToken, post, setToken } from "./request"; +import { clearToken, get, getToken, post, request, setToken } from "./request"; /** GET /accounts/profile — camelCase ProfileModel. */ interface Profile { @@ -43,16 +43,24 @@ const buildSession = (profile: Profile): Session => { const loadSession = async (): Promise => buildSession(await get("/accounts/profile")); /** - * The most recent sign-out's server call, which every sign-in waits for. + * The sign-out request that may still be in flight, so a sign-in can cancel it. * - * The logout response carries `Clear-Site-Data: "cookies", "storage"`. Arriving - * *after* a fresh sign-in it wipes that sign-in's Jwt and refresh cookie, and the - * user is thrown back to the page they just left. The two can genuinely overlap - * now that signing out no longer blocks the UI: the login page appears at once, - * and a password manager can fill and submit it before the logout lands. Awaiting - * a promise that has already settled costs nothing, so this is never cleared. + * Its response carries `Clear-Site-Data: "cookies", "storage"`, which the browser + * applies to the whole origin the moment it arrives — so landing *after* a fresh + * sign-in, it wipes that sign-in's Jwt and refresh cookie and throws the user back + * to the page they just left. The two genuinely can overlap now that signing out + * no longer blocks the UI: the login page appears at once, and a password manager + * can fill and submit it before the logout lands. + * + * Waiting for the logout first was the obvious guard and the wrong one — a request + * that hangs rather than fails would block sign-in for as long as it hung. Nothing + * in the response is worth waiting for: it was already sent, so the server deletes + * the refresh token either way, and only the header we don't want is discarded. */ -let lastLogout: Promise | null = null; +let logoutInFlight: AbortController | null = null; + +/** A sign-in supersedes any sign-out still in flight. */ +const abortPendingLogout = () => logoutInFlight?.abort(); export const sessionMethods = { async getSession(): Promise { @@ -69,7 +77,7 @@ export const sessionMethods = { }, async login(email: string, password: string): Promise { - await lastLogout; + abortPendingLogout(); const { jwt } = await post("/accounts/login", { Username: email, Password: password, @@ -79,7 +87,7 @@ export const sessionMethods = { }, async loginWithMicrosoft(): Promise { - await lastLogout; + abortPendingLogout(); const cfg = await getAppConfig(); if (!cfg.msalClientId) throw new ApiRequestError("MS_NOT_CONFIGURED", "Microsoft sign-in isn't configured."); @@ -114,18 +122,19 @@ export const sessionMethods = { // rendering as if signed in. Removing the key here is also what wakes the // other tabs (see the `storage` listener in SessionContext). clearToken(); - lastLogout = (async () => { - try { - await post("/accounts/logout"); - } catch { - // Swallowed on purpose: signing out must not depend on the server - // answering. Only the server can invalidate the refresh cookie — it is - // HttpOnly, so JS cannot touch it — so if this never lands the cookie - // outlives the sign-out. This browser has no Jwt to pair with it, and the - // next sign-in replaces it. - } - })(); - return lastLogout; + const controller = new AbortController(); + logoutInFlight = controller; + try { + await request("/accounts/logout", { method: "POST", signal: controller.signal }); + } catch { + // Swallowed on purpose: signing out must not depend on the server answering, + // and a sign-in aborting this is a normal outcome rather than a fault. Only + // the server can invalidate the refresh cookie — it is HttpOnly, so JS cannot + // touch it — so if this never lands the cookie outlives the sign-out. This + // browser has no Jwt to pair with it, and the next sign-in replaces it. + } finally { + if (logoutInFlight === controller) logoutInFlight = null; + } }, async updateProfile(changes: { displayName: string }): Promise { diff --git a/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx b/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx index 0014d6b8..18088f4e 100644 --- a/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx +++ b/SW.Bitween.Web/ClientApp/src/auth/SessionContext.tsx @@ -4,6 +4,7 @@ import { useContext, useEffect, useMemo, + useRef, useState, type ReactNode, } from "react"; @@ -31,17 +32,30 @@ export function SessionProvider({ children }: { children: ReactNode }) { const [session, setSession] = useState(null); const [initializing, setInitializing] = useState(true); const queryClient = useQueryClient(); + /** + * Bumped whenever the session is authoritatively replaced or ended. + * + * `getSession()` cannot be cancelled, so a slow one can resolve *after* a + * sign-out and hand `setSession` the very session that was just discarded — + * signing the user back in. Each caller notes the generation it started under + * and drops its own result if that is no longer current. + */ + const generation = useRef(0); useEffect(() => { + const startedAt = generation.current; api .getSession() - .then(setSession) + .then((next) => { + if (generation.current === startedAt) setSession(next); + }) .finally(() => setInitializing(false)); }, []); const adoptSession = useCallback( (next: Session) => { // a different identity invalidates everything previously fetched + generation.current += 1; queryClient.clear(); setSession(next); }, @@ -64,11 +78,14 @@ export function SessionProvider({ children }: { children: ReactNode }) { }, [adoptSession]); const refresh = useCallback(async () => { - setSession(await api.getSession()); + const startedAt = generation.current; + const next = await api.getSession(); + if (generation.current === startedAt) setSession(next); }, []); /** Ends the session locally, then tells the server. Never the other way round. */ const endSession = useCallback(() => { + generation.current += 1; setSession(null); queryClient.clear(); }, [queryClient]);