From 4d87a4e15e8899c6249f43ee28c88059a37b8b16 Mon Sep 17 00:00:00 2001 From: Evan Pelle Date: Mon, 29 Jun 2026 15:06:25 +0000 Subject: [PATCH 1/6] fix(auth): stop transient failures from logging players out / wiping identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Players reported being "logged out every few days." The root cause is that the client's logOut() is unconditionally destructive — it revokes the server session (POST /auth/logout) AND removes the localStorage persistent_id plus cosmetics — and it was invoked on transient, recoverable failures. A non-200 from POST /auth/refresh is a *resolved* fetch (Cloudflare 5xx/ 520-524, a 429, or a spurious edge 401), so it bypasses the network-error catch path hardened in #2636 and hit logOut(). For a guest this wiped the persistent_id and cleared the refresh cookie, so the next refresh was sent cookie-less and the backend silently minted a brand-new guest (identity changes, no error). For linked accounts it forced an unnecessary re-login. getUserMe() did the same on a 401 from /users/@me on every app open. Changes: - doRefreshJwt(): on a non-200, clear __jwt only (mirror the network-error path); do not revoke the session or wipe identity. - getUserMe(): treat a 401 like any other non-200 (return false); no logOut(). - logOut(): gate the persistent_id / cosmetics wipe behind a new userInitiated flag so only an explicit user logout destroys local identity. This also protects the remaining error-path callers (commerce 401s, iss/aud mismatch) for free. - AccountModal.handleLogout(): pass userInitiated=true (the real logout button). - Add tests/client/Auth.test.ts covering both behaviors. Note: the CrazyGames third-party-iframe SameSite=Lax cookie loss is a separate account-persistence issue and is intentionally not addressed here. Co-Authored-By: Claude Opus 4.8 --- src/client/AccountModal.ts | 2 +- src/client/Api.ts | 9 ++--- src/client/Auth.ts | 22 ++++++++++--- tests/client/Auth.test.ts | 67 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 tests/client/Auth.test.ts diff --git a/src/client/AccountModal.ts b/src/client/AccountModal.ts index 6fa65a669a..aea3f78174 100644 --- a/src/client/AccountModal.ts +++ b/src/client/AccountModal.ts @@ -585,7 +585,7 @@ export class AccountModal extends BaseModal { } private async handleLogout() { - await logOut(); + await logOut(false, true); this.close(); // Refresh the page after logout to update the UI state window.location.reload(); diff --git a/src/client/Api.ts b/src/client/Api.ts index 9659ed9a1a..cdbbfcc78b 100644 --- a/src/client/Api.ts +++ b/src/client/Api.ts @@ -70,10 +70,11 @@ export async function getUserMe(): Promise { authorization: `Bearer ${jwt}`, }, }); - if (response.status === 401) { - await logOut(); - return false; - } + // A 401 here is treated like any other non-200 (return false). We + // deliberately do NOT logOut(): the JWT was just refreshed by userAuth(), + // so a 401 from /users/@me is transient/ambiguous and must not revoke the + // session or wipe the persistent identity. The backend already made + // /users/@me 401 non-authoritative. if (response.status !== 200) return false; const body = await response.json(); const result = UserMeResponseSchema.safeParse(body); diff --git a/src/client/Auth.ts b/src/client/Auth.ts index e2b51e3829..4f4f4fb2cb 100644 --- a/src/client/Auth.ts +++ b/src/client/Auth.ts @@ -77,7 +77,10 @@ export async function getAuthHeader(): Promise { return `Bearer ${jwt}`; } -export async function logOut(allSessions: boolean = false): Promise { +export async function logOut( + allSessions: boolean = false, + userInitiated: boolean = false, +): Promise { try { const response = await fetch( getApiBase() + (allSessions ? "/auth/revoke" : "/auth/logout"), @@ -98,9 +101,14 @@ export async function logOut(allSessions: boolean = false): Promise { return false; } finally { __jwt = null; - localStorage.removeItem(PERSISTENT_ID_KEY); - new UserSettings().clearFlag(); - new UserSettings().setSelectedPatternName(undefined); + // Only destroy the local persistent identity / cosmetics on an explicit + // user logout. Error-path callers must NOT wipe identity, or a transient + // failure turns into a permanent brand-new guest account. + if (userInitiated) { + localStorage.removeItem(PERSISTENT_ID_KEY); + new UserSettings().clearFlag(); + new UserSettings().setSelectedPatternName(undefined); + } } } @@ -197,7 +205,11 @@ async function doRefreshJwt(): Promise { }); if (response.status !== 200) { console.error("Refresh failed", response); - logOut(); + // A non-200 here is usually transient (Cloudflare 5xx/520-524, 429 + // rate-limit) or an ambiguous edge error. Do NOT revoke the session or + // wipe the persistent identity for it — mirror the network-error path + // below and let the next refresh recover. + __jwt = null; return; } const json = await response.json(); diff --git a/tests/client/Auth.test.ts b/tests/client/Auth.test.ts new file mode 100644 index 0000000000..7f0ae5a836 --- /dev/null +++ b/tests/client/Auth.test.ts @@ -0,0 +1,67 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// Auth.ts derives the API origin from window.location via ./Api. Pin it so the +// fetch URLs are deterministic in the jsdom environment. +vi.mock("../../src/client/Api", () => ({ + getApiBase: () => "http://localhost:8787", + getAudience: () => "localhost", +})); + +import { logOut, userAuth } from "../../src/client/Auth"; + +const PERSISTENT_ID_KEY = "player_persistent_id"; + +describe("Auth: transient failures must not destroy identity", () => { + beforeEach(() => { + localStorage.clear(); + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it("preserves the persistent ID and session when /auth/refresh returns a transient non-200", async () => { + localStorage.setItem(PERSISTENT_ID_KEY, "keep-me-123"); + const fetchMock = vi.fn(async (url: unknown) => { + if (String(url).includes("/auth/refresh")) { + return { status: 503, ok: false, json: async () => ({}) }; + } + return { status: 200, ok: true, json: async () => ({}) }; + }); + vi.stubGlobal("fetch", fetchMock); + + const result = await userAuth(); + + // Not authenticated after a failed refresh... + expect(result).toBe(false); + // ...but the persistent identity survives so the next refresh can recover. + expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-123"); + // A transient refresh failure must not revoke the session. + const postedLogout = fetchMock.mock.calls.some(([u]) => + String(u).includes("/auth/logout"), + ); + expect(postedLogout).toBe(false); + }); + + it("wipes local identity only on an explicit user-initiated logOut", async () => { + const fetchMock = vi.fn(async () => ({ + status: 200, + ok: true, + json: async () => ({}), + })); + vi.stubGlobal("fetch", fetchMock); + + // Error-path / programmatic logout must preserve the persistent identity. + localStorage.setItem(PERSISTENT_ID_KEY, "keep-me-456"); + await logOut(); + expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-456"); + + // The real "Log out" button passes userInitiated=true and clears identity. + await logOut(false, true); + expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBeNull(); + }); +}); From 7bf2de42f2230cef8877f57495410f7b1be284c9 Mon Sep 17 00:00:00 2001 From: Evan Pelle Date: Mon, 29 Jun 2026 15:43:51 +0000 Subject: [PATCH 2/6] feat(auth): retry transient refreshes; soft-logout + warning modal on session expiry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the transient-failure fix: instead of silently dropping to a guest identity when a JWT can't be obtained, the client now distinguishes WHY the refresh failed and reacts appropriately. doRefreshJwt() now: - Retries transient failures (network error / 5xx / 429) up to 3x with backoff (0.5s, 1s), behind the existing single-flight guard, so a brief edge blip no longer leaves a logged-in user unauthenticated at join time. - Treats 401/403 as definitive (refresh token genuinely dead): a "soft" logout — clears the in-memory JWT but preserves the session cookie + persistent identity — and, if a session was active, dispatches `auth-session-expired`. - Exposes getLastRefreshOutcome() so callers can tell "expired" from "transient". UX: - New : on `auth-session-expired`, prompts re-login — but ONLY for users who were actually signed in to an account (tracked via Api.wasLoggedIn(), set from the last /users/@me). Guests never see it. - Ranked matchmaking shows "couldn't verify your login, try again" on a transient failure instead of the misleading "must login" + account bounce. Adds en.json strings (session_expired.*, matchmaking_button.connection_issue) and extends tests/client/Auth.test.ts: retry count, transient vs definitive outcome, identity preservation, and the active-session expiry dispatch. Co-Authored-By: Claude Opus 4.8 --- index.html | 1 + resources/lang/en.json | 7 ++ src/client/Api.ts | 10 +++ src/client/Auth.ts | 106 +++++++++++++++++++++------- src/client/Main.ts | 1 + src/client/Matchmaking.ts | 17 ++++- src/client/SessionExpiredModal.ts | 66 ++++++++++++++++++ tests/client/Auth.test.ts | 111 ++++++++++++++++++++++++------ 8 files changed, 273 insertions(+), 46 deletions(-) create mode 100644 src/client/SessionExpiredModal.ts diff --git a/index.html b/index.html index 4af07ad2d6..d0761c9cba 100644 --- a/index.html +++ b/index.html @@ -350,6 +350,7 @@ + diff --git a/resources/lang/en.json b/resources/lang/en.json index 187677aa90..e88171ba4a 100644 --- a/resources/lang/en.json +++ b/resources/lang/en.json @@ -994,6 +994,7 @@ "unfavorite": "Remove from favourites" }, "matchmaking_button": { + "connection_issue": "Couldn't verify your login. Please try again.", "description": "(ALPHA)", "login_required": "Login to play ranked!", "must_login": "You must be logged in to play ranked matchmaking.", @@ -1221,6 +1222,12 @@ "slider_tooltip": "{percent}% • {amount}", "title_with_name": "Send Troops to {name}" }, + "session_expired": { + "body": "Your session has expired. Please log in again to continue.", + "dismiss": "Dismiss", + "log_in": "Log in", + "title": "Signed out" + }, "single_modal": { "bots": "Tribes: ", "bots_disabled": "Disabled", diff --git a/src/client/Api.ts b/src/client/Api.ts index cdbbfcc78b..4675e7ef5a 100644 --- a/src/client/Api.ts +++ b/src/client/Api.ts @@ -53,6 +53,15 @@ export async function fetchPlayerById( } } +// Remembers whether the last successful /users/@me showed a linked account. +// Used to decide whether a session-expiry should prompt re-login (linked users) +// or be handled silently (guests). Survives a later auth failure on purpose — +// it records what the user *was*, which is exactly what we need at that point. +let __lastKnownLinked = false; +export function wasLoggedIn(): boolean { + return __lastKnownLinked; +} + let __userMe: Promise | null = null; export async function getUserMe(): Promise { if (__userMe !== null) { @@ -83,6 +92,7 @@ export async function getUserMe(): Promise { console.error("Invalid response", error); return false; } + __lastKnownLinked = hasLinkedAccount(result.data); return result.data; } catch (e) { return false; diff --git a/src/client/Auth.ts b/src/client/Auth.ts index 4f4f4fb2cb..a45ed15982 100644 --- a/src/client/Auth.ts +++ b/src/client/Auth.ts @@ -196,33 +196,91 @@ async function refreshJwt(): Promise { } } +// Outcome of the most recent refresh attempt, so callers (e.g. ranked +// matchmaking) can tell "your session expired" apart from "we couldn't reach +// the auth server right now". +export type RefreshOutcome = "ok" | "expired" | "transient"; +let __lastRefreshOutcome: RefreshOutcome = "ok"; + +export function getLastRefreshOutcome(): RefreshOutcome { + return __lastRefreshOutcome; +} + +const REFRESH_MAX_ATTEMPTS = 3; + +function refreshBackoffMs(attempt: number): number { + // Backoff between attempts: 500ms, then 1000ms. + return 500 * 2 ** (attempt - 1); +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function notifySessionExpired(): void { + if (typeof window === "undefined") return; + window.dispatchEvent(new CustomEvent("auth-session-expired")); +} + async function doRefreshJwt(): Promise { - try { - console.log("Refreshing jwt"); - const response = await fetch(getApiBase() + "/auth/refresh", { - method: "POST", - credentials: "include", - }); - if (response.status !== 200) { - console.error("Refresh failed", response); - // A non-200 here is usually transient (Cloudflare 5xx/520-524, 429 - // rate-limit) or an ambiguous edge error. Do NOT revoke the session or - // wipe the persistent identity for it — mirror the network-error path - // below and let the next refresh recover. - __jwt = null; - return; + // Whether an authenticated session was active before this attempt. Only a + // session that dies mid-use should surface the "signed out" UI. + const hadSession = __jwt !== null; + + for (let attempt = 1; attempt <= REFRESH_MAX_ATTEMPTS; attempt++) { + try { + console.log( + `Refreshing jwt (attempt ${attempt}/${REFRESH_MAX_ATTEMPTS})`, + ); + const response = await fetch(getApiBase() + "/auth/refresh", { + method: "POST", + credentials: "include", + }); + + if (response.status === 200) { + const json = await response.json(); + const { jwt, expiresIn } = json; + __expiresAt = Date.now() + expiresIn * 1000; + __jwt = jwt; + __lastRefreshOutcome = "ok"; + console.log("Refresh succeeded"); + return; + } + + // 401/403 are definitive: the refresh token is genuinely invalid or + // expired, so retrying can't help. Do a "soft" logout — clear the + // in-memory JWT but preserve the session cookie + persistent identity — + // and let a previously-signed-in user be prompted to log in again. + if (response.status === 401 || response.status === 403) { + console.error("Refresh rejected — session expired", response.status); + __jwt = null; + __lastRefreshOutcome = "expired"; + if (hadSession) notifySessionExpired(); + return; + } + + // Everything else (5xx, 429, ...) is transient — fall through to retry. + console.error( + `Refresh failed (status ${response.status}), attempt ${attempt}/${REFRESH_MAX_ATTEMPTS}`, + ); + } catch (e) { + // Network error / server unreachable — transient, fall through to retry. + console.error( + `Refresh failed (network), attempt ${attempt}/${REFRESH_MAX_ATTEMPTS}`, + e, + ); + } + + if (attempt < REFRESH_MAX_ATTEMPTS) { + await delay(refreshBackoffMs(attempt)); } - const json = await response.json(); - const { jwt, expiresIn } = json; - __expiresAt = Date.now() + expiresIn * 1000; - console.log("Refresh succeeded"); - __jwt = jwt; - } catch (e) { - console.error("Refresh failed", e); - // if server unreachable, just clear jwt - __jwt = null; - return; } + + // Transient failures exhausted. Clear the in-memory JWT only — keep the + // session cookie and persistent identity so the next refresh can recover. + console.error("Refresh failed after retries; staying recoverable"); + __jwt = null; + __lastRefreshOutcome = "transient"; } export async function sendMagicLink(email: string): Promise { diff --git a/src/client/Main.ts b/src/client/Main.ts index b3dabb9ada..4975389c87 100644 --- a/src/client/Main.ts +++ b/src/client/Main.ts @@ -43,6 +43,7 @@ import { modalRouter } from "./ModalRouter"; import { initNavigation } from "./Navigation"; import "./NewsModal"; import "./PatternInput"; +import "./SessionExpiredModal"; import "./SinglePlayerModal"; import { StoreModal } from "./Store"; import "./TerritoryPatternsModal"; diff --git a/src/client/Matchmaking.ts b/src/client/Matchmaking.ts index ed848430e1..a6c2250b76 100644 --- a/src/client/Matchmaking.ts +++ b/src/client/Matchmaking.ts @@ -3,7 +3,7 @@ import { customElement, state } from "lit/decorators.js"; import { ClientEnv } from "src/client/ClientEnv"; import { UserMeResponse } from "../core/ApiSchemas"; import { getUserMe, hasLinkedAccount } from "./Api"; -import { getPlayToken } from "./Auth"; +import { getLastRefreshOutcome, getPlayToken } from "./Auth"; import { BaseModal } from "./components/BaseModal"; import "./components/Difficulties"; import { modalHeader } from "./components/ui/ModalHeader"; @@ -126,6 +126,21 @@ export class MatchmakingModal extends BaseModal { userMe.user.google !== undefined || userMe.user.email !== undefined); if (!isLoggedIn) { + // A transient auth-server hiccup (rather than a genuine "not logged in") + // shouldn't bounce the player to the login page — ask them to retry. + if (getLastRefreshOutcome() === "transient") { + window.dispatchEvent( + new CustomEvent("show-message", { + detail: { + message: translateText("matchmaking_button.connection_issue"), + color: "red", + duration: 3000, + }, + }), + ); + this.close(); + return; + } window.dispatchEvent( new CustomEvent("show-message", { detail: { diff --git a/src/client/SessionExpiredModal.ts b/src/client/SessionExpiredModal.ts new file mode 100644 index 0000000000..d4e24dfa23 --- /dev/null +++ b/src/client/SessionExpiredModal.ts @@ -0,0 +1,66 @@ +import { html } from "lit"; +import { customElement } from "lit/decorators.js"; +import { wasLoggedIn } from "./Api"; +import { BaseModal, ModalConfig } from "./components/BaseModal"; +import { translateText } from "./Utils"; + +/** + * App-level warning shown when a previously-signed-in user's auth session + * expires (a definitive 401/403 on /auth/refresh). Auth.ts dispatches the + * "auth-session-expired" window event; we only surface it for users who were + * actually logged in to an account — guests get a fresh session silently. + */ +@customElement("session-expired-modal") +export class SessionExpiredModal extends BaseModal { + connectedCallback(): void { + super.connectedCallback(); + window.addEventListener("auth-session-expired", this.onSessionExpired); + } + + disconnectedCallback(): void { + window.removeEventListener("auth-session-expired", this.onSessionExpired); + super.disconnectedCallback(); + } + + private onSessionExpired = (): void => { + if (!wasLoggedIn()) return; + if (this.isOpen()) return; + this.open(); + }; + + protected modalConfig(): ModalConfig { + return { + title: translateText("session_expired.title"), + hideHeader: false, + hideCloseButton: false, + maxWidth: "420px", + }; + } + + private logIn(): void { + this.close(); + window.showPage?.("page-account"); + } + + protected renderBody() { + return html` +
+

