diff --git a/App.tsx b/App.tsx index 4f459a38..e0729c85 100644 --- a/App.tsx +++ b/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, diff --git a/locales/rn/en.json b/locales/rn/en.json index fb90bce8..b89ab690 100644 --- a/locales/rn/en.json +++ b/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}).", diff --git a/src/components/SidebarDrawer.tsx b/src/components/SidebarDrawer.tsx index 66dea0bd..937b4d95 100644 --- a/src/components/SidebarDrawer.tsx +++ b/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 {username || t('settings.account.title', 'Account')} - {hostname ? ( - {hostname} + {hostLabel ? ( + {hostLabel} ) : null} {busy ? ( @@ -857,8 +860,8 @@ export default function SidebarDrawer({ visible, onClose, docked = false }: Side {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); diff --git a/src/components/settings/AccountSettings.tsx b/src/components/settings/AccountSettings.tsx index e2c4a9ba..0be8994e 100644 --- a/src/components/settings/AccountSettings.tsx +++ b/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 && } {rowText.subtitle ? {toUnicodeEmail(rowText.subtitle)} : null} + {/* Two servers on one host are two accounts with the same address. */} + {nonDefaultPort(a.serverUrl) ? {serverHostLabel(a.serverUrl)} : null} {isDefault && ( {t('settings.account.accounts.default_badge', 'Default account')} diff --git a/src/lib/__tests__/account-id-port.test.ts b/src/lib/__tests__/account-id-port.test.ts new file mode 100644 index 00000000..558f8ce0 --- /dev/null +++ b/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'); + }); +}); diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index 2b2e4edd..1c1989a6 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/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)); diff --git a/src/lib/account-utils.ts b/src/lib/account-utils.ts index a330ecbc..13903bfd 100644 --- a/src/lib/account-utils.ts +++ b/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; diff --git a/src/lib/push-background-task.ts b/src/lib/push-background-task.ts index 53c09bf7..86f22ea2 100644 --- a/src/lib/push-background-task.ts +++ b/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 { @@ -452,6 +454,9 @@ export async function pushBackgroundTask(data: unknown): Promise { 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); diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index c9a61d50..1993c44e 100644 --- a/src/lib/push-notifications.ts +++ b/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>(); +/** + * 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 { - 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; diff --git a/src/stores/account-store.ts b/src/stores/account-store.ts index 58e9f946..e10a3110 100644 --- a/src/stores/account-store.ts +++ b/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()( 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()( }, ), ); + +// 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)); diff --git a/src/stores/auth-store.ts b/src/stores/auth-store.ts index b7eac632..69d80591 100644 --- a/src/stores/auth-store.ts +++ b/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((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((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((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);