From f0617a61b19c4268adca9121981d113de87dba58 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:08:41 +0100 Subject: [PATCH] fix: only treat a server subscription as this browser's when its device id matches --- lib/__tests__/web-push.test.ts | 40 ++++++++++++++++++++++++++++++++++ lib/web-push.ts | 9 +++++--- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts index dbb2fb478..7a44d1daa 100644 --- a/lib/__tests__/web-push.test.ts +++ b/lib/__tests__/web-push.test.ts @@ -815,6 +815,46 @@ describe('an endpoint the push service gave up on', () => { }); }); +describe('a subscription id the server gave to another device', () => { + // Stalwart hands out the lowest free id: this browser's subscription expired, + // and the phone that registered next got the same id. + it('creates its own instead of taking over the other device\'s', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'b'); + const client = makeClient([sub('b', OTHER_DEVICE)]); + installFetch({ [OTHER_DEVICE]: true }); + + await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }); + + expect(client.updatePushSubscription).not.toHaveBeenCalled(); + expect(client.createPushSubscription).toHaveBeenCalledTimes(1); + expect(client.destroyed).toEqual([]); + expect(localStorage.getItem(SUB_KEY)).toBe('push-new'); + }); + + it('leaves it alone when turning off', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'b'); + const client = makeClient([sub('b', OTHER_DEVICE)]); + installFetch({}); + + await disableWebPush({ client, relayBaseUrl: RELAY }); + + expect(client.destroyed).toEqual([]); + }); + + it('leaves it alone on Re-register', async () => { + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'b'); + const client = makeClient([sub('b', OTHER_DEVICE)]); + installFetch({ [OTHER_DEVICE]: true }); + + await enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false, forceRecreate: true }); + + expect(client.destroyed).toEqual([]); + }); +}); + describe('resyncWebPush when the relay changed (#903)', () => { afterEach(() => setNoThirdPartyTraffic(false)); diff --git a/lib/web-push.ts b/lib/web-push.ts index 11ca5a16b..20c969e90 100644 --- a/lib/web-push.ts +++ b/lib/web-push.ts @@ -874,7 +874,9 @@ async function enableWebPushForThisTab( const storedServerId = localStorage.getItem(subIdKey); let replacedServerId: string | null = null; if (storedServerId) { - const match = existingSubs.find((s) => s.id === storedServerId); + // By device too: Stalwart gives the id of an expired subscription to the next + // one, which may be another device's. + const match = existingSubs.find((s) => s.id === storedServerId && s.deviceClientId === deviceClientId); if (match) { if (!params.forceRecreate) { const refreshed = await refreshSubscriptionExpires(params.client, match, emailPush); @@ -910,7 +912,7 @@ async function enableWebPushForThisTab( // 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.id === replacedServerId) continue; if (s.deviceClientId === deviceClientId) { await params.client.destroyPushSubscription(s.id).catch(() => undefined); continue; @@ -1018,7 +1020,8 @@ async function disableWebPushForThisTab(params: DisableWebPushParams): Promise(); - if (storedServerId) idsToDestroy.add(storedServerId); + // The stored id alone can have gone to another device since (Stalwart reuses them). + if (storedServerId && !deviceClientId) idsToDestroy.add(storedServerId); if (deviceClientId) { const existingSubs = await params.client.listPushSubscriptions().catch(() => { failed.push('listing the subscriptions');