${translateText("session_expired.body")}

+
+ + +
+
+ `; + } +} diff --git a/tests/client/Auth.test.ts b/tests/client/Auth.test.ts index 7f0ae5a836..6c3526074a 100644 --- a/tests/client/Auth.test.ts +++ b/tests/client/Auth.test.ts @@ -7,11 +7,25 @@ vi.mock("../../src/client/Api", () => ({ getAudience: () => "localhost", })); -import { logOut, userAuth } from "../../src/client/Auth"; +import { getLastRefreshOutcome, logOut, userAuth } from "../../src/client/Auth"; const PERSISTENT_ID_KEY = "player_persistent_id"; -describe("Auth: transient failures must not destroy identity", () => { +// Build a decodeable JWT whose `iss` matches getApiBase() so userAuth()'s +// claim checks pass. Only the payload matters to decodeJwt. +function fakeJwt(payload: Record): string { + const b64 = (o: unknown) => + Buffer.from(JSON.stringify(o)).toString("base64url"); + return `${b64({ alg: "none" })}.${b64(payload)}.sig`; +} + +const okJson = (data: unknown) => ({ + status: 200, + ok: true, + json: async () => data, +}); + +describe("Auth: refresh resilience and session-expiry handling", () => { beforeEach(() => { localStorage.clear(); vi.spyOn(console, "error").mockImplementation(() => {}); @@ -24,44 +38,99 @@ describe("Auth: transient failures must not destroy identity", () => { vi.unstubAllGlobals(); }); - it("preserves the persistent ID and session when /auth/refresh returns a transient non-200", async () => { + it("retries transient (5xx) refresh failures, then preserves identity without logging out", async () => { localStorage.setItem(PERSISTENT_ID_KEY, "keep-me-123"); const fetchMock = vi.fn(async (url: unknown) => { if (String(url).includes("/auth/refresh")) { return { status: 503, ok: false, json: async () => ({}) }; } - return { status: 200, ok: true, json: async () => ({}) }; + return okJson({}); }); vi.stubGlobal("fetch", fetchMock); const result = await userAuth(); - // Not authenticated after a failed refresh... expect(result).toBe(false); - // ...but the persistent identity survives so the next refresh can recover. + // Transient failures are retried (3 attempts) before giving up. + const refreshCalls = fetchMock.mock.calls.filter(([u]) => + String(u).includes("/auth/refresh"), + ).length; + expect(refreshCalls).toBe(3); + expect(getLastRefreshOutcome()).toBe("transient"); + // Identity preserved and the session is never revoked on a transient blip. expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-123"); - // A transient refresh failure must not revoke the session. - const postedLogout = fetchMock.mock.calls.some(([u]) => - String(u).includes("/auth/logout"), - ); - expect(postedLogout).toBe(false); + expect( + fetchMock.mock.calls.some(([u]) => String(u).includes("/auth/logout")), + ).toBe(false); }); - it("wipes local identity only on an explicit user-initiated logOut", async () => { - const fetchMock = vi.fn(async () => ({ - status: 200, - ok: true, - json: async () => ({}), - })); + it("does NOT retry a definitive 401, and (no prior session) does not raise session-expired", async () => { + localStorage.setItem(PERSISTENT_ID_KEY, "keep-me-401"); + const onExpired = vi.fn(); + window.addEventListener("auth-session-expired", onExpired); + const fetchMock = vi.fn(async (url: unknown) => { + if (String(url).includes("/auth/refresh")) { + return { status: 401, ok: false, json: async () => ({}) }; + } + return okJson({}); + }); + vi.stubGlobal("fetch", fetchMock); + + const result = await userAuth(); + + expect(result).toBe(false); + // 401 is definitive — exactly one attempt, no retry. + const refreshCalls = fetchMock.mock.calls.filter(([u]) => + String(u).includes("/auth/refresh"), + ).length; + expect(refreshCalls).toBe(1); + expect(getLastRefreshOutcome()).toBe("expired"); + expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-401"); + // No active session existed, so we must not nag with the modal. + expect(onExpired).not.toHaveBeenCalled(); + window.removeEventListener("auth-session-expired", onExpired); + }); + + it("raises auth-session-expired when an ACTIVE session is rejected with a 401", async () => { + const onExpired = vi.fn(); + window.addEventListener("auth-session-expired", onExpired); + // First refresh mints a session (already-expired so the next userAuth + // re-refreshes); the second refresh is rejected. + let call = 0; + const fetchMock = vi.fn(async (url: unknown) => { + if (String(url).includes("/auth/refresh")) { + call++; + if (call === 1) { + return okJson({ + jwt: fakeJwt({ iss: "http://localhost:8787" }), + expiresIn: 0, + }); + } + return { status: 401, ok: false, json: async () => ({}) }; + } + return okJson({}); + }); vi.stubGlobal("fetch", fetchMock); - // Error-path / programmatic logout must preserve the persistent identity. + await userAuth(); // establishes __jwt via the first (200) refresh + await userAuth(); // session now active -> second refresh 401s + + expect(getLastRefreshOutcome()).toBe("expired"); + expect(onExpired).toHaveBeenCalledTimes(1); + window.removeEventListener("auth-session-expired", onExpired); + }); + + it("wipes local identity only on an explicit user-initiated logOut", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => okJson({})), + ); + localStorage.setItem(PERSISTENT_ID_KEY, "keep-me-456"); - await logOut(); + await logOut(); // error-path / programmatic logout expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-456"); - // The real "Log out" button passes userInitiated=true and clears identity. - await logOut(false, true); + await logOut(false, true); // the real "Log out" button expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBeNull(); }); }); From 766bf782c61795eaa276668278fae603092a6b71 Mon Sep 17 00:00:00 2001 From: Evan Pelle Date: Mon, 29 Jun 2026 20:18:13 +0000 Subject: [PATCH 3/6] refactor(auth): logOut takes an options object instead of positional booleans logOut(false, true) was opaque at the call site. Replace the two positional booleans with a LogOutOptions object so intent is explicit: logOut({ userInitiated: true }) // the real "Log out" button logOut() // error-path / programmatic (no identity wipe) No behavior change; all existing no-arg callers are unaffected (options default to {}). Co-Authored-By: Claude Opus 4.8 --- src/client/AccountModal.ts | 2 +- src/client/Auth.ts | 20 ++++++++++++++++---- tests/client/Auth.test.ts | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/client/AccountModal.ts b/src/client/AccountModal.ts index aea3f78174..567c78bc89 100644 --- a/src/client/AccountModal.ts +++ b/src/client/AccountModal.ts @@ -585,7 +585,7 @@ export class AccountModal extends BaseModal { } private async handleLogout() { - await logOut(false, true); + await logOut({ userInitiated: true }); this.close(); // Refresh the page after logout to update the UI state window.location.reload(); diff --git a/src/client/Auth.ts b/src/client/Auth.ts index a45ed15982..25d25b8bc9 100644 --- a/src/client/Auth.ts +++ b/src/client/Auth.ts @@ -77,10 +77,22 @@ export async function getAuthHeader(): Promise { return `Bearer ${jwt}`; } -export async function logOut( - allSessions: boolean = false, - userInitiated: boolean = false, -): Promise { +export interface LogOutOptions { + /** Revoke every session (/auth/revoke) instead of just the current one. */ + allSessions?: boolean; + /** + * Set only for an explicit, user-initiated logout — the one case where we + * also wipe the local persistent identity + cosmetics. Error-path callers + * must leave this false, or a transient failure becomes a permanent new + * guest account. + */ + userInitiated?: boolean; +} + +export async function logOut({ + allSessions = false, + userInitiated = false, +}: LogOutOptions = {}): Promise { try { const response = await fetch( getApiBase() + (allSessions ? "/auth/revoke" : "/auth/logout"), diff --git a/tests/client/Auth.test.ts b/tests/client/Auth.test.ts index 6c3526074a..74fca278b3 100644 --- a/tests/client/Auth.test.ts +++ b/tests/client/Auth.test.ts @@ -130,7 +130,7 @@ describe("Auth: refresh resilience and session-expiry handling", () => { await logOut(); // error-path / programmatic logout expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-456"); - await logOut(false, true); // the real "Log out" button + await logOut({ userInitiated: true }); // the real "Log out" button expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBeNull(); }); }); From 6f4a5fb34fe1d59aef344a21824bf22a9147a9d6 Mon Sep 17 00:00:00 2001 From: Evan Pelle Date: Mon, 29 Jun 2026 20:19:39 +0000 Subject: [PATCH 4/6] refactor(auth): rename wasLoggedIn() -> wasLinkedAccount() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everyone holds a session (guests included), so "logged in" was a misleading name for the modal gate. The actual question is whether the account was a linked (non-guest) one — Discord/Google/email — which is exactly what this returns. No behavior change. Co-Authored-By: Claude Opus 4.8 --- src/client/Api.ts | 2 +- src/client/SessionExpiredModal.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client/Api.ts b/src/client/Api.ts index 4675e7ef5a..53ce832166 100644 --- a/src/client/Api.ts +++ b/src/client/Api.ts @@ -58,7 +58,7 @@ export async function fetchPlayerById( // or be handled silently (guests). Survives a later auth failure on purpose — // it records what the user *was*, which is exactly what we need at that point. let __lastKnownLinked = false; -export function wasLoggedIn(): boolean { +export function wasLinkedAccount(): boolean { return __lastKnownLinked; } diff --git a/src/client/SessionExpiredModal.ts b/src/client/SessionExpiredModal.ts index d4e24dfa23..4f53b7bdc6 100644 --- a/src/client/SessionExpiredModal.ts +++ b/src/client/SessionExpiredModal.ts @@ -1,6 +1,6 @@ import { html } from "lit"; import { customElement } from "lit/decorators.js"; -import { wasLoggedIn } from "./Api"; +import { wasLinkedAccount } from "./Api"; import { BaseModal, ModalConfig } from "./components/BaseModal"; import { translateText } from "./Utils"; @@ -23,7 +23,7 @@ export class SessionExpiredModal extends BaseModal { } private onSessionExpired = (): void => { - if (!wasLoggedIn()) return; + if (!wasLinkedAccount()) return; if (this.isOpen()) return; this.open(); }; From b8d9336bbed22024d27dc0ce5bf2ae0000a4e825 Mon Sep 17 00:00:00 2001 From: Evan Pelle Date: Mon, 29 Jun 2026 20:27:06 +0000 Subject: [PATCH 5/6] chore: move session-expired-modal out of the in-game overlays block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's an app-level auth modal (relevant on the menu and in-game), not a game overlay — grouping it with multi-tab-modal etc. was misleading. Place it in its own "App-level overlays" section, still body-level (outside the in-[.in-game]: hidden container) so it renders in both contexts. No behavior change. Co-Authored-By: Claude Opus 4.8 --- index.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/index.html b/index.html index d0761c9cba..f824addc9e 100644 --- a/index.html +++ b/index.html @@ -329,6 +329,9 @@ + + + @@ -350,7 +353,6 @@ - From 3339878983042940f7c7919fe26d75f4a3d61eb9 Mon Sep 17 00:00:00 2001 From: Evan Pelle Date: Wed, 1 Jul 2026 02:20:49 +0000 Subject: [PATCH 6/6] fix(auth): address review follow-ups on transient-failure handling Root fix is at the auth layer rather than per-surface special cases: - Unify the auth-outcome signal (getLastRefreshOutcome -> getLastAuthOutcome) so it spans both /auth/refresh and /users/@me; getUserMe records its own outcome via markAuthOutcome. Matchmaking only treats a failure as transient when getUserMe couldn't determine an answer (userMe === false); a returned-but-unlinked user is a confirmed guest. - getUserMe no longer memoizes `false`, so a transient outage recovers without a page reload; the cache-clear is guarded against stale-promise clobber. - Persist "was linked" (LINKED_ACCOUNT_KEY) so the session-expiry modal still fires for linked users when /users/@me was unreachable; cleared on logout. - Validate the /auth/refresh 200 body before committing state (no more undefined jwt / NaN expiry), and add a per-attempt fetch timeout. - Stop the subscription/purchase 401 handlers from destructively logging out. - Non-blocking startup auth so deep-link joins aren't stalled. - Cleanups: o-button in SessionExpiredModal, showToast + collapsed branches in Matchmaking, backoff lookup table. Adds tests/client/Api.test.ts (no-cache-false recovery, linked persistence, non-destructive 401). Full tests/client suite (526) + tsc/eslint/prettier pass. Co-Authored-By: Claude Opus 4.8 --- src/client/Api.ts | 82 ++++++++++++---- src/client/Auth.ts | 106 +++++++++++++------- src/client/Main.ts | 21 ++-- src/client/Matchmaking.ts | 44 ++++----- src/client/SessionExpiredModal.ts | 20 ++-- tests/client/Api.test.ts | 156 ++++++++++++++++++++++++++++++ tests/client/Auth.test.ts | 8 +- 7 files changed, 332 insertions(+), 105 deletions(-) create mode 100644 tests/client/Api.test.ts diff --git a/src/client/Api.ts b/src/client/Api.ts index 53ce832166..f9672bed4f 100644 --- a/src/client/Api.ts +++ b/src/client/Api.ts @@ -11,7 +11,12 @@ import { UserMeResponseSchema, } from "../core/ApiSchemas"; import { AnalyticsRecord, AnalyticsRecordSchema } from "../core/Schemas"; -import { getAuthHeader, logOut, userAuth } from "./Auth"; +import { + getAuthHeader, + LINKED_ACCOUNT_KEY, + markAuthOutcome, + userAuth, +} from "./Auth"; export async function fetchPlayerById( playerId: string, @@ -53,13 +58,13 @@ export async function fetchPlayerById( } } -// Remembers whether the last successful /users/@me showed a linked account. -// Used to decide whether a session-expiry should prompt re-login (linked users) -// or be handled silently (guests). Survives a later auth failure on purpose — -// it records what the user *was*, which is exactly what we need at that point. -let __lastKnownLinked = false; +// Whether the signed-in user had a linked account, used to decide whether a +// session-expiry should prompt re-login (linked users) or stay silent (guests). +// Backed by localStorage (LINKED_ACCOUNT_KEY) so it survives a reload and a +// transient /users/@me outage — it records what the user *was*, which is exactly +// what we need when the session later expires. export function wasLinkedAccount(): boolean { - return __lastKnownLinked; + return localStorage.getItem(LINKED_ACCOUNT_KEY) === "true"; } let __userMe: Promise | null = null; @@ -67,9 +72,11 @@ export async function getUserMe(): Promise { if (__userMe !== null) { return __userMe; } - __userMe = (async () => { + const pending = (async () => { try { const userAuthResult = await userAuth(); + // No usable token: refreshJwt() already recorded why (transient/expired), + // so don't overwrite that outcome here. if (!userAuthResult) return false; const { jwt } = userAuthResult; @@ -79,26 +86,49 @@ export async function getUserMe(): Promise { authorization: `Bearer ${jwt}`, }, }); - // A 401 here is treated like any other non-200 (return false). We - // deliberately do NOT logOut(): the JWT was just refreshed by userAuth(), - // so a 401 from /users/@me is transient/ambiguous and must not revoke the - // session or wipe the persistent identity. The backend already made - // /users/@me 401 non-authoritative. - if (response.status !== 200) return false; + // A non-200 here (including a 401) is treated as transient/ambiguous, not + // an authoritative logout: the JWT was just validated by userAuth(), so we + // neither revoke the session nor wipe identity. We record the outcome so + // callers can offer "try again" instead of bouncing a valid session. + if (response.status !== 200) { + markAuthOutcome("transient"); + return false; + } const body = await response.json(); const result = UserMeResponseSchema.safeParse(body); if (!result.success) { const error = z.prettifyError(result.error); console.error("Invalid response", error); + markAuthOutcome("transient"); return false; } - __lastKnownLinked = hasLinkedAccount(result.data); + markAuthOutcome("ok"); + localStorage.setItem( + LINKED_ACCOUNT_KEY, + hasLinkedAccount(result.data) ? "true" : "false", + ); return result.data; } catch (e) { + // Network error reaching /users/@me — transient, not a logout. + markAuthOutcome("transient"); return false; } })(); - return __userMe; + __userMe = pending; + // Only memoize a successful result; a `false` (transient/ambiguous failure) + // must not stick for the whole page session, or recovery would need a full + // reload. Clear the cache on failure so the next call retries — but only while + // we still own the slot, so a stale in-flight request that settles after an + // invalidate + newer request can't clobber the newer one. + void pending.then( + (result) => { + if (result === false && __userMe === pending) __userMe = null; + }, + () => { + if (__userMe === pending) __userMe = null; + }, + ); + return pending; } export function invalidateUserMe() { @@ -126,7 +156,10 @@ export async function purchaseWithCurrency( }), }); if (response.status === 401) { - await logOut(); + // A 401 here is ambiguous (a transient/edge rejection is possible), so we + // return false WITHOUT logging out — a spurious 401 must not revoke the + // session. A genuinely expired token is handled by the central refresh + // path on the next authenticated call. return false; } if (!response.ok) { @@ -189,7 +222,10 @@ export async function cancelSubscription(): Promise { }, }); if (response.status === 401) { - await logOut(); + // A 401 here is ambiguous (a transient/edge rejection is possible), so we + // return false WITHOUT logging out — a spurious 401 must not revoke the + // session. A genuinely expired token is handled by the central refresh + // path on the next authenticated call. return false; } if (!response.ok) { @@ -223,7 +259,10 @@ export async function changeSubscriptionTier( }, ); if (response.status === 401) { - await logOut(); + // A 401 here is ambiguous (a transient/edge rejection is possible), so we + // return false WITHOUT logging out — a spurious 401 must not revoke the + // session. A genuinely expired token is handled by the central refresh + // path on the next authenticated call. return false; } if (!response.ok) { @@ -254,7 +293,10 @@ export async function openSubscriptionPortal(): Promise { }), }); if (response.status === 401) { - await logOut(); + // A 401 here is ambiguous (a transient/edge rejection is possible), so we + // return false WITHOUT logging out — a spurious 401 must not revoke the + // session. A genuinely expired token is handled by the central refresh + // path on the next authenticated call. return false; } if (!response.ok) { diff --git a/src/client/Auth.ts b/src/client/Auth.ts index 25d25b8bc9..9f11434f95 100644 --- a/src/client/Auth.ts +++ b/src/client/Auth.ts @@ -9,6 +9,11 @@ import { generateCryptoRandomUUID } from "./Utils"; export type UserAuth = { jwt: string; claims: TokenPayload } | false; const PERSISTENT_ID_KEY = "player_persistent_id"; +// Remembers whether the signed-in user had a linked account, so a later +// session-expiry can prompt re-login (linked users) vs. stay silent (guests) +// even when /users/@me can't be reached at that moment. Persisted (not just +// in-memory) so it survives a reload and an @me outage; cleared on logout. +export const LINKED_ACCOUNT_KEY = "was_linked_account"; let __jwt: string | null = null; let __refreshPromise: Promise | null = null; @@ -118,6 +123,7 @@ export async function logOut({ // failure turns into a permanent brand-new guest account. if (userInitiated) { localStorage.removeItem(PERSISTENT_ID_KEY); + localStorage.removeItem(LINKED_ACCOUNT_KEY); new UserSettings().clearFlag(); new UserSettings().setSelectedPatternName(undefined); } @@ -208,27 +214,48 @@ async function refreshJwt(): Promise { } } -// Outcome of the most recent refresh attempt, so callers (e.g. ranked -// matchmaking) can tell "your session expired" apart from "we couldn't reach -// the auth server right now". -export type RefreshOutcome = "ok" | "expired" | "transient"; -let __lastRefreshOutcome: RefreshOutcome = "ok"; +// Outcome of the most recent authentication check — a /auth/refresh attempt or +// a /users/@me call — so callers (e.g. ranked matchmaking) can tell "your +// session expired" apart from "we couldn't reach the auth server right now". It +// spans both layers because either can be the call that just failed. +export type AuthOutcome = "ok" | "expired" | "transient"; +let __lastAuthOutcome: AuthOutcome = "ok"; -export function getLastRefreshOutcome(): RefreshOutcome { - return __lastRefreshOutcome; +export function getLastAuthOutcome(): AuthOutcome { + return __lastAuthOutcome; } -const REFRESH_MAX_ATTEMPTS = 3; - -function refreshBackoffMs(attempt: number): number { - // Backoff between attempts: 500ms, then 1000ms. - return 500 * 2 ** (attempt - 1); +// Lets the /users/@me path (Api.ts) record its own outcome into the same signal, +// so the transient-vs-expired distinction reflects whichever call last ran. +export function markAuthOutcome(outcome: AuthOutcome): void { + __lastAuthOutcome = outcome; } +const REFRESH_MAX_ATTEMPTS = 3; +// Backoff between attempts, indexed by (attempt - 1): 500ms, then 1000ms. +const REFRESH_BACKOFF_MS = [500, 1000]; +const REFRESH_TIMEOUT_MS = 10_000; + function delay(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +// fetch() with a per-attempt timeout, so a stalled connection can't hang the +// shared refresh promise (and every caller awaiting it) indefinitely. +async function fetchWithTimeout( + url: string, + options: RequestInit, + timeoutMs: number, +): Promise { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + return await fetch(url, { ...options, signal: controller.signal }); + } finally { + clearTimeout(timer); + } +} + function notifySessionExpired(): void { if (typeof window === "undefined") return; window.dispatchEvent(new CustomEvent("auth-session-expired")); @@ -244,39 +271,44 @@ async function doRefreshJwt(): Promise { console.log( `Refreshing jwt (attempt ${attempt}/${REFRESH_MAX_ATTEMPTS})`, ); - const response = await fetch(getApiBase() + "/auth/refresh", { - method: "POST", - credentials: "include", - }); + const response = await fetchWithTimeout( + getApiBase() + "/auth/refresh", + { method: "POST", credentials: "include" }, + REFRESH_TIMEOUT_MS, + ); if (response.status === 200) { const json = await response.json(); const { jwt, expiresIn } = json; - __expiresAt = Date.now() + expiresIn * 1000; - __jwt = jwt; - __lastRefreshOutcome = "ok"; - console.log("Refresh succeeded"); - return; - } - - // 401/403 are definitive: the refresh token is genuinely invalid or - // expired, so retrying can't help. Do a "soft" logout — clear the - // in-memory JWT but preserve the session cookie + persistent identity — - // and let a previously-signed-in user be prompted to log in again. - if (response.status === 401 || response.status === 403) { + // A 200 with an unusable body would otherwise corrupt our state + // (undefined jwt makes hadSession lie; NaN expiry disables future + // refresh). Treat it as a transient failure and retry instead. + if (typeof jwt === "string" && typeof expiresIn === "number") { + __expiresAt = Date.now() + expiresIn * 1000; + __jwt = jwt; + __lastAuthOutcome = "ok"; + console.log("Refresh succeeded"); + return; + } + console.error("Refresh returned 200 with an invalid body"); + } else if (response.status === 401 || response.status === 403) { + // 401/403 are definitive: the refresh token is genuinely invalid or + // expired, so retrying can't help. Do a "soft" logout — clear the + // in-memory JWT but preserve the session cookie + persistent identity — + // and let a previously-signed-in user be prompted to log in again. console.error("Refresh rejected — session expired", response.status); __jwt = null; - __lastRefreshOutcome = "expired"; + __lastAuthOutcome = "expired"; if (hadSession) notifySessionExpired(); return; + } else { + // Everything else (5xx, 429, ...) is transient — fall through to retry. + console.error( + `Refresh failed (status ${response.status}), attempt ${attempt}/${REFRESH_MAX_ATTEMPTS}`, + ); } - - // Everything else (5xx, 429, ...) is transient — fall through to retry. - console.error( - `Refresh failed (status ${response.status}), attempt ${attempt}/${REFRESH_MAX_ATTEMPTS}`, - ); } catch (e) { - // Network error / server unreachable — transient, fall through to retry. + // Network error / timeout / server unreachable — transient, retry. console.error( `Refresh failed (network), attempt ${attempt}/${REFRESH_MAX_ATTEMPTS}`, e, @@ -284,7 +316,7 @@ async function doRefreshJwt(): Promise { } if (attempt < REFRESH_MAX_ATTEMPTS) { - await delay(refreshBackoffMs(attempt)); + await delay(REFRESH_BACKOFF_MS[attempt - 1] ?? 1000); } } @@ -292,7 +324,7 @@ async function doRefreshJwt(): Promise { // session cookie and persistent identity so the next refresh can recover. console.error("Refresh failed after retries; staying recoverable"); __jwt = null; - __lastRefreshOutcome = "transient"; + __lastAuthOutcome = "transient"; } export async function sendMagicLink(email: string): Promise { diff --git a/src/client/Main.ts b/src/client/Main.ts index 4975389c87..aa16a1324e 100644 --- a/src/client/Main.ts +++ b/src/client/Main.ts @@ -506,14 +506,19 @@ class Client { } }; - if ((await userAuth()) === false) { - // Not logged in - onUserMe(false); - } else { - // JWT appears to be valid - // TODO: Add caching - getUserMe().then(onUserMe); - } + // Resolve auth in the background: a slow/degraded auth server (now retried a + // few times) must not delay menu wiring or deep-link lobby joins below. The + // join path re-checks auth itself, and refreshes are single-flighted. + void userAuth().then((auth) => { + if (auth === false) { + // Not logged in + onUserMe(false); + } else { + // JWT appears to be valid + // TODO: Add caching + void getUserMe().then(onUserMe); + } + }); const settingsModal = document.querySelector( "user-setting", diff --git a/src/client/Matchmaking.ts b/src/client/Matchmaking.ts index a6c2250b76..483946e513 100644 --- a/src/client/Matchmaking.ts +++ b/src/client/Matchmaking.ts @@ -3,12 +3,12 @@ import { customElement, state } from "lit/decorators.js"; import { ClientEnv } from "src/client/ClientEnv"; import { UserMeResponse } from "../core/ApiSchemas"; import { getUserMe, hasLinkedAccount } from "./Api"; -import { getLastRefreshOutcome, getPlayToken } from "./Auth"; +import { getLastAuthOutcome, getPlayToken } from "./Auth"; import { BaseModal } from "./components/BaseModal"; import "./components/Difficulties"; import { modalHeader } from "./components/ui/ModalHeader"; import { JoinLobbyEvent } from "./Main"; -import { translateText } from "./Utils"; +import { showToast, translateText } from "./Utils"; @customElement("matchmaking-modal") export class MatchmakingModal extends BaseModal { @@ -126,32 +126,24 @@ export class MatchmakingModal extends BaseModal { userMe.user.google !== undefined || userMe.user.email !== undefined); if (!isLoggedIn) { - // A transient auth-server hiccup (rather than a genuine "not logged in") - // shouldn't bounce the player to the login page — ask them to retry. - if (getLastRefreshOutcome() === "transient") { - window.dispatchEvent( - new CustomEvent("show-message", { - detail: { - message: translateText("matchmaking_button.connection_issue"), - color: "red", - duration: 3000, - }, - }), - ); - this.close(); - return; - } - window.dispatchEvent( - new CustomEvent("show-message", { - detail: { - message: translateText("matchmaking_button.must_login"), - color: "red", - duration: 3000, - }, - }), + // Distinguish a transient auth hiccup (server unreachable / 5xx, or a + // /users/@me blip) from a genuine "not logged in": only the latter should + // bounce the player to the login page; the former just asks them to retry. + // `userMe === false` means we couldn't get an answer (transient possible); + // a returned-but-unlinked userMe is a confirmed guest who must log in. + const transient = + userMe === false && getLastAuthOutcome() === "transient"; + showToast( + translateText( + transient + ? "matchmaking_button.connection_issue" + : "matchmaking_button.must_login", + ), + "red", + 3000, ); this.close(); - window.showPage?.("page-account"); + if (!transient) window.showPage?.("page-account"); return; } diff --git a/src/client/SessionExpiredModal.ts b/src/client/SessionExpiredModal.ts index 4f53b7bdc6..3e7fe7ffc3 100644 --- a/src/client/SessionExpiredModal.ts +++ b/src/client/SessionExpiredModal.ts @@ -47,18 +47,18 @@ export class SessionExpiredModal extends BaseModal {

${translateText("session_expired.body")}

- - + >
`; diff --git a/tests/client/Api.test.ts b/tests/client/Api.test.ts new file mode 100644 index 0000000000..9aec6e0f33 --- /dev/null +++ b/tests/client/Api.test.ts @@ -0,0 +1,156 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +// getUserMe()/getAuthHeader() resolve auth via ./Auth. Mock it so we can drive +// the token + outcome bookkeeping deterministically and observe what Api records. +const { userAuthMock, markAuthOutcomeMock, getAuthHeaderMock } = vi.hoisted( + () => ({ + userAuthMock: vi.fn(), + markAuthOutcomeMock: vi.fn(), + getAuthHeaderMock: vi.fn(), + }), +); + +vi.mock("../../src/client/Auth", () => ({ + userAuth: userAuthMock, + getAuthHeader: getAuthHeaderMock, + markAuthOutcome: markAuthOutcomeMock, + LINKED_ACCOUNT_KEY: "was_linked_account", +})); + +import { + cancelSubscription, + getUserMe, + invalidateUserMe, + wasLinkedAccount, +} from "../../src/client/Api"; + +const LINKED_ACCOUNT_KEY = "was_linked_account"; + +function userMeBody(linked: boolean) { + return { + user: linked ? { email: "player@example.com" } : {}, + player: { + publicId: "public-123", + adfree: false, + achievements: { singleplayerMap: [] }, + friends: [], + subscription: null, + }, + }; +} + +const okJson = (data: unknown) => ({ + status: 200, + ok: true, + json: async () => data, +}); + +describe("Api.getUserMe: transient handling and caching", () => { + beforeEach(() => { + localStorage.clear(); + invalidateUserMe(); + userAuthMock.mockReset(); + markAuthOutcomeMock.mockReset(); + getAuthHeaderMock.mockReset(); + getAuthHeaderMock.mockResolvedValue("Bearer test-token"); + vi.spyOn(console, "error").mockImplementation(() => {}); + vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.spyOn(console, "log").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it("does not cache a transient /users/@me failure, so a retry can recover", async () => { + userAuthMock.mockResolvedValue({ jwt: "jwt-1", claims: {} }); + let meCalls = 0; + const fetchMock = vi.fn(async (url: unknown) => { + if (String(url).includes("/users/@me")) { + meCalls++; + if (meCalls === 1) { + return { status: 503, ok: false, json: async () => ({}) }; + } + return okJson(userMeBody(true)); + } + return okJson({}); + }); + vi.stubGlobal("fetch", fetchMock); + + const first = await getUserMe(); + expect(first).toBe(false); + expect(markAuthOutcomeMock).toHaveBeenCalledWith("transient"); + + // The `false` was NOT memoized: a second call re-hits the server and the + // now-recovered backend succeeds. + const second = await getUserMe(); + expect(second).not.toBe(false); + expect(meCalls).toBe(2); + }); + + it("on success records linked status (persisted) and an 'ok' outcome", async () => { + userAuthMock.mockResolvedValue({ jwt: "jwt-1", claims: {} }); + vi.stubGlobal( + "fetch", + vi.fn(async (url: unknown) => + String(url).includes("/users/@me") + ? okJson(userMeBody(true)) + : okJson({}), + ), + ); + + const me = await getUserMe(); + expect(me).not.toBe(false); + expect(localStorage.getItem(LINKED_ACCOUNT_KEY)).toBe("true"); + expect(wasLinkedAccount()).toBe(true); + expect(markAuthOutcomeMock).toHaveBeenLastCalledWith("ok"); + }); + + it("remembers a guest (no linked account) as not-linked", async () => { + userAuthMock.mockResolvedValue({ jwt: "jwt-1", claims: {} }); + vi.stubGlobal( + "fetch", + vi.fn(async (url: unknown) => + String(url).includes("/users/@me") + ? okJson(userMeBody(false)) + : okJson({}), + ), + ); + + await getUserMe(); + expect(localStorage.getItem(LINKED_ACCOUNT_KEY)).toBe("false"); + expect(wasLinkedAccount()).toBe(false); + }); +}); + +describe("Api: authenticated endpoints don't destroy the session on a 401", () => { + beforeEach(() => { + localStorage.clear(); + invalidateUserMe(); + getAuthHeaderMock.mockReset(); + getAuthHeaderMock.mockResolvedValue("Bearer test-token"); + vi.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it("cancelSubscription returns false on 401 without calling /auth/logout", async () => { + const fetchMock = vi.fn(async (_url: unknown) => ({ + status: 401, + ok: false, + json: async () => ({}), + })); + vi.stubGlobal("fetch", fetchMock); + + const result = await cancelSubscription(); + expect(result).toBe(false); + // A spurious 401 must not revoke the session. + expect( + fetchMock.mock.calls.some(([u]) => String(u).includes("/auth/logout")), + ).toBe(false); + }); +}); diff --git a/tests/client/Auth.test.ts b/tests/client/Auth.test.ts index 74fca278b3..db8217d73d 100644 --- a/tests/client/Auth.test.ts +++ b/tests/client/Auth.test.ts @@ -7,7 +7,7 @@ vi.mock("../../src/client/Api", () => ({ getAudience: () => "localhost", })); -import { getLastRefreshOutcome, logOut, userAuth } from "../../src/client/Auth"; +import { getLastAuthOutcome, logOut, userAuth } from "../../src/client/Auth"; const PERSISTENT_ID_KEY = "player_persistent_id"; @@ -56,7 +56,7 @@ describe("Auth: refresh resilience and session-expiry handling", () => { String(u).includes("/auth/refresh"), ).length; expect(refreshCalls).toBe(3); - expect(getLastRefreshOutcome()).toBe("transient"); + expect(getLastAuthOutcome()).toBe("transient"); // Identity preserved and the session is never revoked on a transient blip. expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-123"); expect( @@ -84,7 +84,7 @@ describe("Auth: refresh resilience and session-expiry handling", () => { String(u).includes("/auth/refresh"), ).length; expect(refreshCalls).toBe(1); - expect(getLastRefreshOutcome()).toBe("expired"); + expect(getLastAuthOutcome()).toBe("expired"); expect(localStorage.getItem(PERSISTENT_ID_KEY)).toBe("keep-me-401"); // No active session existed, so we must not nag with the modal. expect(onExpired).not.toHaveBeenCalled(); @@ -115,7 +115,7 @@ describe("Auth: refresh resilience and session-expiry handling", () => { await userAuth(); // establishes __jwt via the first (200) refresh await userAuth(); // session now active -> second refresh 401s - expect(getLastRefreshOutcome()).toBe("expired"); + expect(getLastAuthOutcome()).toBe("expired"); expect(onExpired).toHaveBeenCalledTimes(1); window.removeEventListener("auth-session-expired", onExpired); });