From a330d77d6d2e043007a63c0b90829dd817f8a651 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:43:22 +0200 Subject: [PATCH] fix: keep push, queued edits and iCal feeds safe on a replaced server The app compares the server behind an account with the one it recorded and drops what came from a replaced one. The released version recorded none, so on the first start of this one there is nothing to compare with, and a server replaced before the update (Stalwart 1.0 is a fresh install) went unnoticed: - the push subscription expiry the released version recorded made the subscription the new server lacks look revoked, and push was turned off for good; it is now forgotten on the first sight of an account, so the subscription is re-created - the queue of offline edits the released version left was replayed against the new server, where its ids name other messages, so a queued delete deleted another message; it is now discarded on the first sight, with the notice a replaced server shows. Nothing proves a queued edit belongs to the server: the queue records none, a reinstalled Stalwart may accept old JMAP states, and a queued move or delete took its message out of the cached list, so there is no copy to compare the server's message with iCal subscriptions recognise their mirror calendar by name, but a sync still imported the feed into whatever calendar the recorded id named: - the feed now goes only into a calendar that carries the subscription's name, and otherwise into a new mirror; when the name can't be read, the sync fails and is tried again - a rename the server refused renamed the subscription anyway, which then no longer recognised its mirror: pruning stopped and removing the subscription kept the calendar. Now nothing changes and the error is shown - a replaced server claimed subscriptions made before they recorded their account for every account with the same JMAP id, another server's account included. They now go only when no other signed-in account has that id, or may have it, and a sync that finds their mirror records the account on them --- src/lib/__tests__/push-notifications.test.ts | 34 +++++ src/lib/push-notifications.ts | 14 +- src/lib/server-change.ts | 62 ++++++-- .../auth-store-server-change.test.ts | 17 +++ .../calendar-subscriptions-store.test.ts | 134 +++++++++++++++++- src/stores/__tests__/server-change.test.ts | 74 +++++++++- src/stores/calendar-subscriptions-store.ts | 122 +++++++++++----- 7 files changed, 399 insertions(+), 58 deletions(-) diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index ee4225f7..9fedf0de 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -33,6 +33,10 @@ vi.mock('react-native', () => { }, NativeEventEmitter, PermissionsAndroid: { RESULTS: { GRANTED: 'granted' }, request: vi.fn(async () => 'granted') }, + // What the stores the server check drops from need at load. + AppState: { currentState: 'active', addEventListener: () => ({ remove: () => undefined }) }, + Appearance: { getColorScheme: () => 'dark', addChangeListener: () => ({ remove: () => undefined }) }, + Linking: { openURL: async () => undefined }, }; }); @@ -96,6 +100,7 @@ import type { EmailPushConfig } from '../../api/types'; import { jmapClient } from '../../api/jmap-client'; import { NativeModules } from 'react-native'; import { generateAccountId } from '../account-utils'; +import { reconcileServerFingerprint } from '../server-change'; const OUR_DCID = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; const ACCOUNT_ID = generateAccountId('user@example.com', 'https://mail.example.com'); @@ -1118,6 +1123,35 @@ describe('resyncPushNotifications', () => { expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull(); }); + it('re-creates the subscription when this build first sees a server replaced before it ran (0.16.23 to 1.0)', async () => { + // Left by the released build, which recorded the expiry but no server + // fingerprint, against the 0.16 server; the fresh 1.0 install holds no + // subscriptions. The first check can't tell a replacement from none. + await AsyncStorage.setItem(SUB_KEY, 'existing'); + await AsyncStorage.setItem(EXPIRES_KEY, inDays(60)); + await AsyncStorage.setItem('push:accountIds:v1', JSON.stringify([ACCOUNT_ID])); + listMock.mockResolvedValue([]); + const session10 = { + apiUrl: 'https://mail.example.com/jmap/', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' }, + accounts: { + 'jmap-primary': { + name: 'user', isPersonal: true, isReadOnly: false, + accountCapabilities: { 'urn:ietf:params:jmap:mail': {}, 'urn:ietf:params:jmap:principals:owner': {} }, + }, + }, + capabilities: { 'urn:ietf:params:jmap:core': {}, 'urn:ietf:params:jmap:metadata': {} }, + }; + + expect(await reconcileServerFingerprint(ACCOUNT_ID, session10 as never)).toBe(false); + const result = await resyncPushNotifications({ relayBaseUrl: RELAY }); + + expect(result?.subscriptionId).toBe('new-server-id'); + expect(createMock).toHaveBeenCalledTimes(1); + expect(await readPushAccountIds()).toEqual([ACCOUNT_ID]); + expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull(); + }); + it('does not take over another device\'s subscription that reuses the recorded id', async () => { // Stalwart's ids are short and sequential: after a reinstall another // device's subscription can come back under the id this device recorded. diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index fec7f4c2..332c6426 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -1274,6 +1274,16 @@ export async function forgetPushServerState(accountId: string): Promise { ]); } +/** + * Forget the recorded subscription's expiry alone, for an account whose + * server may have been replaced unseen (lib/server-change): the next resync + * then re-creates a subscription the server lacks instead of taking it for + * revoked. + */ +export async function forgetPushSubscriptionExpiry(accountId: string): Promise { + await AsyncStorage.removeItem(subscriptionExpiresKey(accountId)); +} + /** * Tear down push for a single account. Destroys every JMAP subscription the * server holds for this device (assumes the jmapClient is currently @@ -1339,8 +1349,8 @@ export async function disablePushForAccount(accountId: string): Promise { * True when the server no longer has this account's recorded subscription * although it wasn't due to expire - someone revoked it. Without a known * expiry (a registration made by an older build, or one on a server that was - * replaced since) nothing can be told apart, so it counts as not revoked. - * Throws when the server can't be asked. + * replaced since, or may have been) nothing can be told apart, so it counts + * as not revoked. Throws when the server can't be asked. */ async function wasRevokedOnServer(accountId: string): Promise { const storedServerId = await AsyncStorage.getItem(subscriptionIdKey(accountId)); diff --git a/src/lib/server-change.ts b/src/lib/server-change.ts index d9e3bfee..a554e87e 100644 --- a/src/lib/server-change.ts +++ b/src/lib/server-change.ts @@ -12,7 +12,7 @@ import { toast } from '../stores/toast-store'; import { clearBodyHeights } from './body-heights'; import { clearBodyDocuments } from './email-body-document'; import { clearEmailDetailCache } from './email-detail-cache'; -import { forgetPushServerState } from './push-notifications'; +import { forgetPushServerState, forgetPushSubscriptionExpiry } from './push-notifications'; import { primaryAccountId, readServerFingerprint, @@ -97,22 +97,62 @@ export async function forgetServerDerivedState( // Get the emptied caches onto disk before the new fingerprint is. await step(() => flushPersistedWrites()); - if (droppedOps > 0) { - toast.warning(t( - 'offline.discarded_changes', - '{count, plural, one {# offline change was} other {# offline changes were}} discarded because the mail server changed', - { count: droppedOps }, - )); + announceDiscardedChanges(droppedOps); + if (failures.length > 0) throw failures[0]; +} + +function announceDiscardedChanges(count: number): void { + if (count <= 0) return; + toast.warning(t( + 'offline.discarded_changes', + '{count, plural, one {# offline change was} other {# offline changes were}} discarded because the mail server changed', + { count }, + )); +} + +/** + * The first sight of an account: no build before this one recorded its + * server, so nothing tells whether it was replaced before this one ran (an + * update of the app and a reinstall for Stalwart 1.0 can fall together). + * The caches stay, as they do while the server stays the same; dropping them + * would cost everyone who updates a full download. What would act on the + * server by itself goes: + * - the outbox. Its ops replay on their own, and on a replaced server an id + * names another message: a queued delete deletes that one. Nothing proves + * an op was made on this server: the queue records no server, a + * reinstalled Stalwart may accept the old JMAP states, and a queued move + * or delete took its message out of the cached list, so there is no copy + * to compare the server's message under that id with. Offline edits of + * whoever updates with some still unsent are lost, and announced. + * - the push subscription's recorded expiry. A replaced server holds no + * subscription, which the resync would take for a revocation and turn + * push off for good; without the expiry it re-creates it. + * Every step runs; the first failure is thrown afterwards. + */ +async function forgetUnconfirmedServerState(accountId: string): Promise { + const failures: unknown[] = []; + let droppedOps = 0; + try { + droppedOps = await useOutboxStore.getState().clearAccount(accountId); + } catch (err) { + failures.push(err); } + try { + await forgetPushSubscriptionExpiry(accountId); + } catch (err) { + failures.push(err); + } + announceDiscardedChanges(droppedOps); if (failures.length > 0) throw failures[0]; } /** * Compare the session with the fingerprint recorded for the local account * (username@host) and drop what was derived from the server when it changed. - * The first sight of an account only records it. The fingerprint is written - * after the caches are gone, so an interrupted drop is repeated next time. - * Resolves to true when caches were dropped. + * On the first sight of an account only what can't be confirmed to belong + * to its server goes (forgetUnconfirmedServerState). The fingerprint is + * written after that, so an interrupted drop is repeated next time. Resolves + * to true when caches were dropped. */ export async function reconcileServerFingerprint(accountId: string, session: JMAPSession): Promise { const next = sessionFingerprint(session); @@ -131,6 +171,8 @@ export async function reconcileServerFingerprint(accountId: string, session: JMA previous: await readServerPrimaryAccountId(accountId), next: primary || null, }); + } else { + await forgetUnconfirmedServerState(accountId); } await writeServerFingerprint(accountId, next, primary); return recorded !== null; diff --git a/src/stores/__tests__/auth-store-server-change.test.ts b/src/stores/__tests__/auth-store-server-change.test.ts index 2274dcf9..6b0a2e9a 100644 --- a/src/stores/__tests__/auth-store-server-change.test.ts +++ b/src/stores/__tests__/auth-store-server-change.test.ts @@ -78,9 +78,11 @@ vi.mock('../../lib/push-notifications', () => ({ teardownPushNotifications: vi.fn(async () => undefined), teardownPushNotificationsForAccount: vi.fn(async () => undefined), forgetPushServerState: vi.fn(async () => undefined), + forgetPushSubscriptionExpiry: vi.fn(async () => undefined), })); import AsyncStorage from '@react-native-async-storage/async-storage'; +import { forgetPushSubscriptionExpiry } from '../../lib/push-notifications'; import { useAuthStore } from '../auth-store'; import { useAccountStore } from '../account-store'; import { useEmailStore } from '../email-store'; @@ -190,4 +192,19 @@ describe('restoring the session on a replaced server', () => { expect(useOutboxStore.getState().entries.map((e) => e.op.emailId)).toEqual(['b']); expect(await AsyncStorage.getItem(OUTBOX_KEY)).not.toBeNull(); }); + + it('sends none of the queue the build before left, the first time this one sees the server (Stalwart 1.0)', async () => { + // No fingerprint: the released build never recorded one, and the server + // may have been replaced before this one ran. "b" can be another message. + await AsyncStorage.setItem(OUTBOX_KEY, JSON.stringify(QUEUED)); + + expect(await useAuthStore.getState().restoreSession()).toBe(true); + await new Promise((r) => setTimeout(r, 20)); + + expect(emailSets()).toEqual([]); + expect(useOutboxStore.getState().entries).toEqual([]); + expect(await AsyncStorage.getItem(OUTBOX_KEY)).toBeNull(); + expect(forgetPushSubscriptionExpiry).toHaveBeenCalledWith(ACCOUNT); + expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + }); }); diff --git a/src/stores/__tests__/calendar-subscriptions-store.test.ts b/src/stores/__tests__/calendar-subscriptions-store.test.ts index ba609fbd..7d827c1c 100644 --- a/src/stores/__tests__/calendar-subscriptions-store.test.ts +++ b/src/stores/__tests__/calendar-subscriptions-store.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('../../api/calendar', () => ({ createCalendar: vi.fn(), @@ -16,7 +16,7 @@ vi.mock('../../api/jmap-client', () => ({ jmapClient: { accountId: 'acc-1', isConnected: true, username: 'user', serverUrl: 'https://mail.example.org' }, })); const calendarStore = vi.hoisted(() => ({ - importEvents: vi.fn(async () => ({ created: 1, updated: 0 })), + importEvents: vi.fn(async (_events: unknown[], _calendarId: string) => ({ created: 1, updated: 0 })), fetchCalendars: vi.fn(async () => undefined), refresh: vi.fn(async () => undefined), })); @@ -32,6 +32,7 @@ vi.mock('@react-native-async-storage/async-storage', () => ({ })); import * as calendarApi from '../../api/calendar'; +import { useAccountStore } from '../account-store'; import { normalizeFeedUrl, selectAccountSubscriptions, @@ -185,8 +186,9 @@ describe('subscriptions of a server that was replaced (0.16.23 to 1.0)', () => { expect(useCalendarSubscriptionsStore.getState().subscriptions).toEqual([]); }); - it('drops no events from a calendar under the recorded id that carries another name', async () => { - // The reinstalled server gave "c" to the user's own calendar. + it('neither drops events from nor imports the feed into a calendar under the recorded id that carries another name', async () => { + // The reinstalled server gave "c" to the user's own calendar (a server + // replaced before this build first saw it, so nothing unlinked it). vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Personal'); vi.mocked(calendarApi.queryEvents).mockResolvedValue(['p1']); vi.mocked(calendarApi.getEventIdentities).mockResolvedValue([{ id: 'p1', uid: 'dentist', calendarIds: { c: true } }] as never); @@ -198,6 +200,21 @@ describe('subscriptions of a server that was replaced (0.16.23 to 1.0)', () => { expect(calendarApi.queryEvents).not.toHaveBeenCalled(); expect(calendarApi.deleteEvents).not.toHaveBeenCalled(); expect(calendarApi.updateEvent).not.toHaveBeenCalled(); + // The feed goes into a new mirror, as for an unlinked subscription. + expect(calendarApi.createCalendar).toHaveBeenCalledWith('Holidays', '#ff0000'); + expect(calendarStore.importEvents.mock.calls.map(([, calendarId]) => calendarId)).toEqual(['n1']); + expect(only()).toMatchObject({ calendarId: 'n1', accountId: 'acc-1', localAccountId: ME, lastError: null }); + }); + + it('touches nothing when it cannot tell whether the recorded calendar is the mirror', async () => { + vi.mocked(calendarApi.getCalendarName).mockRejectedValue(new Error('Network request failed')); + useCalendarSubscriptionsStore.setState({ subscriptions: [mine()] }); + + await useCalendarSubscriptionsStore.getState().syncSubscription('mine'); + + expect(calendarStore.importEvents).not.toHaveBeenCalled(); + expect(calendarApi.createCalendar).not.toHaveBeenCalled(); + expect(only()).toMatchObject({ calendarId: 'c', lastSyncAt: 5, lastError: 'Network request failed' }); }); it('still drops events that left the feed from its own mirror calendar', async () => { @@ -233,4 +250,113 @@ describe('subscriptions of a server that was replaced (0.16.23 to 1.0)', () => { expect(calendarApi.updateCalendar).not.toHaveBeenCalled(); expect(only().name).toBe('Public holidays'); }); + + it('keeps its name when the mirror could not be renamed, so it still prunes and removes its mirror', async () => { + vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Holidays'); + vi.mocked(calendarApi.updateCalendar).mockRejectedValueOnce(new Error('Network request failed')); + useCalendarSubscriptionsStore.setState({ subscriptions: [mine()] }); + + await expect(useCalendarSubscriptionsStore.getState().updateSubscription('mine', { name: 'Public holidays', color: '#00ff00' })) + .rejects.toThrow('Network request failed'); + expect(only()).toMatchObject({ name: 'Holidays', color: '#ff0000' }); + + // The mirror on the server is still called "Holidays". + vi.mocked(calendarApi.queryEvents).mockResolvedValue(['x1']); + vi.mocked(calendarApi.getEventIdentities).mockResolvedValue([{ id: 'x1', uid: 'gone', calendarIds: { c: true } }] as never); + await useCalendarSubscriptionsStore.getState().syncSubscription('mine'); + expect(calendarApi.deleteEvents).toHaveBeenCalledWith(['x1']); + expect(calendarApi.createCalendar).not.toHaveBeenCalled(); + await useCalendarSubscriptionsStore.getState().removeSubscription('mine'); + expect(calendarApi.deleteCalendar).toHaveBeenCalledWith('c'); + }); + + it('keeps its name when the mirror\'s name cannot be read', async () => { + vi.mocked(calendarApi.getCalendarName).mockRejectedValueOnce(new Error('Network request failed')); + useCalendarSubscriptionsStore.setState({ subscriptions: [mine()] }); + + await expect(useCalendarSubscriptionsStore.getState().updateSubscription('mine', { name: 'Public holidays' })) + .rejects.toThrow('Network request failed'); + + expect(calendarApi.updateCalendar).not.toHaveBeenCalled(); + expect(only().name).toBe('Holidays'); + }); +}); + +describe('subscriptions made before they recorded their account', () => { + // Accounts on two servers can get the same JMAP id for their own data: + // usera on one.example and userb on two.example are both "d" here. + const USERA = 'usera@one.example'; + const USERB = 'userb@two.example'; + const account = (id: string, jmapAccountId?: string) => ({ + id, serverUrl: `https://${id.split('@')[1]}`, username: id.split('@')[0], displayName: id, email: id, + avatarColor: '#000', lastLoginAt: 0, isConnected: true, hasError: false, isDefault: id === USERA, jmapAccountId, + }); + // userb's, made on two.example before subscriptions recorded their account. + const legacy: CalendarSubscription = { + id: 'sports', name: 'Sports', url: 'https://feeds.example/s.ics', calendarId: 'k', accountId: 'd', lastSyncAt: 5, lastError: null, + }; + const only = () => useCalendarSubscriptionsStore.getState().subscriptions[0]; + + beforeEach(() => { + vi.clearAllMocks(); + useCalendarSubscriptionsStore.setState({ syncing: {}, subscriptions: [{ ...legacy }] }); + globalThis.fetch = vi.fn(async () => ({ + ok: true, status: 200, headers: { get: () => null }, text: async () => 'BEGIN:VCALENDAR\r\nEND:VCALENDAR\r\n', + })) as unknown as typeof fetch; + vi.mocked(calendarApi.parseCalendarBlob).mockResolvedValue([{ uid: 'e1', title: 'Final', start: '2027-01-01T00:00:00' }]); + vi.mocked(calendarApi.queryEvents).mockResolvedValue([]); + vi.mocked(calendarApi.getEventIdentities).mockResolvedValue([]); + vi.mocked(calendarApi.createCalendar).mockResolvedValue({ id: 'n1', name: 'Sports' } as never); + }); + + afterEach(() => { + useAccountStore.setState({ accounts: [] }); + }); + + it('are not taken by a replaced server\'s account when another account has the same JMAP id', async () => { + useAccountStore.setState({ accounts: [account(USERA, 'd'), account(USERB, 'd')] }); + + await useCalendarSubscriptionsStore.getState().unlinkAccount(USERA, { previous: 'd', next: 'd' }); + + expect(only()).toEqual(legacy); + }); + + it('are not taken while another account\'s JMAP id is not known yet', async () => { + useAccountStore.setState({ accounts: [account(USERA, 'd'), account(USERB)] }); + + await useCalendarSubscriptionsStore.getState().unlinkAccount(USERA, { previous: 'd', next: 'd' }); + + expect(only()).toEqual(legacy); + }); + + it('go with the replaced server\'s account when no other account has their JMAP id', async () => { + useAccountStore.setState({ accounts: [account(USERA, 'd'), account(USERB, 'e')] }); + + await useCalendarSubscriptionsStore.getState().unlinkAccount(USERA, { previous: 'd', next: 'h' }); + + expect(only()).toMatchObject({ calendarId: null, localAccountId: USERA, accountId: 'h', lastSyncAt: null }); + }); + + it('become the account\'s once a sync finds their mirror in it', async () => { + // The client serves user@mail.example.org, JMAP id "acc-1". + vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Sports'); + useCalendarSubscriptionsStore.setState({ subscriptions: [{ ...legacy, accountId: 'acc-1' }] }); + + await useCalendarSubscriptionsStore.getState().syncSubscription('sports'); + + expect(only()).toMatchObject({ calendarId: 'k', accountId: 'acc-1', localAccountId: 'user@mail.example.org', lastError: null }); + expect(calendarStore.importEvents).toHaveBeenCalledWith([expect.objectContaining({ uid: 'e1' })], 'k'); + }); + + it('are left alone by a sync that does not find their mirror when another account may own them', async () => { + useAccountStore.setState({ accounts: [account('user@mail.example.org', 'acc-1'), account(USERB, 'acc-1')] }); + vi.mocked(calendarApi.getCalendarName).mockResolvedValue('Personal'); + useCalendarSubscriptionsStore.setState({ subscriptions: [{ ...legacy, accountId: 'acc-1' }] }); + + await useCalendarSubscriptionsStore.getState().syncSubscription('sports'); + + expect(calendarApi.createCalendar).not.toHaveBeenCalled(); + expect(calendarStore.importEvents).not.toHaveBeenCalled(); + expect(only()).toEqual({ ...legacy, accountId: 'acc-1' }); + }); }); diff --git a/src/stores/__tests__/server-change.test.ts b/src/stores/__tests__/server-change.test.ts index 34e2b4c2..cbc85791 100644 --- a/src/stores/__tests__/server-change.test.ts +++ b/src/stores/__tests__/server-change.test.ts @@ -302,15 +302,75 @@ describe('reconcileServerFingerprint', () => { expect(useToastStore.getState().toasts).toEqual([]); }); - it('only records the first server it sees for an account', async () => { - await seed(ACCOUNT); - await AsyncStorage.removeItem(`server-fingerprint:v1:${ACCOUNT}`); + describe('on the first sight of an account (no build before recorded its server)', () => { + // The update may have come after the server was replaced (0.16.23 to 1.0). + const EXPIRES_KEY = `push:subscriptionExpires:v1:${ACCOUNT}`; + const firstSight = async () => { + await seed(ACCOUNT); + await AsyncStorage.removeItem(`server-fingerprint:v1:${ACCOUNT}`); + // Recorded by the released build against the server it knew. + await AsyncStorage.setItem(EXPIRES_KEY, '2026-12-01T00:00:00.000Z'); + }; - expect(await reconcileServerFingerprint(ACCOUNT, session('1.0'))).toBe(false); + it('keeps the caches, and drops the queued changes with a notice and the push expiry (Stalwart 1.0)', async () => { + await firstSight(); - expect(useEmailStore.getState().mailboxState).toBe('s1iaq'); - expect(useOutboxStore.getState().entries).toHaveLength(2); - expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + expect(await reconcileServerFingerprint(ACCOUNT, session('1.0'))).toBe(false); + + // What the app only shows stays. + expect(useEmailStore.getState().mailboxState).toBe('s1iaq'); + expect(useCalendarSubscriptionsStore.getState().subscriptions).toEqual(SUBSCRIPTIONS); + expect(await AsyncStorage.getItem(`push:notifiedIds:v1:${ACCOUNT}`)).not.toBeNull(); + // What would act on the server by itself goes: queued and failed ops... + expect(useOutboxStore.getState().entries).toEqual([]); + expect(useOutboxStore.getState().failed).toEqual([]); + const keys = await storedKeys(); + expect(keys).not.toContain(`webmail:outbox:v1:${ACCOUNT}`); + expect(keys).not.toContain(`webmail:outbox:v1:${ACCOUNT}:failed`); + expect(useToastStore.getState().toasts.map((toast) => toast.title)).toEqual([ + '3 offline changes were discarded because the mail server changed', + ]); + // ... and the expiry that would take a missing subscription for revoked. + expect(await AsyncStorage.getItem(EXPIRES_KEY)).toBeNull(); + expect(await AsyncStorage.getItem(`push:subscriptionId:v2:${ACCOUNT}`)).toBe('a'); + expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + // Another account's queue waits for its own first sight. + expect(JSON.parse((await AsyncStorage.getItem(`webmail:outbox:v1:${OTHER}`))!)).toHaveLength(2); + }); + + it('says nothing when no changes were queued (Stalwart 0.16.23)', async () => { + await firstSight(); + await useOutboxStore.getState().clearAccount(ACCOUNT); + + await reconcileServerFingerprint(ACCOUNT, session('0.16.23')); + + expect(useToastStore.getState().toasts).toEqual([]); + expect(await AsyncStorage.getItem(EXPIRES_KEY)).toBeNull(); + }); + + it('does it again with the next session when it failed part-way', async () => { + await firstSight(); + const removeItem = vi.mocked(AsyncStorage.removeItem); + const remove = removeItem.getMockImplementation()!; + removeItem.mockImplementation(async (key: string) => { + if (key === EXPIRES_KEY) throw new Error('disk full'); + return remove(key); + }); + + try { + await expect(reconcileServerFingerprint(ACCOUNT, session('1.0'))).rejects.toThrow('disk full'); + } finally { + removeItem.mockImplementation(remove); + } + expect(useOutboxStore.getState().entries).toEqual([]); + expect(await readServerFingerprint(ACCOUNT)).toBeNull(); + + expect(await reconcileServerFingerprint(ACCOUNT, session('1.0'))).toBe(false); + expect(await AsyncStorage.getItem(EXPIRES_KEY)).toBeNull(); + expect(await readServerFingerprint(ACCOUNT)).toBe(sessionFingerprint(session('1.0'))); + // Announced once. + expect(useToastStore.getState().toasts).toHaveLength(1); + }); }); it('tries again with the next session when dropping failed part-way', async () => { diff --git a/src/stores/calendar-subscriptions-store.ts b/src/stores/calendar-subscriptions-store.ts index b1db69a7..113dffb9 100644 --- a/src/stores/calendar-subscriptions-store.ts +++ b/src/stores/calendar-subscriptions-store.ts @@ -16,6 +16,8 @@ import { import { jmapClient } from '../api/jmap-client'; import { uploadBytes } from '../api/blob'; import { generateAccountId } from '../lib/account-utils'; +import { readServerPrimaryAccountId } from '../lib/server-fingerprint'; +import { useAccountStore } from './account-store'; import { useCalendarStore } from './calendar-store'; import { t } from './locale-store'; @@ -34,7 +36,8 @@ export interface CalendarSubscription { accountId?: string; /** * The signed-in account (username@host) it belongs to: JMAP account ids - * repeat across servers. Unset on subscriptions made before it was kept. + * repeat across servers. Unset on subscriptions made before it was kept, + * until a sync finds their mirror calendar in an account. */ localAccountId?: string; /** Minutes between automatic refreshes (default 60). */ @@ -72,8 +75,9 @@ interface SubscriptionsState { * its calendar ids name other calendars now, or none. The account's * subscriptions keep their feed, name, colour and interval, lose their * mirror calendar and get a new one on the next sync. Subscriptions made - * before the account was recorded on them are matched by the JMAP account - * ids the old and the new server gave it. + * before the account was recorded on them go with it when they name a JMAP + * account id the old or the new server gave it, and no other signed-in + * account has that id. */ unlinkAccount: (localAccountId: string, jmapAccountIds: { previous: string | null; next: string | null }) => Promise; } @@ -108,6 +112,7 @@ export function currentLocalAccountId(): string | null { // The mirror calendar is recognised by the name the subscription gave it. // A calendar under the recorded id with another name is not the mirror: the // id may be reused by a replaced server, or the user renamed the calendar. +// False when the server can't be asked. async function isMirrorCalendar(calendarId: string, name: string): Promise { try { return (await getCalendarName(calendarId)) === name; @@ -116,6 +121,29 @@ async function isMirrorCalendar(calendarId: string, name: string): Promise, +): Promise { + if (sub.accountId && !jmapAccountIds.includes(sub.accountId)) return false; + for (const other of useAccountStore.getState().accounts) { + if (other.id === localAccountId) continue; + const otherJmapId = (await readServerPrimaryAccountId(other.id)) ?? other.jmapAccountId ?? null; + // Not known yet (not signed in to since the id was first recorded). + if (otherJmapId === null) return false; + if (!sub.accountId || otherJmapId === sub.accountId) return false; + } + return true; +} + // webcal:// and webcals:// are just iCalendar over HTTP(S) — clients map // them to https. Credentials embedded in the URL (#275) are moved into an // Authorization header because RN's WHATWG fetch rejects userinfo in URLs. @@ -164,17 +192,18 @@ async function fetchAndParseFeed(url: string): Promise[]> return events.filter((e) => !!e.start); } -// Reconcile the local calendar with the remote feed: drop events whose UID +// Reconcile the mirror calendar with the remote feed: drop events whose UID // disappeared upstream (unlinking rather than deleting when the event also // lives in another calendar), then import the current set (importEvents -// dedupes by UID, so unchanged events are left in place). Nothing is dropped -// from a calendar that doesn't carry the subscription's name. -async function syncFeedIntoCalendar(calendarId: string, url: string, name: string): Promise { +// dedupes by UID, so unchanged events are left in place). Callers make sure +// `calendarId` is the mirror: nothing goes into, nor is dropped from, another +// calendar. One made for this sync has nothing to drop. +async function syncFeedIntoCalendar(calendarId: string, url: string, dropStale: boolean): Promise { const parsed = await fetchAndParseFeed(url); - const parsedUids = new Set(parsed.map((e) => e.uid).filter(Boolean) as string[]); - try { - if (await isMirrorCalendar(calendarId, name)) { + if (dropStale) { + const parsedUids = new Set(parsed.map((e) => e.uid).filter(Boolean) as string[]); + try { const ids = await queryEvents([calendarId], '', ''); const existing = await getEventIdentities(ids); const stale = existing.filter((e) => e.uid && !parsedUids.has(e.uid)); @@ -186,9 +215,9 @@ async function syncFeedIntoCalendar(calendarId: string, url: string, name: strin else await updateEvent(e.id, { calendarIds: others }); } if (toDelete.length > 0) await deleteEvents(toDelete); + } catch { + // Non-fatal: if we can't enumerate existing events, still import new ones. } - } catch { - // Non-fatal: if we can't enumerate existing events, still import new ones. } await useCalendarStore.getState().importEvents(parsed, calendarId); @@ -219,7 +248,7 @@ export const useCalendarSubscriptionsStore = create()( await useCalendarStore.getState().fetchCalendars(); try { set({ syncing: { ...get().syncing, [sub.id]: true } }); - await syncFeedIntoCalendar(calendar.id, sub.url, name); + await syncFeedIntoCalendar(calendar.id, sub.url, false); set({ subscriptions: [...get().subscriptions, { ...sub, lastSyncAt: Date.now() }] }); await useCalendarStore.getState().refresh(); return { ...sub, lastSyncAt: Date.now() }; @@ -240,8 +269,7 @@ export const useCalendarSubscriptionsStore = create()( updateSubscription: async (id, updates) => { const sub = get().subscriptions.find((s) => s.id === id); if (!sub) return; - const next: CalendarSubscription = { - ...sub, + const changes: Partial = { ...(updates.name !== undefined ? { name: updates.name.trim() } : {}), ...(updates.url !== undefined ? { url: updates.url.trim() } : {}), ...(updates.color !== undefined ? { color: updates.color } : {}), @@ -249,26 +277,24 @@ export const useCalendarSubscriptionsStore = create()( ? { refreshIntervalMinutes: updates.refreshIntervalMinutes } : {}), }; + const next: CalendarSubscription = { ...sub, ...changes }; // An unlinked subscription has no calendar to rename yet; its next - // sync makes one with the new name and colour. - if ( - sub.calendarId - && (next.name !== sub.name || next.color !== sub.color) - && (await isMirrorCalendar(sub.calendarId, sub.name)) - ) { - try { + // sync makes one with the new name and colour. A mirror is renamed + // first, and when that fails (or its name can't be read) nothing + // changes: a sync recognises the mirror by the subscription's name, + // and would take the old-named calendar for another one. + if (sub.calendarId && (next.name !== sub.name || next.color !== sub.color)) { + if ((await getCalendarName(sub.calendarId)) === sub.name) { // Only what changed: Stalwart 1.0 refuses a colour that is not // #rgb, #rrggbb or a CSS name, which another client may have set. await updateCalendar(sub.calendarId, { ...(next.name !== sub.name ? { name: next.name } : {}), ...(next.color !== sub.color ? { color: next.color } : {}), }); - await useCalendarStore.getState().fetchCalendars(); - } catch { - // The mirror calendar keeps its old name/colour; the sub still updates. + void useCalendarStore.getState().fetchCalendars().catch(() => undefined); } } - set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? next : s)) }); + set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? { ...s, ...changes } : s)) }); if (next.url !== sub.url) await get().syncSubscription(id); }, @@ -292,27 +318,49 @@ export const useCalendarSubscriptionsStore = create()( const sub = get().subscriptions.find((s) => s.id === id); if (!sub) return; if (get().syncing[id]) return; - // An unlinked subscription gets its new mirror in its own account only. + // Only its own account syncs a subscription, and an unlinked one gets + // its new mirror in its own account only. const localAccountId = currentLocalAccountId(); - if (!sub.calendarId && (!localAccountId || (sub.localAccountId && sub.localAccountId !== localAccountId))) return; + if (sub.localAccountId && sub.localAccountId !== localAccountId) return; + if (!sub.calendarId && !localAccountId) return; set({ syncing: { ...get().syncing, [id]: true } }); try { + const jmapAccountId = currentAccountId(); let calendarId = sub.calendarId; + // Throws when the server can't be asked; the next sync tries again. + if (calendarId && (await getCalendarName(calendarId)) !== sub.name) { + // The recorded id names another calendar, or none: the server was + // replaced unseen, or the mirror was renamed or removed elsewhere. + // The feed goes into its own mirror only: a new one, made as for + // an unlinked subscription. One made before subscriptions recorded + // their account may be another account's, with the same JMAP id. + if (!sub.localAccountId + && !(localAccountId && (await ownsLegacySubscription(sub, localAccountId, [jmapAccountId])))) { + return; + } + calendarId = null; + } else if (calendarId && !sub.localAccountId && localAccountId) { + // Its mirror is in this account, so it is this account's alone + // from now on, not every account's with the same JMAP id. + const owner = { localAccountId, accountId: sub.accountId ?? jmapAccountId ?? undefined }; + set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? { ...s, ...owner } : s)) }); + } if (!calendarId) { // As addSubscription does. Linked right away, so a feed that // fails below doesn't get another calendar on the next sync. calendarId = (await createCalendar(sub.name, sub.color)).id; - const linked = { calendarId, accountId: currentAccountId() ?? undefined, localAccountId: localAccountId ?? undefined }; + const linked = { calendarId, accountId: jmapAccountId ?? undefined, localAccountId: localAccountId ?? undefined }; set({ subscriptions: get().subscriptions.map((s) => (s.id === id ? { ...s, ...linked } : s)) }); await useCalendarStore.getState().fetchCalendars(); } - await syncFeedIntoCalendar(calendarId, sub.url, sub.name); + const linkedBefore = calendarId === sub.calendarId; + await syncFeedIntoCalendar(calendarId, sub.url, linkedBefore); set({ subscriptions: get().subscriptions.map((s) => s.id === id ? { ...s, lastSyncAt: Date.now(), lastError: null } : s, ), }); - if (!sub.calendarId) await useCalendarStore.getState().refresh(); + if (!linkedBefore) await useCalendarStore.getState().refresh(); } catch (err) { const message = err instanceof Error ? err.message : t('calendar.subscription.refresh_error', 'Failed to refresh subscription'); set({ @@ -348,11 +396,15 @@ export const useCalendarSubscriptionsStore = create()( if (!useCalendarSubscriptionsStore.persist.hasHydrated()) { await useCalendarSubscriptionsStore.persist.rehydrate(); } - const owned = (s: CalendarSubscription) => (s.localAccountId - ? s.localAccountId === localAccountId - : !s.accountId || s.accountId === previous || s.accountId === next); + const owned = new Set(); + for (const s of get().subscriptions) { + const mine = s.localAccountId + ? s.localAccountId === localAccountId + : await ownsLegacySubscription(s, localAccountId, [previous, next]); + if (mine) owned.add(s.id); + } set({ - subscriptions: get().subscriptions.map((s) => (owned(s) + subscriptions: get().subscriptions.map((s) => (owned.has(s.id) ? { ...s, calendarId: null,