From 0b01956535562debc2247ef66420f139c359d397 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Fri, 28 Aug 2026 14:15:15 -0700 Subject: [PATCH] Ask whose computer this is, and believe the answer Sign-out never cleared local storage. It stopped push, flushed settings and removed the subscription -- that last one reasoned explicitly that a browser left holding someone's mail becomes somebody else's next -- and then left the settings cache and the recently-addressed list on disk. That list is other people's addresses, and nothing ever removed it. Clearing it on sign-out is now unconditional, because lending a laptop is the same exposure as a public machine, only quieter. The keep-list is short and deliberate: lastUser, which only a trusted device writes; the trust flag; and the random push device id. Everything else goes, so a key added later is forgotten by default rather than by nobody having thought about it. "Keep me signed in on this device" defaulted to true, which assumed the answer most costly to get wrong -- someone on a library machine got a thirty-day cookie unless they noticed a ticked box. It now asks whose computer this is, defaults to not yours, and says what each answer does. Untrusted means a session cookie, nothing written locally, no push subscription, and a five minute idle sign-out. The idle timer is there because the alternative does not work: custom beforeunload text was removed from browsers years ago, and no event fires at all for walking away from a signed-in screen, which is the case that matters. A timer needs nobody's cooperation. Reads are gated as well as writes, since a machine trusted once still has the residue; an untrusted sign-in purges it outright. The wire keeps calling this `remember` -- it is persisted in SESSION_FILE, and renaming it would invalidate every session file on upgrade for a change of vocabulary. Verified in a browser against the mock, not only in tests: untrusted sign-in leaves localStorage empty through a full session including folder expansion; trusted writes settings, recent and lastUser as before; sign-out clears recent and settings while keeping lastUser; an untrusted sign-in afterwards clears even that. --- web/src/lib/__tests__/idleLogout.test.ts | 54 ++++++++++++ web/src/lib/__tests__/storage.test.ts | 105 +++++++++++++++++++++++ web/src/lib/__tests__/theme.test.ts | 7 +- web/src/lib/idleLogout.ts | 59 +++++++++++++ web/src/lib/storage.ts | 89 +++++++++++++++++++ web/src/lib/webpush.ts | 5 ++ web/src/lib/webpushEnable.ts | 7 ++ web/src/store/contacts.ts | 5 +- web/src/store/session.ts | 22 +++++ web/src/views/Login.tsx | 17 ++-- 10 files changed, 361 insertions(+), 9 deletions(-) create mode 100644 web/src/lib/__tests__/idleLogout.test.ts create mode 100644 web/src/lib/__tests__/storage.test.ts create mode 100644 web/src/lib/idleLogout.ts diff --git a/web/src/lib/__tests__/idleLogout.test.ts b/web/src/lib/__tests__/idleLogout.test.ts new file mode 100644 index 0000000..3f594b7 --- /dev/null +++ b/web/src/lib/__tests__/idleLogout.test.ts @@ -0,0 +1,54 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { startIdleLogout, stopIdleLogout, IDLE_TIMEOUT_MS } from "@/lib/idleLogout"; + +describe("idle sign-out on an untrusted device", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => { + stopIdleLogout(); + vi.useRealTimers(); + }); + + it("signs out after five minutes of nothing happening", () => { + const expire = vi.fn(); + startIdleLogout(expire); + expect(IDLE_TIMEOUT_MS).toBe(5 * 60 * 1000); + + vi.advanceTimersByTime(IDLE_TIMEOUT_MS - 1); + expect(expire).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(expire).toHaveBeenCalledTimes(1); + }); + + it("starts the clock again on any sign of a person", () => { + const expire = vi.fn(); + startIdleLogout(expire); + + vi.advanceTimersByTime(IDLE_TIMEOUT_MS - 1000); + window.dispatchEvent(new Event("keydown")); + vi.advanceTimersByTime(IDLE_TIMEOUT_MS - 1000); + expect(expire).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1000); + expect(expire).toHaveBeenCalledTimes(1); + }); + + it("fires once, not repeatedly, and stops listening afterwards", () => { + const expire = vi.fn(); + startIdleLogout(expire); + vi.advanceTimersByTime(IDLE_TIMEOUT_MS * 3); + expect(expire).toHaveBeenCalledTimes(1); + + // A late event must not resurrect a timer for a session that has ended. + window.dispatchEvent(new Event("keydown")); + vi.advanceTimersByTime(IDLE_TIMEOUT_MS * 2); + expect(expire).toHaveBeenCalledTimes(1); + }); + + it("stops cleanly, so a trusted sign-in is never signed out", () => { + const expire = vi.fn(); + startIdleLogout(expire); + stopIdleLogout(); + vi.advanceTimersByTime(IDLE_TIMEOUT_MS * 2); + expect(expire).not.toHaveBeenCalled(); + }); +}); diff --git a/web/src/lib/__tests__/storage.test.ts b/web/src/lib/__tests__/storage.test.ts new file mode 100644 index 0000000..b598e79 --- /dev/null +++ b/web/src/lib/__tests__/storage.test.ts @@ -0,0 +1,105 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { + accountKey, + clearAllData, + clearSignedInData, + isDeviceTrusted, + loadJson, + loadRaw, + saveJson, + setDeviceTrusted, +} from "@/lib/storage"; + +/** + * The gate is a privacy boundary rather than a convenience, so it is tested + * from both sides: that a trusted device still works exactly as it did, and + * that an untrusted one leaves nothing to find. + */ +describe("device-trusted storage", () => { + let store: Map; + + beforeEach(() => { + store = new Map(); + Object.defineProperty(globalThis, "localStorage", { + configurable: true, + value: { + get length() { + return store.size; + }, + key: (i: number) => [...store.keys()][i] ?? null, + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, v), + removeItem: (k: string) => void store.delete(k), + }, + }); + setDeviceTrusted(false); + }); + + afterEach(() => { + setDeviceTrusted(false); + Reflect.deleteProperty(globalThis, "localStorage"); + }); + + it("writes nothing at all when the device is not trusted", () => { + saveJson("settings", { theme: "dark" }); + saveJson(accountKey("acct1", "recent"), [{ email: "someone@example.com" }]); + expect([...store.keys()].filter((k) => k !== "ihasmail:deviceTrusted")).toEqual([]); + }); + + it("does not read residue left by an earlier trusted session", () => { + setDeviceTrusted(true); + saveJson(accountKey("acct1", "recent"), [{ email: "someone@example.com" }]); + setDeviceTrusted(false); + // The bytes are still on disk until a purge; the gate must not serve them. + expect(loadRaw(accountKey("acct1", "recent"), [])).toEqual([]); + }); + + it("round-trips normally on a trusted device", () => { + setDeviceTrusted(true); + saveJson("settings", { theme: "dark" }); + expect(loadJson("settings", { theme: "light", accent: "blue" })).toEqual({ theme: "dark", accent: "blue" }); + expect(isDeviceTrusted()).toBe(true); + }); + + it("remembers trust across a reload, so a trusted device still paints from cache", () => { + setDeviceTrusted(true); + expect(store.get("ihasmail:deviceTrusted")).toBe("1"); + setDeviceTrusted(false); + expect(store.has("ihasmail:deviceTrusted")).toBe(false); + }); + + it("clears the account's data on sign-out but keeps the deliberate exceptions", () => { + setDeviceTrusted(true); + saveJson("settings", { theme: "dark" }); + saveJson("mbx-expanded", { a: true }); + saveJson(accountKey("acct1", "recent"), [{ email: "someone@example.com" }]); + store.set("ihasmail:lastUser", "me@example.com"); + store.set("ihasmail:pushDeviceId", "ihasmail-abc"); + + clearSignedInData(); + + expect(store.has("ihasmail:settings")).toBe(false); + expect(store.has("ihasmail:mbx-expanded")).toBe(false); + expect(store.has("ihasmail:acct1:recent")).toBe(false); + // Kept on purpose: prefills sign-in, and only a trusted device wrote it. + expect(store.get("ihasmail:lastUser")).toBe("me@example.com"); + expect(store.get("ihasmail:pushDeviceId")).toBe("ihasmail-abc"); + }); + + it("clears everything, lastUser included, for an untrusted sign-in", () => { + setDeviceTrusted(true); + saveJson("settings", { theme: "dark" }); + store.set("ihasmail:lastUser", "me@example.com"); + + clearAllData(); + + expect([...store.keys()]).toEqual([]); + }); + + it("leaves keys belonging to anything else alone", () => { + setDeviceTrusted(true); + store.set("someone-elses-key", "keep me"); + clearAllData(); + expect(store.get("someone-elses-key")).toBe("keep me"); + }); +}); diff --git a/web/src/lib/__tests__/theme.test.ts b/web/src/lib/__tests__/theme.test.ts index f73ce3e..930d42d 100644 --- a/web/src/lib/__tests__/theme.test.ts +++ b/web/src/lib/__tests__/theme.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_SETTINGS, DEVICE_KEYS, acceptRemote, isDarkTheme, syncedPart, toggleTarget, useSettings, type Theme } from "@/store/settings"; -import { loadJson, saveJson } from "@/lib/storage"; +import { loadJson, saveJson, setDeviceTrusted } from "@/lib/storage"; /** * "ihasmail" is a dark theme wearing ihasmail.org's palette. Everything that @@ -60,9 +60,14 @@ describe("the default theme", () => { removeItem: (k: string) => void store.delete(k), }, }); + // Reads and writes are gated on device trust now, and the gate defaults to + // closed. These tests are about `loadJson`'s merge, so open it and put it + // back -- an untrusted device is covered by storage.test.ts instead. + setDeviceTrusted(true); try { fn(); } finally { + setDeviceTrusted(false); Reflect.deleteProperty(globalThis, "localStorage"); } }; diff --git a/web/src/lib/idleLogout.ts b/web/src/lib/idleLogout.ts new file mode 100644 index 0000000..cabb49c --- /dev/null +++ b/web/src/lib/idleLogout.ts @@ -0,0 +1,59 @@ +/** + * Sign out an untrusted device after a few minutes of inactivity. + * + * This exists because the alternative does not work. Asking someone to + * remember to sign out relies on the person, which is the part you cannot rely + * on when the machine is not theirs β€” and a browser cannot help: custom + * `beforeunload` text was removed years ago, and no event fires at all for the + * case that actually matters, which is walking away from a signed-in screen. + * + * A timer needs nobody's cooperation, so that is what this is. + * + * Trusted devices are left alone entirely: the whole point of saying a machine + * is yours is not being signed out of it. + */ +const IDLE_MS = 5 * 60 * 1000; + +/** Coarse enough not to fire constantly, broad enough to catch a person reading. */ +const ACTIVITY = ["mousedown", "keydown", "touchstart", "scroll", "focus"] as const; + +let timer: ReturnType | null = null; +let onExpire: (() => void) | null = null; + +function arm(): void { + if (timer) clearTimeout(timer); + timer = setTimeout(() => { + timer = null; + const fn = onExpire; + stopIdleLogout(); + fn?.(); + }, IDLE_MS); +} + +/** + * Reading a long message is not idleness, but it produces no events either. + * Visibility is the honest signal available: a hidden tab is one nobody is + * looking at, so the clock keeps running; showing it again is activity. + */ +function onVisibility(): void { + if (document.visibilityState === "visible") arm(); +} + +export function startIdleLogout(expire: () => void): void { + stopIdleLogout(); + onExpire = expire; + for (const ev of ACTIVITY) window.addEventListener(ev, arm, { passive: true, capture: true }); + document.addEventListener("visibilitychange", onVisibility); + arm(); +} + +export function stopIdleLogout(): void { + if (timer) clearTimeout(timer); + timer = null; + onExpire = null; + for (const ev of ACTIVITY) window.removeEventListener(ev, arm, { capture: true }); + document.removeEventListener("visibilitychange", onVisibility); +} + +/** Exported for tests, which should not wait five real minutes. */ +export const IDLE_TIMEOUT_MS = IDLE_MS; diff --git a/web/src/lib/storage.ts b/web/src/lib/storage.ts index 8c5e64e..852a400 100644 --- a/web/src/lib/storage.ts +++ b/web/src/lib/storage.ts @@ -1,6 +1,93 @@ +/** + * Local storage, gated on whether this device is trusted. + * + * Everything here is a *cache* or a screen preference β€” the real copy lives in + * the account's JMAP Files (see `settingsSync`). That makes it safe to write + * nothing at all, which is what an untrusted device does: on a shared or public + * machine the cost of a stale first frame is nothing beside leaving someone's + * address book on it. + * + * Reads are gated as well as writes. A machine that was trusted once still has + * the residue, and honouring it would let a previous session's data surface in + * a later untrusted one. + */ const PREFIX = "ihasmail:"; +/** + * Kept when a session ends. Everything else is cleared, so a key added later + * is forgotten by default rather than by nobody having thought about it. + * + * - `lastUser` is a deliberate convenience: it prefills the sign-in field, and + * it is only ever written by a trusted device in the first place. + * - `deviceTrusted` is how the next boot knows to read at all. + * - `pushDeviceId` is a random id for this browser, so re-subscribing replaces + * rather than accumulates. The subscription itself is removed on sign-out. + */ +const KEEP_ON_SIGN_OUT = ["lastUser", "deviceTrusted", "pushDeviceId"]; + +const TRUST_KEY = `${PREFIX}deviceTrusted`; + +/** + * Read at module load rather than waiting for the session, so a trusted device + * still paints its first frame from cache. An untrusted one has nothing to + * read, so there is nothing to wait for. + */ +let trusted = (() => { + try { + return localStorage.getItem(TRUST_KEY) === "1"; + } catch { + return false; + } +})(); + +export function isDeviceTrusted(): boolean { + return trusted; +} + +/** Set from the session's `remember` flag, which is the answer given at sign-in. */ +export function setDeviceTrusted(value: boolean): void { + trusted = value; + try { + if (value) localStorage.setItem(TRUST_KEY, "1"); + else localStorage.removeItem(TRUST_KEY); + } catch { + /* private mode: the in-memory flag still holds for this tab */ + } +} + +/** Every `ihasmail:` key currently present, without the prefix. */ +function ownKeys(): string[] { + const out: string[] = []; + try { + for (let i = 0; i < localStorage.length; i++) { + const k = localStorage.key(i); + if (k && k.startsWith(PREFIX)) out.push(k.slice(PREFIX.length)); + } + } catch { + /* ignore */ + } + return out; +} + +/** + * Drop what this browser was holding for a signed-in account. Called on every + * sign-out, trusted or not: handing a laptop to someone else is the same + * exposure as a public machine, only quieter. + */ +export function clearSignedInData(): void { + for (const key of ownKeys()) { + if (KEEP_ON_SIGN_OUT.includes(key)) continue; + removeKey(key); + } +} + +/** Everything, `lastUser` included β€” for signing in to a device we do not trust. */ +export function clearAllData(): void { + for (const key of ownKeys()) removeKey(key); +} + export function loadJson(key: string, fallback: T): T { + if (!trusted) return fallback; try { const raw = localStorage.getItem(PREFIX + key); if (raw == null) return fallback; @@ -11,6 +98,7 @@ export function loadJson(key: string, fallback: T): T { } export function loadRaw(key: string, fallback: T): T { + if (!trusted) return fallback; try { const raw = localStorage.getItem(PREFIX + key); if (raw == null) return fallback; @@ -21,6 +109,7 @@ export function loadRaw(key: string, fallback: T): T { } export function saveJson(key: string, value: unknown): void { + if (!trusted) return; try { localStorage.setItem(PREFIX + key, JSON.stringify(value)); } catch { diff --git a/web/src/lib/webpush.ts b/web/src/lib/webpush.ts index 0432d03..2c8cd3f 100644 --- a/web/src/lib/webpush.ts +++ b/web/src/lib/webpush.ts @@ -19,6 +19,7 @@ */ import { CAP, client } from "@/jmap/client"; import type { GetResponse, Id, SetResponse } from "@/jmap/types"; +import { isDeviceTrusted } from "@/lib/storage"; export const VAPID_CAP = "urn:ietf:params:jmap:webpush-vapid"; export const EMAILPUSH_CAP = "urn:ietf:params:jmap:emailpush"; @@ -93,6 +94,10 @@ export function encodeKey(buffer: ArrayBuffer | null): string { /** A stable id for this browser, so a re-subscribe replaces rather than piles up. */ export function deviceClientId(): string { const KEY = "ihasmail:pushDeviceId"; + // An untrusted device gets a per-session id instead of a stored one. It is + // the same trade private mode already makes below: re-subscribing will not + // reuse it, which costs nothing when push is refused there anyway. + if (!isDeviceTrusted()) return `ihasmail-${crypto.randomUUID()}`; try { const existing = localStorage.getItem(KEY); if (existing) return existing; diff --git a/web/src/lib/webpushEnable.ts b/web/src/lib/webpushEnable.ts index 8017977..c84f222 100644 --- a/web/src/lib/webpushEnable.ts +++ b/web/src/lib/webpushEnable.ts @@ -6,6 +6,7 @@ * permission prompt, none of which exists under a test runner. */ import { CAP } from "@/jmap/client"; +import { isDeviceTrusted } from "@/lib/storage"; import { useSession } from "@/store/session"; import { useMail } from "@/store/mail"; import { @@ -67,6 +68,12 @@ export async function enableWebPush(): Promise<{ ok: true } | { ok: false; reaso if (Notification.permission === "denied") { return { ok: false, reason: "Notifications are blocked for this site in your browser's settings." }; } + // A subscription outlives the tab and belongs to the account, not the + // session -- so on a machine the user has told us is not theirs, it would go + // on delivering their mail to it long after they had gone. + if (!isDeviceTrusted()) { + return { ok: false, reason: "Background notifications need a device you have marked as your own. Sign in again with \u201CThis is my own device\u201D ticked." }; + } const key = applicationServerKey(); if (!key) return { ok: false, reason: "This mail server does not publish a push key." }; diff --git a/web/src/store/contacts.ts b/web/src/store/contacts.ts index 2e07567..f461821 100644 --- a/web/src/store/contacts.ts +++ b/web/src/store/contacts.ts @@ -1,4 +1,5 @@ import { create } from "zustand"; +import { accountKey, loadRaw, saveJson } from "@/lib/storage"; import { CAP, client, setErrorMessage } from "@/jmap/client"; import type { AddressBook, ContactCard, EmailAddress, GetResponse, Id, Principal, QueryResponse, SetResponse } from "@/jmap/types"; import { contactDisplayName, contactEmails, sortKey } from "@/lib/contacts"; @@ -441,7 +442,7 @@ export const useContacts = create((set, get) => ({ const next = [...addrs.filter((a) => a.email), ...cur.filter((r) => !addrs.some((a) => a.email.toLowerCase() === r.email.toLowerCase()))].slice(0, 200); set({ recent: next }); try { - localStorage.setItem(`ihasmail:${get().accountId}:recent`, JSON.stringify(next)); + saveJson(accountKey(get().accountId, "recent"), next); } catch { /* ignore */ } @@ -466,7 +467,7 @@ useSession.subscribe((s) => { const accountId = s.accountFor(CAP.contacts); let recent: EmailAddress[] = []; try { - recent = JSON.parse(localStorage.getItem(`ihasmail:${accountId}:recent`) ?? "[]") as EmailAddress[]; + recent = loadRaw(accountKey(accountId, "recent"), []); } catch { /* ignore */ } diff --git a/web/src/store/session.ts b/web/src/store/session.ts index db82aa3..df13cea 100644 --- a/web/src/store/session.ts +++ b/web/src/store/session.ts @@ -7,6 +7,8 @@ import { setServerLocale } from "@/lib/datetime"; import { flushSettingsPush, stopSettingsSync } from "@/lib/settingsSync"; import { reloadIfServerRebuilt } from "@/lib/staleBuild"; import { unsubscribeThisDevice } from "@/lib/webpush"; +import { clearAllData, clearSignedInData, setDeviceTrusted } from "@/lib/storage"; +import { startIdleLogout, stopIdleLogout } from "@/lib/idleLogout"; export type AuthStatus = "loading" | "anonymous" | "authenticated"; @@ -81,6 +83,11 @@ export const useSession = create((set, get) => ({ } catch { /* ignore */ } + stopIdleLogout(); + // Unconditional. The push subscription above is removed for exactly this + // reason -- that a browser left holding someone's mail is somebody else's + // problem next -- and the address book cached here is the same argument. + clearSignedInData(); client.session = null; set({ status: "anonymous", session: null, accountId: null }); }, @@ -112,6 +119,19 @@ export const useSession = create((set, get) => ({ function applySession(s: JmapSession, set: (p: Partial) => void) { client.session = s; setServerLocale(s.ihasmail?.userLocale); + // `remember` is the answer to "is this device yours", given at sign-in and + // carried on the session -- so a reload arrives at the same answer without + // the client storing it, which on an untrusted device it could not do anyway. + const trusted = Boolean(s.ihasmail?.remember); + setDeviceTrusted(trusted); + if (trusted) { + stopIdleLogout(); + } else { + // Residue from an earlier trusted session on this machine is exactly what + // an untrusted sign-in is asking us not to keep. + clearAllData(); + startIdleLogout(() => void useSession.getState().logout()); + } const accountId = s.primaryAccounts[CAP.mail] ?? Object.keys(s.accounts)[0] ?? null; set({ status: "authenticated", session: s, accountId, error: null }); } @@ -119,6 +139,8 @@ function applySession(s: JmapSession, set: (p: Partial) => void) { client.onUnauthenticated(() => { push.stop(); stopSettingsSync(); + stopIdleLogout(); + clearSignedInData(); client.session = null; // Ask before showing the sign-in form rather than after. A deploy is the // usual reason to be signed out here, and reloading a form someone has diff --git a/web/src/views/Login.tsx b/web/src/views/Login.tsx index 402fd98..52c3aae 100644 --- a/web/src/views/Login.tsx +++ b/web/src/views/Login.tsx @@ -22,7 +22,7 @@ export function LoginPage() { const [username, setUsername] = useState(() => localStorage.getItem("ihasmail:lastUser") ?? ""); const [password, setPassword] = useState(""); const [showPw, setShowPw] = useState(false); - const [remember, setRemember] = useState(true); + const [trustDevice, setTrustDevice] = useState(false); const [busy, setBusy] = useState(false); const [error, setError] = useState(null); @@ -34,8 +34,8 @@ export function LoginPage() { try { // No two-factor code: the field is not on this form until the flow works // end to end, and the server treats an absent code as none given. - await login(username.trim(), password, "", remember); - localStorage.setItem("ihasmail:lastUser", username.trim()); + await login(username.trim(), password, "", trustDevice); + if (trustDevice) localStorage.setItem("ihasmail:lastUser", username.trim()); } catch (err) { if (err instanceof ApiError) { if (err.code === "invalid_credentials") { @@ -74,10 +74,15 @@ export function LoginPage() { -