fix: keep two accounts on the same host apart by port

App.tsx

@@ -654,7 +654,10 @@ export default function App() {
   );
   const activeAccountId = useAuthStore((s) => s.activeAccountId);
   React.useEffect(() => {
-    if (!isAuthenticated || !client) return;
+    // The setup asks the server through the client: it waits for the
+    // session, and runs again once one comes up (an account added or
+    // switched to, a cold start that began offline).
+    if (!isAuthenticated || !client || !haveLiveSession) return;
 
     let cancelled = false;
     const doSetup = async () => {
@@ -700,7 +703,7 @@ export default function App() {
       unsubscribe();
       unsubscribeUp();
     };
-  }, [client, isAuthenticated, emailNotificationsEnabled, activeAccountId]);
+  }, [client, isAuthenticated, emailNotificationsEnabled, activeAccountId, haveLiveSession]);
 
   // Muting a folder or account changes the server-side push filter (#78),
   // which the setup above would only send at the next launch. Send it now,

locales/rn/en.json

@@ -560,10 +560,10 @@
     "export_failed": "Export failed",
     "forwarded": "Forwarded",
     "load_failed": "Failed to load email",
-    "next": "Next",
-    "not_spam_short": "Not spam",
     "new_tag": "New tag",
     "new_tag_placeholder": "Tag name",
