From b64afb462fe2086c8b8521e6fdeab35a345ba43a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 17 Jun 2026 09:07:42 +0200 Subject: [PATCH] fix: reap dead push subscriptions from other devices via relay liveness check --- src/lib/__tests__/push-notifications.test.ts | 131 +++++++++++++++++++ src/lib/push-notifications.ts | 63 +++++++-- 2 files changed, 181 insertions(+), 13 deletions(-) create mode 100644 src/lib/__tests__/push-notifications.test.ts diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts new file mode 100644 index 00000000..2435e7bb --- /dev/null +++ b/src/lib/__tests__/push-notifications.test.ts @@ -0,0 +1,131 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// Provide the Android native FCM surface setupPushNotifications needs. The +// global test-setup mocks react-native with an empty NativeModules, so override +// it here with a BulwarkFcm module and a pre-33 Platform.Version (which skips +// the runtime permission request). +vi.mock('react-native', () => { + class NativeEventEmitter { + addListener() { + return { remove: () => undefined }; + } + } + return { + Platform: { OS: 'android', Version: 30, select: (s: { default?: T; android?: T }) => s.android ?? s.default }, + NativeModules: { + BulwarkFcm: { + getToken: vi.fn(async () => 'fcm-token-xyz'), + deleteToken: vi.fn(async () => undefined), + }, + }, + NativeEventEmitter, + PermissionsAndroid: { RESULTS: { GRANTED: 'granted' }, request: vi.fn(async () => 'granted') }, + }; +}); + +vi.mock('../../api/jmap-client', () => ({ + jmapClient: { username: 'user@example.com', serverUrl: 'https://mail.example.com' }, +})); + +vi.mock('../../api/push', () => ({ + listPushSubscriptions: vi.fn(async () => []), + createPushSubscription: vi.fn(async () => 'new-server-id'), + verifyPushSubscription: vi.fn(async () => undefined), + destroyPushSubscription: vi.fn(async () => undefined), + updatePushSubscription: vi.fn(async () => true), +})); + +import AsyncStorage from '@react-native-async-storage/async-storage'; +import { setupPushNotifications, deviceClientIdKey } from '../push-notifications'; +import { + listPushSubscriptions, + destroyPushSubscription, +} from '../../api/push'; +import { generateAccountId } from '../account-utils'; + +const OUR_DCID = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; +const ACCOUNT_ID = generateAccountId('user@example.com', 'https://mail.example.com'); +const RELAY = 'https://relay.example.com'; + +// State the fake relay reports for each foreign deviceClientId's /active probe. +type RelayState = 'dead' | 'live' | 'unknown'; + +function installFetch(states: Record): void { + global.fetch = vi.fn(async (input: RequestInfo | URL) => { + const url = typeof input === 'string' ? input : input.toString(); + if (url.includes('/api/push/register')) { + return { ok: true, status: 200, json: async () => ({ ok: true }) } as Response; + } + if (url.includes('/api/push/verify/')) { + return { ok: true, status: 200, json: async () => ({ verificationCode: 'CODE' }) } as Response; + } + const active = url.match(/\/api\/push\/active\/([^/?]+)$/); + if (active) { + const dcid = decodeURIComponent(active[1]); + const state = states[dcid] ?? 'unknown'; + if (state === 'unknown') { + return { ok: false, status: 404, json: async () => ({ error: 'Unknown subscription' }) } as Response; + } + return { ok: true, status: 200, json: async () => ({ active: state === 'live' }) } as Response; + } + throw new Error(`unexpected fetch: ${url}`); + }) as typeof fetch; +} + +const destroyMock = destroyPushSubscription as ReturnType; +const listMock = listPushSubscriptions as ReturnType; + +function sub(id: string, deviceClientId: string) { + return { id, deviceClientId, expires: new Date(Date.now() + 86400000).toISOString(), types: ['Email'] }; +} + +describe('setupPushNotifications leftover reaping', () => { + beforeEach(async () => { + vi.clearAllMocks(); + await AsyncStorage.clear(); + // Pin our deviceClientId so we control which leftovers are "ours". + await AsyncStorage.setItem(deviceClientIdKey(ACCOUNT_ID), OUR_DCID); + }); + + it('reaps our own and relay-confirmed-dead leftovers, keeps live and unverifiable ones', async () => { + listMock.mockResolvedValue([ + sub('own-old', OUR_DCID), // our own previous attempt -> reap + sub('foreign-dead', 'deaddeaddeaddeaddeaddeaddeaddead'), // relay: dead -> reap + sub('foreign-live', 'livelivelivelivelivelivelivelive'), // relay: live -> keep + sub('foreign-unknown', 'unknwunknwunknwunknwunknwunknwun'), // relay: 404 -> keep + ]); + installFetch({ + deaddeaddeaddeaddeaddeaddeaddead: 'dead', + livelivelivelivelivelivelivelive: 'live', + unknwunknwunknwunknwunknwunknwun: 'unknown', + }); + + const result = await setupPushNotifications({ relayBaseUrl: RELAY }); + + expect(result.verified).toBe(true); + const reaped = destroyMock.mock.calls.map((c) => c[0]); + expect(reaped).toContain('own-old'); + expect(reaped).toContain('foreign-dead'); + expect(reaped).not.toContain('foreign-live'); + expect(reaped).not.toContain('foreign-unknown'); + }); + + it('keeps foreign subs when the relay probe fails (network error)', async () => { + listMock.mockResolvedValue([sub('foreign-x', 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx')]); + global.fetch = vi.fn(async (input: RequestInfo | URL) => { + const url = typeof input === 'string' ? input : input.toString(); + if (url.includes('/api/push/register')) { + return { ok: true, status: 200, json: async () => ({ ok: true }) } as Response; + } + if (url.includes('/api/push/verify/')) { + return { ok: true, status: 200, json: async () => ({ verificationCode: 'CODE' }) } as Response; + } + if (url.includes('/api/push/active/')) throw new Error('network down'); + throw new Error(`unexpected fetch: ${url}`); + }) as typeof fetch; + + await setupPushNotifications({ relayBaseUrl: RELAY }); + + expect(destroyMock.mock.calls.map((c) => c[0])).not.toContain('foreign-x'); + }); +}); diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index 8c3f67b5..eaafec23 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -217,6 +217,33 @@ async function registerWithRelay(params: { } } +/** + * Ask the relay whether a leftover subscription is dead. Returns true ONLY when + * the relay positively reports it inactive - a record it knows about that has + * never forwarded a push and isn't freshly registered. Every other outcome (the + * relay doesn't recognise the id, an older relay without this endpoint, a + * network blip, or a live subscription) returns false, so we never reap + * anything we can't confirm is dead. This is what lets setup clear its own + * abandoned attempts - and dead siblings left by reinstalls that regenerated + * the deviceClientId - without disturbing another live device or the PWA that + * shares the account. + */ +async function relayReportsDead( + relayBaseUrl: string, + subscriptionId: string, +): Promise { + try { + const res = await fetch( + buildRelayUrl(relayBaseUrl, `/api/push/active/${encodeURIComponent(subscriptionId)}`), + ); + if (!res.ok) return false; + const body = (await res.json()) as { active?: unknown }; + return body.active === false; + } catch { + return false; + } +} + async function pollVerificationCode( relayBaseUrl: string, subscriptionId: string, @@ -306,19 +333,29 @@ export async function setupPushNotifications( await AsyncStorage.removeItem(subKey); } - // Reap any leftover Stalwart subscriptions still bound to this account's - // deviceClientId. These pile up when a previous enable attempt failed - // mid-flow (verify race, network blip, app killed). Stalwart per-account - // rate-limits PushVerification posts, so leaving stragglers around blocks - // the new one's verification - the symptom is a confusing "code does not - // match". Only reap matches for THIS account's deviceClientId; other - // accounts' subscriptions on this server (rare but possible if the same - // JMAP server backs multiple accounts) must be left alone. - const stragglers = existingSubs.filter( - (s) => s.deviceClientId === deviceClientId && s.id !== storedServerId, - ); - for (const s of stragglers) { - await destroyPushSubscription(s.id).catch(() => undefined); + // Reap leftover Stalwart subscriptions that would otherwise starve the new + // one's verification. Stalwart emits only one PushVerification per account + // per ~60s and picks the oldest unverified subscription, so a single stale + // straggler blocks every fresh attempt - the symptom is the confusing + // "Timed out waiting for PushVerification" error. We can't read a + // subscription's verified state or URL over JMAP (Stalwart hides both), only + // its deviceClientId, so we decide what's safe to remove like this: + // - same deviceClientId as ours: a previous attempt from THIS device, + // always safe to reap. + // - a different deviceClientId: could be another live device or the PWA on + // this account. Ask the relay whether it's still alive and only reap the + // ones it confirms are dead. Anything live - or anything the relay can't + // vouch for (a different relay, a non-Bulwark client, a network blip) - + // is left untouched. + for (const s of existingSubs) { + if (s.id === storedServerId) continue; + if (s.deviceClientId === deviceClientId) { + await destroyPushSubscription(s.id).catch(() => undefined); + continue; + } + if (await relayReportsDead(relayBaseUrl, s.deviceClientId)) { + await destroyPushSubscription(s.id).catch(() => undefined); + } } const serverAssignedId = await createPushSubscription({