diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index 479cde68..2b2e4edd 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -66,6 +66,10 @@ const { DETACHED, SYNC_SESSION } = vi.hoisted(() => { // The subscription's expiry, and what an update of it settles on. expires: null as string | null, settles: null as string | null, + // Every subscription of the account, for a listing with ids: null, and + // whether that listing fails. + all: [] as Array<{ id: string; deviceClientId?: string }>, + listFails: false, log: [] as unknown[], session: SYNC_SESSION, }, @@ -96,6 +100,11 @@ vi.mock('../../api/jmap-client', () => ({ async request(calls: Array<[string, Record, string]>) { const methodResponses = calls.map(([name, args, id]) => { DETACHED.log.push([name, args]); + if (name === 'PushSubscription/get' && args.ids === null) { + if (DETACHED.listFails) throw new Error('network down'); + return [name, { list: DETACHED.all }, id]; + } + if (args.destroy) return [name, { destroyed: args.destroy }, id]; if (name === 'PushSubscription/get') { const listed = { id: args.ids[0], @@ -1854,6 +1863,63 @@ describe('teardownPushNotificationsForAccount', () => { expect(destroyMock.mock.calls.map((c) => c[0])).toEqual(['recorded']); expect(await AsyncStorage.getItem(SUB_KEY)).toBeNull(); }); + + describe('for a signed-in account the client does not serve', () => { + const OTHER = 'bob@other.example.net'; + const OTHER_DCID = 'bobsphonebobsphonebobsphonebobsp'; + const destroyedThroughOwnClient = () => DETACHED.log + .filter((entry) => (entry as unknown[])[0] === 'PushSubscription/set') + .map((entry) => ((entry as [string, { destroy: string[] }])[1]).destroy); + + beforeEach(async () => { + await AsyncStorage.setItem('push:relayBaseUrl:v1', RELAY); + await AsyncStorage.setItem(`push:subscriptionId:v2:${OTHER}`, 'b'); + await AsyncStorage.setItem(deviceClientIdKey(OTHER), OTHER_DCID); + // The open account's own push, with the same short id. + listMock.mockResolvedValue([{ id: 'b', types: ['EmailDelivery'] }]); + Object.assign(DETACHED, { + loaded: true, + all: [{ id: 'b', deviceClientId: OTHER_DCID }, { id: 'c', deviceClientId: 'ffffffffffffffffffffffffffffffff' }], + listFails: false, + log: [], + }); + }); + + afterEach(() => { + listMock.mockImplementation(async () => []); + Object.assign(DETACHED, { loaded: true, all: [], listFails: false }); + }); + + it('destroys its subscriptions through a client of its own', async () => { + await teardownPushNotificationsForAccount(OTHER); + + expect(DETACHED.log[0]).toEqual(['load', OTHER]); + expect(destroyedThroughOwnClient()).toEqual([['b']]); + expect(listMock).not.toHaveBeenCalled(); + expect(destroyMock).not.toHaveBeenCalled(); + expect(await AsyncStorage.getItem(`push:subscriptionId:v2:${OTHER}`)).toBeNull(); + }); + + it("never destroys the recorded id on the open account when its own server can't list", async () => { + listMock.mockRejectedValue(new Error('network down')); + DETACHED.listFails = true; + + await teardownPushNotificationsForAccount(OTHER); + + expect(destroyMock).not.toHaveBeenCalled(); + expect(destroyedThroughOwnClient()).toEqual([['b']]); + + // An account that can't be loaded leaves every server alone. + DETACHED.log = []; + DETACHED.loaded = false; + await AsyncStorage.setItem(`push:subscriptionId:v2:${OTHER}`, 'b'); + await AsyncStorage.setItem(deviceClientIdKey(OTHER), OTHER_DCID); + await teardownPushNotificationsForAccount(OTHER); + expect(destroyMock).not.toHaveBeenCalled(); + expect(destroyedThroughOwnClient()).toEqual([]); + expect(await AsyncStorage.getItem(`push:subscriptionId:v2:${OTHER}`)).toBeNull(); + }); + }); }); describe('notificationTapJmapAccountId', () => { diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index a4d4c733..16e9148a 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -1682,10 +1682,13 @@ export async function recordPushSubscriptionExpires(accountId: string, expires: /** * Tear down push for a single account. Destroys every JMAP subscription the - * server holds for this device (assumes the jmapClient is currently - * authenticated to that account; the active-account logout flow guarantees - * this) and tells the relay to drop its mapping. Other accounts' push setups - * are untouched. + * server holds for this device and tells the relay to drop its mapping. + * Other accounts' push setups are untouched. The account the jmapClient + * serves goes through it; another signed-in account (removed from the + * account list while a different one is open) through a client of its own, + * never the singleton: subscription ids repeat across accounts ("b" in every + * Stalwart account), so a destroy by id there could end the open account's + * own push. * * The FCM token is deliberately left alive: the relay mapping is gone so * nothing gets forwarded, and deleting the token makes the next getToken() @@ -1706,8 +1709,9 @@ export async function teardownPushNotificationsForAccount( // failed enable can leave a registration this client no longer tracks (#841). // The recorded id alone is trusted only when the server can't be asked: on // a server reinstalled at the same address it can name another device's. + const server = await pushSubscriptionsOf(accountId); const idsToDestroy = new Set(); - const existing = storedDcid ? await listPushSubscriptions().catch(() => null) : null; + const existing = server && storedDcid ? await server.list().catch(() => null) : null; if (existing) { for (const s of existing) { const recordedWithoutDevice = s.id === storedSubId && !s.deviceClientId; @@ -1717,7 +1721,7 @@ export async function teardownPushNotificationsForAccount( idsToDestroy.add(storedSubId); } for (const id of idsToDestroy) { - await destroyPushSubscription(id).catch(() => undefined); + await server?.destroy(id).catch(() => undefined); } if (relayBaseUrl && storedDcid) { await deregisterFromRelay(relayBaseUrl, storedDcid); @@ -1729,6 +1733,38 @@ export async function teardownPushNotificationsForAccount( await writePushAccountIds(remaining); } +/** + * Listing and destroying the account's push subscriptions, through the + * jmapClient when it serves the account, else through a client of its own. + * Null when the account can't be loaded: its server is left alone then. + */ +async function pushSubscriptionsOf(accountId: string): Promise<{ + list: () => Promise; + destroy: (id: string) => Promise; +} | null> { + const username = jmapClient.username; + const serverUrl = jmapClient.serverUrl; + if (username && serverUrl && generateAccountId(username, serverUrl) === accountId) { + return { list: listPushSubscriptions, destroy: destroyPushSubscription }; + } + const client = new JMAPClient(); + if (!(await client.loadAccount(accountId).catch(() => false))) return null; + const using = [CAPABILITIES.CORE]; + return { + list: async () => { + const res = await client.request( + [['PushSubscription/get', { ids: null, properties: ['id', 'deviceClientId'] }, '0']], + using, + ); + return (requireMethodResult(res, '0', 'PushSubscription/get').list as ListedSubscription[] | undefined) ?? []; + }, + destroy: async (id) => { + const res = await client.request([['PushSubscription/set', { destroy: [id] }, '0']], using); + requireMethodResult(res, '0', 'PushSubscription/set'); + }, + }; +} + /** * Turn push off for one account because the user asked to: the settings * toggle, or revoking this device (here, or from another device - see