diff --git a/web/src/lib/__tests__/accountRouting.test.ts b/web/src/lib/__tests__/accountRouting.test.ts new file mode 100644 index 0000000..56dec1e --- /dev/null +++ b/web/src/lib/__tests__/accountRouting.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from "vitest"; +import { accountForCapability, ownAccountForCapability, type SessionLike } from "@/lib/accountRouting"; + +/** + * Found by sharing a folder between two real accounts. + * + * Switching to the account somebody shared pointed everything at it, because + * the rule was "use the selected account if it can do this" and a shared file + * account can, by definition, do files. ihasmail keeps its own settings in the + * account's Files, so changing any setting while looking at somebody's shared + * folder wrote `settings.json` into *their* storage, creating the `ihasmail` + * folder there to do it. Reading someone else's data by mistake is bad; writing + * yours into it is worse, and it was the same one-line rule doing both. + */ + +const CAL = "urn:ietf:params:jmap:calendars"; +const FILES = "urn:ietf:params:jmap:filenode"; +const MAIL = "urn:ietf:params:jmap:mail"; + +/** Mine does everything; theirs is a shared account with only files on it. */ +const shared = (): SessionLike => ({ + accounts: { + mine: { isPersonal: true, accountCapabilities: { [MAIL]: {}, [FILES]: {}, [CAL]: {} } }, + theirs: { isPersonal: false, accountCapabilities: { [FILES]: {} } }, + }, + primaryAccounts: { [MAIL]: "mine", [FILES]: "mine", [CAL]: "mine" }, +}); + +describe("what the reader is looking at", () => { + it("follows the switch into a shared account for what was shared", () => { + expect(accountForCapability(shared(), "theirs", FILES)).toBe("theirs"); + }); + + it("leaves everything else on the reader's own account", () => { + expect(accountForCapability(shared(), "theirs", MAIL)).toBe("mine"); + expect(accountForCapability(shared(), "theirs", CAL)).toBe("mine"); + }); + + it("still follows a switch between the reader's own accounts", () => { + const s = shared(); + s.accounts.second = { isPersonal: true, accountCapabilities: { [MAIL]: {} } }; + expect(accountForCapability(s, "second", MAIL)).toBe("second"); + }); + + it("gives up rather than aim at a shared account for something unshared", () => { + // No primary for calendars, and theirs does not offer them. The old rule + // fell back to the selection, which is somebody else's account. + const s = shared(); + delete s.primaryAccounts[CAL]; + expect(accountForCapability(s, "theirs", CAL)).toBeNull(); + }); + + it("lets one of the reader's own accounts stand in when there is no primary", () => { + const s = shared(); + delete s.primaryAccounts[CAL]; + expect(accountForCapability(s, "mine", CAL)).toBe("mine"); + }); +}); + +describe("what belongs to the reader", () => { + it("stays on their own account while they look at a shared one", () => { + // The one that matters: settings are written through this. + expect(ownAccountForCapability(shared(), FILES)).toBe("mine"); + }); + + it("ignores a primary account the server says is not the reader's", () => { + const s = shared(); + s.primaryAccounts[FILES] = "theirs"; + expect(ownAccountForCapability(s, FILES)).toBe("mine"); + }); + + it("finds a personal account when no primary is named", () => { + const s = shared(); + delete s.primaryAccounts[FILES]; + expect(ownAccountForCapability(s, FILES)).toBe("mine"); + }); + + it("answers nothing rather than a shared account", () => { + const s: SessionLike = { + accounts: { theirs: { isPersonal: false, accountCapabilities: { [FILES]: {} } } }, + primaryAccounts: {}, + }; + expect(ownAccountForCapability(s, FILES)).toBeNull(); + }); +}); diff --git a/web/src/lib/accountRouting.ts b/web/src/lib/accountRouting.ts new file mode 100644 index 0000000..c020c50 --- /dev/null +++ b/web/src/lib/accountRouting.ts @@ -0,0 +1,78 @@ +/** + * Which account a request goes to. + * + * A JMAP session lists more than one account whenever anything is shared with + * you: the sharer's account appears alongside your own, carrying whichever + * capabilities they shared. Switching to one is how you read their files, so + * some requests have to follow that selection. + * + * Others must never follow it, and telling the two apart is the whole point of + * this file. ihasmail keeps its own settings in the account's Files — that is + * what makes them travel between devices — and a shared file account advertises + * the file capability by definition. So the obvious rule, "use whichever + * account is selected if it can do this", writes your settings into the other + * person's storage the moment you change one while looking at their folder. It + * would create the `ihasmail` folder there to do it. + * + * Two questions, then, and they have different answers: + * + * - what am I *looking at* -> `accountForCapability`, follows the selection + * - what is *mine* -> `ownAccountForCapability`, never does + * + * There is a third rule hiding in the first. A capability the selected account + * does not advertise used to fall back to that account anyway, so a session + * with no primary account for something would aim it at whoever was selected — + * someone else. Falling back to nothing is the honest answer: the feature is + * unavailable, which is true, rather than pointed at a stranger's data. + */ +import type { Id } from "@/jmap/types"; + +export interface AccountLike { + /** JMAP: true when the account belongs to the authenticated user. */ + isPersonal: boolean; + accountCapabilities?: Record; +} + +export interface SessionLike { + accounts: Record; + primaryAccounts: Record; +} + +const advertises = (account: AccountLike | undefined, cap: string): boolean => + Boolean(account && cap in (account.accountCapabilities ?? {})); + +/** + * The account to read and write for this capability, honouring the switcher. + * + * Use for anything the reader is looking at: their mail, a shared calendar, + * somebody's files. Not for anything of the reader's own — see below. + */ +export function accountForCapability(session: SessionLike | null, selectedId: Id | null, cap: string): Id | null { + if (!session) return null; + const selected = selectedId ? session.accounts[selectedId] : undefined; + if (selected && advertises(selected, cap)) return selectedId; + const primary = session.primaryAccounts[cap]; + if (primary) return primary; + // No primary, and the selection cannot serve this. Falling back to the + // selection would aim the request at a shared account for something nobody + // shared; only one of the reader's own accounts may stand in. + if (selected && selected.isPersonal) return selectedId; + return null; +} + +/** + * The reader's own account for this capability, whatever they are looking at. + * + * Use for the reader's own state -- synced settings, signature images, push + * registration. These belong to them and follow them, and must not land in an + * account somebody shared just because it happens to be on screen. + */ +export function ownAccountForCapability(session: SessionLike | null, cap: string): Id | null { + if (!session) return null; + const primary = session.primaryAccounts[cap]; + // A primary account is the reader's own by definition, but check rather than + // assume: a server that named a shared one here would otherwise be trusted. + if (primary && session.accounts[primary]?.isPersonal !== false) return primary; + const own = Object.entries(session.accounts).find(([, a]) => a.isPersonal && advertises(a, cap)); + return own?.[0] ?? null; +} diff --git a/web/src/lib/settingsSync.ts b/web/src/lib/settingsSync.ts index 62023e4..fe5066b 100644 --- a/web/src/lib/settingsSync.ts +++ b/web/src/lib/settingsSync.ts @@ -36,7 +36,7 @@ let armed = false; let listenersBound = false; export function settingsSyncAvailable(): boolean { - return client.hasCapability(CAP.filenode) && Boolean(useSession.getState().accountFor(CAP.filenode)); + return client.hasCapability(CAP.filenode) && Boolean(useSession.getState().ownAccountFor(CAP.filenode)); } /** @@ -46,7 +46,7 @@ export function settingsSyncAvailable(): boolean { */ export async function loadRemoteSettings(): Promise | null> { if (!settingsSyncAvailable()) return null; - const accountId = useSession.getState().accountFor(CAP.filenode)!; + const accountId = useSession.getState().ownAccountFor(CAP.filenode)!; try { const folderId = await ensureFolder(accountId); const node = await findInFolder(accountId, folderId, FILE); @@ -109,7 +109,7 @@ export async function flushSettingsPush(): Promise { async function writeSettings(body: Record): Promise { if (!settingsSyncAvailable()) return; - const accountId = useSession.getState().accountFor(CAP.filenode)!; + const accountId = useSession.getState().ownAccountFor(CAP.filenode)!; const json = JSON.stringify(body, null, 2); // Byte length, not character count: a template or a signature with any // non-ASCII in it would otherwise be reported shorter than it is. diff --git a/web/src/lib/signatureImages.ts b/web/src/lib/signatureImages.ts index 7b7a574..8a656c1 100644 --- a/web/src/lib/signatureImages.ts +++ b/web/src/lib/signatureImages.ts @@ -13,7 +13,7 @@ import { toast } from "@/ui/toast"; /** Upload an image for use in a signature; returns a same-origin blob URL. */ export async function uploadSignatureImage(file: File): Promise { - const accountId = useSession.getState().accountFor(CAP.filenode); + const accountId = useSession.getState().ownAccountFor(CAP.filenode); if (!accountId || !client.hasCapability(CAP.filenode)) { toast.error("Images in signatures need the Files feature, which this account doesn't have."); throw new Error("filenode unavailable"); @@ -42,7 +42,7 @@ export async function uploadSignatureImage(file: File): Promise { /** Store the full HTML of an over-sized signature in Files; returns the blob id. */ export async function storeSignatureHtml(html: string): Promise { - const accountId = useSession.getState().accountFor(CAP.filenode); + const accountId = useSession.getState().ownAccountFor(CAP.filenode); if (!accountId || !client.hasCapability(CAP.filenode)) throw new Error("This signature is too long for the server and the Files feature (needed to store long signatures) is not available."); const up = await client.upload(accountId, new Blob([html], { type: "text/html" }), { type: "text/html" }); const folderId = await ensureFolder(accountId); @@ -77,7 +77,9 @@ export async function externalizeDataImages(html: string): Promise { /** Load the full HTML of a marker signature. */ export async function loadStoredSignature(blobId: string, type = "text/html"): Promise { - const accountId = useSession.getState().accountFor(CAP.filenode) ?? useSession.getState().accountId; + // No `?? accountId` fallback: a signature is the reader's own, and the + // selected account may be somebody else's shared one. + const accountId = useSession.getState().ownAccountFor(CAP.filenode); if (!accountId) throw new Error("no account"); return client.fetchBlobText(accountId, blobId, type); } diff --git a/web/src/lib/webpushEnable.ts b/web/src/lib/webpushEnable.ts index d2f8155..8017977 100644 --- a/web/src/lib/webpushEnable.ts +++ b/web/src/lib/webpushEnable.ts @@ -78,7 +78,7 @@ export async function enableWebPush(): Promise<{ ok: true } | { ok: false; reaso userVisibleOnly: true, applicationServerKey: decodeApplicationServerKey(key), })); - const accountId = useSession.getState().accountFor(CAP.mail); + const accountId = useSession.getState().ownAccountFor(CAP.mail); const inboxId = useMail.getState().roleId("inbox"); await createSubscription(subscriptionPayload(sub, accountId, inboxId)); listenForVerification(); diff --git a/web/src/store/session.ts b/web/src/store/session.ts index 1eb9f5d..2e7a562 100644 --- a/web/src/store/session.ts +++ b/web/src/store/session.ts @@ -2,6 +2,7 @@ import { create } from "zustand"; import { apiFetch, ApiError, CAP, client } from "@/jmap/client"; import type { Id, JmapSession } from "@/jmap/types"; import { push, type PushState } from "@/jmap/push"; +import { accountForCapability, ownAccountForCapability } from "@/lib/accountRouting"; import { setServerLocale } from "@/lib/datetime"; import { flushSettingsPush, stopSettingsSync } from "@/lib/settingsSync"; import { unsubscribeThisDevice } from "@/lib/webpush"; @@ -22,8 +23,10 @@ interface SessionState { logout(): Promise; refresh(): Promise; setAccount(id: Id): void; - /** Returns the accountId for a capability (primary), falling back to the selected mail account. */ + /** The account to read and write for a capability, honouring the account switcher. */ accountFor(cap: string): Id | null; + /** The user's own account for a capability, whatever they are looking at. */ + ownAccountFor(cap: string): Id | null; } export const useSession = create((set, get) => ({ @@ -97,11 +100,11 @@ export const useSession = create((set, get) => ({ }, accountFor(cap) { - const s = get().session; - if (!s) return null; - const selected = get().accountId; - if (selected && s.accounts[selected] && cap in (s.accounts[selected]?.accountCapabilities ?? {})) return selected; - return s.primaryAccounts[cap] ?? selected ?? null; + return accountForCapability(get().session, get().accountId, cap); + }, + + ownAccountFor(cap) { + return ownAccountForCapability(get().session, cap); }, }));