+    "next": "Next",
+    "not_spam_short": "Not spam",
     "pause_animation": "Pause animation",
     "play_animation": "Play animation",
     "preview": {
@@ -1341,6 +1341,7 @@
         "accounts_desc_mobile": "Background notifications of each signed-in account on this device. Switch to an account to turn them on or off for it.",
         "confirm_disable_message_mobile": "This device will stop receiving new-mail alerts for this account.",
         "description_mobile": "Receive system notifications for new mail when the app is closed. Delivered via the Bulwark push relay; the relay never sees mail content.",
+        "err_account_changed": "The account changed while push was being set up. Try again.",
         "err_android_only": "Push notifications are only available on Android.",
         "err_apns_empty": "iOS returned an empty push token.",
         "err_apns_token": "iOS could not issue a push token ({detail}).",

src/components/SidebarDrawer.tsx

@@ -21,7 +21,7 @@ import { useAccountStore } from '../stores/account-store';
 import { useSettingsStore } from '../stores/settings-store';
 import { useKeywordsStore, keywordToken } from '../stores/keywords-store';
 import { useLocaleStore } from '../stores/locale-store';
-import { MAX_ACCOUNTS, accountRowText } from '../lib/account-utils';
+import { MAX_ACCOUNTS, accountRowText, serverHostLabel } from '../lib/account-utils';
 import { toUnicodeEmail } from '../lib/idn';
 import {
   buildMailboxTree, flattenVisible, mailboxSubtreeIds, ownMailboxes, type MailboxNode,
@@ -788,6 +788,9 @@ export default function SidebarDrawer({ visible, onClose, docked = false }: Side
     if (!serverUrl) return '';
     try { return new URL(serverUrl).hostname; } catch { return serverUrl; }
   }, [serverUrl]);
+  // The header names the server with its port when that isn't the default:
+  // two servers on one host are two accounts with the same address.
+  const hostLabel = React.useMemo(() => (serverUrl ? serverHostLabel(serverUrl) : ''), [serverUrl]);
   const accountEmailFull = React.useMemo(() => {
     if (!username) return hostname;
     if (username.includes('@')) return username;
@@ -836,8 +839,8 @@ export default function SidebarDrawer({ visible, onClose, docked = false }: Side
             </View>
             <View style={styles.accountInfo}>
               <Text style={styles.accountName} numberOfLines={1}>{username || t('settings.account.title', 'Account')}</Text>
-              {hostname ? (
-                <Text style={styles.accountEmail} numberOfLines={1}>{hostname}</Text>
+              {hostLabel ? (
+                <Text style={styles.accountEmail} numberOfLines={1}>{hostLabel}</Text>
               ) : null}
             </View>
             {busy ? (
@@ -857,8 +860,8 @@ export default function SidebarDrawer({ visible, onClose, docked = false }: Side
             <ScrollView style={styles.accountMenuList}>
               {accounts.map((acc) => {
                 const isActive = acc.id === activeAccountId;
-                let accHost = '';
-                try { accHost = new URL(acc.serverUrl).hostname; } catch { accHost = acc.serverUrl; }
+                // With the port when it isn't the default (see hostLabel).
+                const accHost = serverHostLabel(acc.serverUrl);
                 // The address leads when another account has the same name
                 // (webmail 3bb9f7df8).
                 const rowText = accountRowText(acc, accounts);

src/components/settings/AccountSettings.tsx

@@ -15,7 +15,7 @@ import { setPendingSettingsTab } from '../../navigation/pending-settings-tab';
 import { sharedAccountSettingsTabs } from '../../lib/capabilities';
 import { jmapClient } from '../../api/jmap-client';
 import { fetchMailQuota, type MailQuota } from '../../api/quota';
-import { MAX_ACCOUNTS, accountRowText } from '../../lib/account-utils';
+import { MAX_ACCOUNTS, accountRowText, nonDefaultPort, serverHostLabel } from '../../lib/account-utils';
 import { toUnicodeEmail } from '../../lib/idn';
 import type { RootStackParamList } from '../../navigation/types';
 
@@ -189,6 +189,8 @@ export function AccountSettings() {
                     {isActive && <Check size={14} color={c.success} />}
                   </View>
                   {rowText.subtitle ? <Text style={styles.rowSub} numberOfLines={1}>{toUnicodeEmail(rowText.subtitle)}</Text> : null}
+                  {/* Two servers on one host are two accounts with the same address. */}
+                  {nonDefaultPort(a.serverUrl) ? <Text style={styles.rowSub} numberOfLines={1}>{serverHostLabel(a.serverUrl)}</Text> : null}
                   {isDefault && (
                     <Text style={styles.defaultBadge}>
                       {t('settings.account.accounts.default_badge', 'Default account')}

src/lib/__tests__/account-id-port.test.ts

@@ -0,0 +1,95 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { generateAccountId, nonDefaultPort, serverHostLabel, setKnownAccounts } from '../account-utils';
+import { useAccountStore } from '../../stores/account-store';
+
+// The same address on two servers on one host was one account: ids were
+// username@host, so signing in on http://10.10.10.62:8016 replaced the
+// account of http://10.10.10.62:8120. The port is part of the id now, while
+// accounts already signed in keep the id they have.
+
+const entry = (username: string, serverUrl: string) => ({
+  serverUrl,
+  username,
+  displayName: username,
+  email: username,
+  lastLoginAt: 1,
+  isConnected: true,
+  hasError: false,
+});
+
+beforeEach(() => {
+  useAccountStore.setState({ accounts: [], activeAccountId: null, defaultAccountId: null });
+});
+
+afterEach(() => {
+  useAccountStore.setState({ accounts: [], activeAccountId: null, defaultAccountId: null });
+  setKnownAccounts([]);
+});
+
+describe('generateAccountId', () => {
+  it('keeps the ids of accounts on the default ports', () => {
+    expect(generateAccountId('u@example.org', 'https://mail.example.org')).toBe('u@example.org@mail.example.org');
+    expect(generateAccountId('u@example.org', 'https://mail.example.org:443/')).toBe('u@example.org@mail.example.org');
+    expect(generateAccountId('u@example.org', 'http://mail.example.org:80/jmap')).toBe('u@example.org@mail.example.org');
+  });
+
+  it('tells two servers on one host apart by port', () => {
+    const a = generateAccountId('usera@example.org', 'http://10.10.10.62:8120');
+    const b = generateAccountId('usera@example.org', 'http://10.10.10.62:8016');
+    expect(a).toBe('usera@example.org@10.10.10.62:8120');
+    expect(b).toBe('usera@example.org@10.10.10.62:8016');
+  });
+
+  it('keeps the host-only id of an account signed in before ports counted', () => {
+    setKnownAccounts([{ id: 'usera@example.org@10.10.10.62', username: 'usera@example.org', serverUrl: 'http://10.10.10.62:8120' }]);
+    expect(generateAccountId('usera@example.org', 'http://10.10.10.62:8120')).toBe('usera@example.org@10.10.10.62');
+    expect(generateAccountId('usera@example.org', 'http://10.10.10.62:8120/')).toBe('usera@example.org@10.10.10.62');
+    // Another server on the same host is another account.
+    expect(generateAccountId('usera@example.org', 'http://10.10.10.62:8016')).toBe('usera@example.org@10.10.10.62:8016');
+  });
+});
+
+describe('the account registry', () => {
+  it('adds the same address on another port of the same host as a second account', () => {
+    const store = useAccountStore.getState();
+    const a = store.addAccount(entry('usera@example.org', 'http://10.10.10.62:8120'));
+    const b = useAccountStore.getState().addAccount(entry('usera@example.org', 'http://10.10.10.62:8016'));
+    expect(a).not.toBe(b);
+    const { accounts } = useAccountStore.getState();
+    expect(accounts.map((x) => x.serverUrl)).toEqual(['http://10.10.10.62:8120', 'http://10.10.10.62:8016']);
+  });
+
+  it('keeps an existing account under its old id when another port is added', () => {
+    // As persisted by an earlier version.
+    useAccountStore.setState({
+      accounts: [{
+        ...entry('usera@example.org', 'http://10.10.10.62:8120'),
+        id: 'usera@example.org@10.10.10.62',
+        avatarColor: '#000',
+        isDefault: true,
+      }],
+      activeAccountId: 'usera@example.org@10.10.10.62',
+      defaultAccountId: 'usera@example.org@10.10.10.62',
+    });
+    // Signing in on the old server again updates that account.
+    expect(useAccountStore.getState().addAccount(entry('usera@example.org', 'http://10.10.10.62:8120')))
+      .toBe('usera@example.org@10.10.10.62');
+    const b = useAccountStore.getState().addAccount(entry('usera@example.org', 'http://10.10.10.62:8016'));
+    expect(b).toBe('usera@example.org@10.10.10.62:8016');
+    expect(useAccountStore.getState().accounts.map((x) => [x.id, x.serverUrl])).toEqual([
+      ['usera@example.org@10.10.10.62', 'http://10.10.10.62:8120'],
+      ['usera@example.org@10.10.10.62:8016', 'http://10.10.10.62:8016'],
+    ]);
+  });
+});
+
+describe('serverHostLabel', () => {
+  it('shows the port only when it is not the default', () => {
+    expect(serverHostLabel('https://mail.example.org')).toBe('mail.example.org');
+    expect(serverHostLabel('https://mail.example.org:443')).toBe('mail.example.org');
+    expect(serverHostLabel('http://10.10.10.62:8016/')).toBe('10.10.10.62:8016');
+    expect(serverHostLabel('https://mail.example.org:8443/jmap')).toBe('mail.example.org:8443');
+    expect(nonDefaultPort('http://mail.example.org:80')).toBeNull();
+    expect(nonDefaultPort('https://mail.example.org:80')).toBe('80');
+  });
+});

src/lib/__tests__/push-notifications.test.ts

@@ -80,6 +80,7 @@ vi.mock('../../api/jmap-client', () => ({
   jmapClient: {
     username: 'user@example.com',
     serverUrl: 'https://mail.example.com',
+    isConnected: true,
     accountId: 'jmap-primary',
     currentSession: { capabilities: { 'urn:ietf:params:jmap:core': {} } },
   },
@@ -1582,6 +1583,45 @@ describe('resyncPushNotifications', () => {
     expect(createMock).not.toHaveBeenCalled();
   });
 
+  it('waits for the session instead of asking the server without one', async () => {
+    // An account being added: the client holds its credentials, no session yet.
+    const client = jmapClient as unknown as { isConnected: boolean };
+    client.isConnected = false;
+    try {
+      await AsyncStorage.setItem(SUB_KEY, 'existing');
+      await AsyncStorage.setItem(EXPIRES_KEY, inDays(5));
+      const result = await resyncPushNotifications({ relayBaseUrl: RELAY });
+      expect(result).toBeNull();
+      expect(listMock).not.toHaveBeenCalled();
+      expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull();
+    } finally {
+      client.isConnected = true;
+    }
+  });
+
+  it('keeps push on when the client moved to another account during the revocation check', async () => {
+    await AsyncStorage.setItem(SUB_KEY, 'existing');
+    await AsyncStorage.setItem(EXPIRES_KEY, inDays(5));
+    await AsyncStorage.setItem('push:accountIds:v1', JSON.stringify([ACCOUNT_ID]));
+    const client = jmapClient as unknown as { serverUrl: string };
+    // The listing answers for the account added meanwhile, which has none of ours.
+    listMock.mockImplementation(async () => {
+      client.serverUrl = 'https://mail.example.com:8443';
+      return [];
+    });
+    try {
+      const result = await resyncPushNotifications({ relayBaseUrl: RELAY });
+      expect(result).toBeNull();
+      expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull();
+      expect(await AsyncStorage.getItem(SUB_KEY)).toBe('existing');
+      expect(await readPushAccountIds()).toEqual([ACCOUNT_ID]);
+    } finally {
+      client.serverUrl = 'https://mail.example.com';
+      listMock.mockReset();
+      listMock.mockResolvedValue([]);
+    }
+  });
+
   it('does not re-register a subscription another device revoked', async () => {
     await AsyncStorage.setItem(SUB_KEY, 'existing');
     await AsyncStorage.setItem(EXPIRES_KEY, inDays(5));

src/lib/account-utils.ts

@@ -13,7 +13,34 @@ export class AccountLimitError extends Error {
   }
 }
 
-export function generateAccountId(username: string, serverUrl: string): string {
+/**
+ * The port of `serverUrl` when it is not the scheme's default (443 for
+ * https, 80 for http), else null. Read from the text, since React Native's
+ * URL keeps a default port where the WHATWG one drops it.
+ */
+export function nonDefaultPort(serverUrl: string): string | null {
+  const m = /^([a-z][a-z0-9+.-]*):\/\/(?:[^@/?#]*@)?(?:\[[^\]]*\]|[^:/?#]*):(\d+)(?=[/?#]|$)/i.exec(serverUrl.trim());
+  if (!m) return null;
+  const scheme = m[1].toLowerCase();
+  const port = String(Number(m[2]));
+  if ((scheme === 'https' && port === '443') || (scheme === 'http' && port === '80')) return null;
+  return port;
+}
+
+/** `serverUrl`'s host, with its port when that is not the default. */
+export function serverHostLabel(serverUrl: string): string {
+  let host: string;
+  try {
+    host = new URL(serverUrl).hostname;
+  } catch {
+    return serverUrl.replace(/^https?:\/\//, '').replace(/\/.*$/, '');
+  }
+  const port = nonDefaultPort(serverUrl);
+  return port ? `${host}:${port}` : host;
+}
+
+/** The id an account got before ports were part of it: username@host. */
+function hostOnlyAccountId(username: string, serverUrl: string): string {
   let host = serverUrl;
   try {
     host = new URL(serverUrl).hostname;
@@ -23,6 +50,47 @@ export function generateAccountId(username: string, serverUrl: string): string {
   return `${username}@${host}`;
 }
 
+/** What the registry tells generateAccountId (structural: no AccountEntry import). */
+export interface KnownAccount {
+  id: string;
+  username: string;
+  serverUrl: string;
+}
+
+let knownAccounts: readonly KnownAccount[] = [];
+
+/**
+ * The registry's accounts, so generateAccountId keeps the ids accounts were
+ * signed in under. The account store feeds this whenever it changes; the
+ * headless push task, which reads the registry from storage, feeds it too.
+ */
+export function setKnownAccounts(accounts: readonly KnownAccount[]): void {
+  knownAccounts = accounts;
+}
+
+/**
+ * The local id of the account `username` has on `serverUrl`:
+ * username@host, plus `:port` when the server is not on its scheme's
+ * default port. Two servers on one host are two accounts.
+ *
+ * Accounts were once keyed by username@host only. One signed in that way on
+ * a server with a port keeps that id, and with it its credentials, caches,
+ * outbox, push registration and settings: when the registry holds the
+ * host-only id for the same server, that is the answer. A second server on
+ * the same host then gets the id with its port, and the two stay apart.
+ * Accounts on the default ports have the same id either way.
+ */
+export function generateAccountId(username: string, serverUrl: string): string {
+  const legacy = hostOnlyAccountId(username, serverUrl);
+  const host = serverHostLabel(serverUrl);
+  const id = `${username}@${host}`;
+  if (id === legacy) return id;
+  const kept = knownAccounts.find((a) => a.id === legacy
+    && a.username === username
+    && `${a.username}@${serverHostLabel(a.serverUrl)}` === id);
+  return kept ? kept.id : id;
+}
+
 /** What naming an account in a list needs (structural: no AccountEntry import). */
 export interface NamedAccount {
   id: string;

src/lib/push-background-task.ts

@@ -5,6 +5,7 @@ import { CAPABILITIES } from '../api/types';
 import type { Email, JMAPMethodCall, JMAPSession, Mailbox } from '../api/types';
 import { detectDeviceLocale, isSupportedLocale, translate, type LocaleCode } from '../i18n';
 import { secureFetch } from './client-cert';
+import { setKnownAccounts, type KnownAccount } from './account-utils';
 import { freshStoredCredentials } from './stored-token-refresh';
 import {
   generateEmailAvatarColor,
@@ -194,6 +195,7 @@ export function parseRelayPushData(data: unknown): RelayPushData {
 interface RegistryAccount {
   id: string;
   username?: string;
+  serverUrl?: string;
 }
 
 async function readAccountRegistry(): Promise<RegistryAccount[]> {
@@ -452,6 +454,9 @@ export async function pushBackgroundTask(data: unknown): Promise<void> {
     if (accountIds.length === 0) return;
 
     const registry = await readAccountRegistry();
+    // Without the account store loaded, account ids are still the ones the
+    // registry holds (see generateAccountId).
+    setKnownAccounts(registry.filter((a): a is KnownAccount => !!a.username && !!a.serverUrl));
     const jmapAccountIds = await readPushJmapAccountIds();
     const match = matchAccountsForPush(payload, accountIds, jmapAccountIds, registry);
     if (match.ambiguous) await showPlainNotification(payload);

src/lib/push-notifications.ts

@@ -1224,6 +1224,28 @@ async function pollVerificationCode(
 // PushVerification"). Callers share the first in-flight run instead.
 const inFlightSetups = new Map<string, Promise<PushSetupResult>>();
 
+/**
+ * The local account the singleton client serves with a live session; null
+ * while it has none, such as during an account change, when it holds the
+ * credentials of the account being signed in to before its session is up.
+ * The setup asks the server through the client, so it must know the client
+ * still serves the account it is setting up: a listing of another account's
+ * subscriptions took the account's own for revoked and turned its push off.
+ */
+function servedAccountId(): string | null {
+  const username = jmapClient.username;
+  const serverUrl = jmapClient.serverUrl;
+  if (!jmapClient.isConnected || !username || !serverUrl) return null;
+  return generateAccountId(username, serverUrl);
+}
+
+function accountChangedError(): PushSetupError {
+  return new PushSetupError(
+    'account',
+    t('settings.notifications.push.err_account_changed', 'The account changed while push was being set up. Try again.'),
+  );
+}
+
 /**
  * Full setup flow: ask permission, fetch the device's FCM token, register
  * with the relay, create a JMAP PushSubscription, poll for the verification
@@ -1300,12 +1322,10 @@ async function setupPushNotificationsInner(
 
   // setupPushNotifications operates on the currently-loaded jmapClient. We
   // need its username/serverUrl up-front so we can key per-account state.
-  const username = jmapClient.username;
-  const serverUrl = jmapClient.serverUrl;
-  if (!username || !serverUrl) {
+  const accountId = servedAccountId();
+  if (!accountId) {
     throw new PushSetupError('account', t('settings.notifications.push.err_no_account', 'No account loaded - cannot set up push.'));
   }
-  const accountId = generateAccountId(username, serverUrl);
 
   await migrateLegacyPushKeys();
 
@@ -1381,6 +1401,8 @@ async function setupPushNotificationsInner(
   // A failed listing ends the run here: taken for "the server has no
   // subscription", it would drop the key of a working registration.
   logPhase('jmap');
+  // From here on the server is asked through the client (see servedAccountId).
+  if (servedAccountId() !== accountId) throw accountChangedError();
   let existingSubs: PushSubscription[];
   try {
     existingSubs = await listPushSubscriptions();
@@ -1868,12 +1890,15 @@ function isOwnSubscription(
 export async function resyncPushNotifications(
   params: PushSetupParams,
 ): Promise<PushSetupResult | null> {
-  const username = jmapClient.username;
-  const serverUrl = jmapClient.serverUrl;
-  if (!username || !serverUrl) return null;
-  const accountId = generateAccountId(username, serverUrl);
+  // Without a live session (an account change under way) there is nothing
+  // to ask; the caller runs this again once the session is up.
+  const accountId = servedAccountId();
+  if (!accountId) return null;
   if (await AsyncStorage.getItem(optedOutKey(accountId))) return null;
-  if (await wasRevokedOnServer(accountId)) {
+  const revoked = await wasRevokedOnServer(accountId);
+  // The listing went to whatever account the client served meanwhile.
+  if (servedAccountId() !== accountId) return null;
+  if (revoked) {
     logPhase('revoked', 'subscription gone before it was due to expire; leaving push off');
     await disablePushForAccount(accountId);
     return null;

src/stores/account-store.ts

@@ -1,7 +1,7 @@
 import { create } from 'zustand';
 import { persist } from 'zustand/middleware';
 import { createPersistStorage } from './persist-storage';
-import { AccountLimitError, generateAccountId, MAX_ACCOUNTS } from '../lib/account-utils';
+import { AccountLimitError, generateAccountId, MAX_ACCOUNTS, setKnownAccounts } from '../lib/account-utils';
 import { generateAvatarColor } from '../lib/avatar-utils';
 
 export interface AccountEntry {
@@ -58,6 +58,9 @@ export const useAccountStore = create<AccountState>()(
               a.id === id
                 ? {
                     ...a,
+                    // The same server under another URL (a path, a scheme):
+                    // the credentials just stored carry the new one.
+                    serverUrl: entry.serverUrl || a.serverUrl,
                     displayName: entry.displayName || a.displayName,
                     email: entry.email || a.email,
                     lastLoginAt: entry.lastLoginAt,
@@ -180,3 +183,8 @@ export const useAccountStore = create<AccountState>()(
     },
   ),
 );
+
+// generateAccountId keeps the ids accounts were signed in under, which it
+// reads from the registry (see lib/account-utils).
+setKnownAccounts(useAccountStore.getState().accounts);
+useAccountStore.subscribe((state) => setKnownAccounts(state.accounts));

src/stores/auth-store.ts

@@ -203,6 +203,19 @@ async function checkServerChange(accountId: string, session: JMAPSession): Promi
   }
 }
 
+// The registry names the server the account's stored credentials sign in
+// to. Before ports were part of account ids, signing in to a second server
+// on the same host stored its credentials under the first server's account
+// and left the registry naming the first server. That account has been the
+// second server's since (its mail, its sends), and now says so; the ids
+// stay consistent with it (see generateAccountId).
+function followStoredServer(accountId: string): void {
+  const served = jmapClient.serverUrl;
+  const accountStore = useAccountStore.getState();
+  const entry = accountStore.getAccountById(accountId);
+  if (served && entry && entry.serverUrl !== served) accountStore.updateAccount(accountId, { serverUrl: served });
+}
+
 // The outbox replays against whatever account the JMAP client serves, and a
 // replaced server may have given the queued ids to other messages. So it
 // runs only once the account's session is up and its server was checked.
@@ -993,6 +1006,7 @@ export const useAuthStore = create<AuthState>((set, get) => ({
         set({ isLoading: false, error: 'Failed to load session' });
         return;
       }
+      followStoredServer(accountId);
       await checkServerChange(accountId, session);
       if (!isCurrent()) return;
 
@@ -1139,6 +1153,7 @@ export const useAuthStore = create<AuthState>((set, get) => ({
       });
 
       const session = jmapClient.currentSession!;
+      followStoredServer(target.id);
       await checkServerChange(target.id, session);
       applyConnectedState(set, session, target.serverUrl, target.username, target.id);
       perfMark('restore-done');
@@ -1183,6 +1198,7 @@ export const useAuthStore = create<AuthState>((set, get) => ({
         errorMessage: undefined,
       });
       const fresh = jmapClient.currentSession!;
+      followStoredServer(activeAccountId);
       await checkServerChange(activeAccountId, fresh);
       if (!isCurrent()) return false;
       applyConnectedState(set, fresh, target.serverUrl, target.username, activeAccountId);