fix: stop a previous account's settings leaking into the next sign-in #1185

lib/sign-out-cleanup.ts

@@ -8,7 +8,8 @@ import { fileStorage } from '@/lib/plugin-storage';
  * Kept on purpose: device preferences (`settings-storage`, `theme-storage`,
  * `locale-storage`, the Files view settings) and `template-storage`. With
  * settings sync off they exist only here, so deleting them would destroy
- * the user's own work rather than a cache.
+ * the user's own work rather than a cache. With sync on, the settings store
+ * resets settings and templates itself (forgetSyncedSettings).
  */
 const SIGNED_OUT_STORAGE_KEYS = [
   'email-snapshot',

stores/__tests__/settings-store-account-leak.test.ts

@@ -0,0 +1,184 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+const apiFetch = vi.hoisted(() => vi.fn());
+
+vi.mock('@/lib/browser-navigation', () => ({ apiFetch }));
+
+import { useSettingsStore } from '../settings-store';
+import { useTemplateStore } from '../template-store';
+
+// #1185: settings-storage is one store for the whole browser. After signing
+// out of A, an account B without server settings showed A's tags, templates
+// and preferences, and the next push wrote them into B's server file.
+
+const A = { username: 'a@example.com', serverUrl: 'https://mail.example.com' };
+const B = { username: 'b@example.com', serverUrl: 'https://mail.example.com' };
+const SECRET_TAG = { id: 'customer-x', label: 'Customer X', color: 'red' };
+
+const json = (body: unknown, status = 200) =>
+  new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } });
+
+/** Server files by username; a missing entry means "no settings yet". */
+let serverFiles: Record<string, unknown> = {};
+let failLoads = false;
+
+function posted(): Array<{ username: string; settings: Record<string, unknown> }> {
+  return apiFetch.mock.calls
+    .filter(([, init]) => (init as RequestInit | undefined)?.method === 'POST')
+    .map(([, init]) => JSON.parse((init as RequestInit).body as string));
+}
+
+async function signIn(account: typeof A) {
+  const settings = useSettingsStore.getState();
+  await settings.loadFromServer(account.username, account.serverUrl).finally(() => {
+    useSettingsStore.getState().enableSync(account.username, account.serverUrl);
+  });
+}
+
+async function signOut() {
+  await useSettingsStore.getState().flushSync();
+  useSettingsStore.getState().disableSync();
+  useSettingsStore.getState().forgetSyncedSettings();
+}
+
+/** Signed in as A, with a custom tag and a template that reached A's file. */
+async function setUpAccountA() {
+  serverFiles[A.username] = { emailKeywords: [SECRET_TAG], fontSize: 'large' };
+  await signIn(A);
+  useTemplateStore.getState().addTemplate({
+    name: 'A only', subject: '', body: '', category: '', isFavorite: false,
+  });
+  await useSettingsStore.getState().flushSync();
+  expect(useSettingsStore.getState().emailKeywords).toEqual([SECRET_TAG]);
+}
+
+beforeEach(async () => {
+  localStorage.clear();
+  serverFiles = {};
+  failLoads = false;
+  apiFetch.mockReset();
+  apiFetch.mockImplementation(async (_url: string, init?: RequestInit) => {
+    if (init?.method === 'POST') return json({ ok: true });
+    if (failLoads) return json({ error: 'boom' }, 500);
+    const username = (init?.headers as Record<string, string>)['x-settings-username'];
+    return json({ settings: serverFiles[username] ?? null });
+  });
+  useSettingsStore.getState().disableSync();
+  useSettingsStore.getState().resetToDefaults();
+  useTemplateStore.setState({ templates: [], recentTemplateIds: [], deletedTemplateIds: {} });
+});
+
+describe('switching people in one browser (#1185)', () => {
+  it('starts the next account from defaults after a sign-out', async () => {
+    await setUpAccountA();
+    await signOut();
+
+    expect(useSettingsStore.getState().emailKeywords).not.toContainEqual(SECRET_TAG);
+    expect(useTemplateStore.getState().templates).toEqual([]);
+
+    apiFetch.mockClear();
+    await signIn(B);
+    useSettingsStore.getState().updateSetting('sendDelaySeconds', 10);
+    await useSettingsStore.getState().flushSync();
+
+    const pushes = posted();
+    expect(pushes.map((p) => p.username)).toEqual([B.username]);
+    expect(pushes[0].settings.emailKeywords).not.toContainEqual(SECRET_TAG);
+    expect(pushes[0].settings.templates).toEqual([]);
+    expect(pushes[0].settings.fontSize).toBe('medium');
+  });
+
+  it("drops A's state when B signs in without a sign-out in between", async () => {
+    await setUpAccountA();
+    useSettingsStore.getState().disableSync(); // e.g. the session expired
+
+    apiFetch.mockClear();
+    await signIn(B);
+    useSettingsStore.getState().updateSetting('sendDelaySeconds', 10);
+    await useSettingsStore.getState().flushSync();
+
+    expect(useSettingsStore.getState().emailKeywords).not.toContainEqual(SECRET_TAG);
+    const pushes = posted();
+    expect(pushes.map((p) => p.username)).toEqual([B.username]);
+    expect(pushes[0].settings.emailKeywords).not.toContainEqual(SECRET_TAG);
+    expect(pushes[0].settings.templates).toEqual([]);
+  });
+
+  it("does not merge A's templates into B's server file", async () => {
+    await setUpAccountA();
+    useSettingsStore.getState().disableSync();
+    serverFiles[B.username] = { fontSize: 'small', templates: [], deletedTemplateIds: {} };
+
+    apiFetch.mockClear();
+    await signIn(B);
+    await useSettingsStore.getState().flushSync();
+
+    expect(useTemplateStore.getState().templates).toEqual([]);
+    expect(useSettingsStore.getState().emailKeywords).not.toContainEqual(SECRET_TAG);
+    expect(useSettingsStore.getState().fontSize).toBe('small');
+    expect(posted()).toEqual([]);
+  });
+
+  it("keeps sync off when B's settings cannot be loaded over A's", async () => {
+    await setUpAccountA();
+    useSettingsStore.getState().disableSync();
+    failLoads = true;
+
+    apiFetch.mockClear();
+    await signIn(B);
+    useSettingsStore.getState().updateSetting('sendDelaySeconds', 10);
+    await useSettingsStore.getState().flushSync();
+
+    expect(useSettingsStore.getState().emailKeywords).not.toContainEqual(SECRET_TAG);
+    expect(posted()).toEqual([]);
+  });
+
+  it('lets the same account pick up where it left off', async () => {
+    await setUpAccountA();
+    useSettingsStore.getState().disableSync();
+    failLoads = true;
+
+    await signIn(A);
+    useSettingsStore.getState().updateSetting('sendDelaySeconds', 10);
+    await useSettingsStore.getState().flushSync();
+
+    expect(useSettingsStore.getState().emailKeywords).toEqual([SECRET_TAG]);
+    expect(posted().at(-1)?.username).toBe(A.username);
+  });
+});
+
+describe('settings that never reached a server', () => {
+  it('lets the first synced sign-in adopt the device settings', async () => {
+    useSettingsStore.getState().addKeyword(SECRET_TAG);
+
+    await signIn(A);
+    expect(useSettingsStore.getState().emailKeywords).toContainEqual(SECRET_TAG);
+    useSettingsStore.getState().updateSetting('sendDelaySeconds', 10);
+    await useSettingsStore.getState().flushSync();
+    expect(posted()[0].settings.emailKeywords).toContainEqual(SECRET_TAG);
+
+    // From here on they live in A's file, so a sign-out may drop them.
+    await signOut();
+    expect(useSettingsStore.getState().emailKeywords).not.toContainEqual(SECRET_TAG);
+  });
+
+  it('keeps them on sign-out when sync was never used', () => {
+    useSettingsStore.getState().addKeyword(SECRET_TAG);
+    useSettingsStore.getState().forgetSyncedSettings();
+    expect(useSettingsStore.getState().emailKeywords).toContainEqual(SECRET_TAG);
+  });
+
+  it('keeps them on sign-out when the user opted out of sync', async () => {
+    await setUpAccountA();
+    useSettingsStore.setState({ settingsSyncDisabled: true });
+    await signOut();
+    expect(useSettingsStore.getState().emailKeywords).toEqual([SECRET_TAG]);
+  });
+
+  it('keeps device-only settings through the reset', async () => {
+    useSettingsStore.setState({ proInterface: true });
+    await setUpAccountA();
+    await signOut();
+    expect(useSettingsStore.getState().proInterface).toBe(true);
+  });
+});

stores/auth-store.ts

@@ -1196,6 +1196,10 @@ function forgetCalendarSubscriptions(client: IJMAPClient): void {
 
 function performFullLogout(set: (state: Partial<AuthState>) => void): void {
   useSettingsStore.getState().disableSync();
+  // With settings sync on, the local settings and templates are a copy of the
+  // signed-out account's server file. Left in place, the next account to sign
+  // in here would show them and push them to its own file (#1185).
+  useSettingsStore.getState().forgetSyncedSettings();
 
   set({
     isAuthenticated: false,
@@ -2235,6 +2239,9 @@ export const useAuthStore = create<AuthState>()(
         // while the clients are torn down.
         const cleanup = clearAllCredentials(endSessionSlot);
 
+        // The settings are reset below; save the last edits first.
+        await useSettingsStore.getState().flushSync();
+
         // Disconnect all clients
         set({ client: null });
         for (const c of clients.values()) {

stores/settings-store.ts

@@ -36,6 +36,35 @@ let isLoadingFromServer = false;
 let pushTemplatesAfterLoad = false;
 // Wired up in the window-only init block below; null during SSR.
 let requestSync: (() => void) | null = null;
+// Account whose settings could not be loaded while this browser still held
+// another account's. enableSync leaves sync off for it, or the next change
+// would push the other account's settings under its name (#1185).
+let blockedSyncAccountId: string | null = null;
+
+// settings-storage is one store for the whole browser. This key names the
+// account whose server copy it currently mirrors, so a different account
+// signing in can tell the local settings are not its own (#1185). Unset in
+// browsers that never synced, where the local settings are the user's own
+// device settings and the first sign-in adopts them.
+const SETTINGS_OWNER_KEY = 'settings-sync-owner';
+
+function readSettingsOwner(): string | null {
+  try {
+    return typeof window === 'undefined' ? null : window.localStorage.getItem(SETTINGS_OWNER_KEY);
+  } catch {
+    return null;
+  }
+}
+
+function writeSettingsOwner(accountId: string | null): void {
+  try {
+    if (typeof window === 'undefined') return;
+    if (accountId) window.localStorage.setItem(SETTINGS_OWNER_KEY, accountId);
+    else window.localStorage.removeItem(SETTINGS_OWNER_KEY);
+  } catch {
+    /* storage unavailable */
+  }
+}
 
 const SYNC_DEBOUNCE_MS = 2000;
 
@@ -590,6 +619,12 @@ interface SettingsState {
   flushSync: () => Promise<void>;
   disableSync: () => void;
   loadFromServer: (username: string, serverUrl: string) => Promise<boolean>;
+  /**
+   * Called on a full sign-out: drops the settings and templates this browser
+   * mirrored from an account's server copy, so the next person to sign in
+   * does not start from them (#1185). Device-only settings are kept.
+   */
+  forgetSyncedSettings: () => void;
 }
 
 const DEFAULT_SETTINGS = {
@@ -804,7 +839,29 @@ const DEFAULT_SETTINGS = {
 
 export const useSettingsStore = create<SettingsState>()(
   persist(
-    (set, get) => ({
+    (set, get) => {
+      /**
+       * Back to defaults for everything an account's server copy holds:
+       * settings (device-only ones kept) and templates. Never pushed, since
+       * sync may still point at the account the state came from.
+       */
+      const resetSyncedState = () => {
+        const defaults: Record<string, unknown> = { ...DEFAULT_SETTINGS };
+        for (const key of DEVICE_LOCAL_SETTING_KEYS) delete defaults[key];
+        const wasLoading = isLoadingFromServer;
+        isLoadingFromServer = true;
+        try {
+          set(defaults as Partial<SettingsState>);
+          templateSyncBridge?.applySyncedState([], {}, { merge: false });
+        } finally {
+          isLoadingFromServer = wasLoading;
+        }
+        applyFontSize(DEFAULT_SETTINGS.fontSize);
+        applyDensity(DEFAULT_SETTINGS.density);
+        applyAnimations(DEFAULT_SETTINGS.animationsEnabled);
+      };
+
+      return {
       ...DEFAULT_SETTINGS,
 
       updateSetting: (key, value) => {
@@ -1175,6 +1232,12 @@ export const useSettingsStore = create<SettingsState>()(
 
       // Settings sync methods
       enableSync: (username: string, serverUrl: string) => {
+        const accountId = generateAccountId(username, serverUrl);
+        if (blockedSyncAccountId === accountId) {
+          syncWarn('Settings sync stays off for', username, '- its settings could not be loaded');
+          return;
+        }
+        writeSettingsOwner(accountId);
         syncUsername = username;
         syncServerUrl = serverUrl;
         syncEnabled = true;
@@ -1208,6 +1271,21 @@ export const useSettingsStore = create<SettingsState>()(
       },
 
       loadFromServer: async (username: string, serverUrl: string) => {
+        const accountId = generateAccountId(username, serverUrl);
+        const owner = readSettingsOwner();
+        const heldByOther = owner !== null && owner !== accountId;
+        // The local settings belong to another account and this one's could
+        // not be read: show defaults and keep sync off, rather than showing
+        // the other account's settings or pushing them under this name.
+        const failWithoutForeignState = () => {
+          if (heldByOther) {
+            resetSyncedState();
+            writeSettingsOwner(null);
+            blockedSyncAccountId = accountId;
+          }
+          return false;
+        };
+        blockedSyncAccountId = null;
         try {
           syncLog('Loading settings from server for', username);
           pushTemplatesAfterLoad = false;
@@ -1220,22 +1298,31 @@ export const useSettingsStore = create<SettingsState>()(
           if (!res.ok) {
             const body = await res.json().catch(() => ({}));
             syncLog('Settings fetch failed:', body.error || `status ${res.status}`);
-            return false;
+            return failWithoutForeignState();
           }
           const { settings } = await res.json();
           if (!settings) {
             syncLog('No server settings found yet');
+            // A new account starts from defaults, not from whatever the
+            // previous account left in this browser (#1185).
+            if (heldByOther) resetSyncedState();
+            writeSettingsOwner(accountId);
             pushTemplatesAfterLoad = localTemplatesAhead(null);
             return false;
           }
           if (settings && typeof settings === 'object') {
+            // Importing merges templates and leaves keys the blob lacks as
+            // they are, so another account's state would carry over into
+            // this one and be pushed back to its server copy.
+            if (heldByOther) resetSyncedState();
             isLoadingFromServer = true;
             // Merge (not replace) per-account maps for the account being loaded,
             // so multi-account logins don't clobber each other by login order.
             get().importSettings(JSON.stringify(settings), {
-              serverAccountId: generateAccountId(username, serverUrl),
+              serverAccountId: accountId,
             });
             isLoadingFromServer = false;
+            writeSettingsOwner(accountId);
             pushTemplatesAfterLoad = localTemplatesAhead(settings);
             syncLog('Settings loaded from server successfully');
             // The per-account preferred sender identity (#507) is re-applied by
@@ -1248,10 +1335,19 @@ export const useSettingsStore = create<SettingsState>()(
         } catch (error) {
           syncError('Failed to load settings from server:', error);
           isLoadingFromServer = false;
-          return false;
+          return failWithoutForeignState();
         }
       },
-    }),
+
+      forgetSyncedSettings: () => {
+        // Never synced: the local settings are this device's own and exist
+        // nowhere else. Synced but opted out: they were never uploaded either.
+        if (readSettingsOwner() === null || get().settingsSyncDisabled) return;
+        resetSyncedState();
+        writeSettingsOwner(null);
+      },
+      };
+    },
     {
       name: 'settings-storage',
       version: 7,