fix: move push registrations to the organisation's relay when the relay changes #903

lib/__tests__/web-push.test.ts

@@ -16,8 +16,11 @@ import {
   revokePushDevice,
   waitForWebPushResync,
 } from '@/lib/web-push';
+import { DEFAULT_RELAY_BASE_URL } from '@/lib/push-relays';
+import { setNoThirdPartyTraffic } from '@/lib/third-party-mode';
 
 const RELAY = 'https://relay.example';
+const RELAY_BASE_KEY = `bulwark.push.relayBase.v1.${'acct-1'}`;
 const ACCOUNT_ID = 'acct-1';
 const THIS_DEVICE = 'aaaaaaaabbbbbbbbccccccccdddddddd';
 const OTHER_DEVICE = '11111111222222223333333344444444';
@@ -469,6 +472,52 @@ describe('enableWebPush', () => {
 // Registrations made before the client learned about the delivery filter are
 // repaired in the background on app start - nobody should have to find the
 // settings toggle to stop the spam pushes.
+describe('resyncWebPush when the relay changed (#903)', () => {
+  afterEach(() => setNoThirdPartyTraffic(false));
+
+  it('moves a registration made with the built-in relay to the admin relay with no third-party traffic', async () => {
+    setNoThirdPartyTraffic(true);
+    localStorage.setItem(DEVICE_KEY, THIS_DEVICE);
+    localStorage.setItem(SUB_KEY, 'push-old');
+    localStorage.setItem(RELAY_BASE_KEY, DEFAULT_RELAY_BASE_URL);
+    const client = makeClient([sub('push-old', THIS_DEVICE, { url: `${DEFAULT_RELAY_BASE_URL}/api/push/jmap/${THIS_DEVICE}` })]);
+    const calls = installFetch({});
+
+    expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true);
+    // A new server record that points at the admin relay replaces the old one.
+    expect((client.createPushSubscription as ReturnType<typeof vi.fn>).mock.calls[0][0]).toMatchObject({ url: `${RELAY}/api/push/jmap/${THIS_DEVICE}` });
+    expect(client.destroyed).toContain('push-old');
+    expect(client.updatePushSubscription).not.toHaveBeenCalled();
+    expect(localStorage.getItem(RELAY_BASE_KEY)).toBe(RELAY);
+    expect(calls.some((c) => c.url.startsWith(DEFAULT_RELAY_BASE_URL))).toBe(false);
+  });
+
+  it('treats a registration from before the relay was recorded as the built-in one in that mode', async () => {
+    setNoThirdPartyTraffic(true);
+    localStorage.setItem(DEVICE_KEY, THIS_DEVICE);
+    localStorage.setItem(SUB_KEY, 'push-old');
+    const client = makeClient([sub('push-old', THIS_DEVICE)]);
+    installFetch({});
+
+    expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true);
+    expect(client.createPushSubscription).toHaveBeenCalledTimes(1);
+    expect(client.destroyed).toContain('push-old');
+  });
+
+  it('re-registers inside the daily limit when the admin switched relays, and only once', async () => {
+    localStorage.setItem(DEVICE_KEY, THIS_DEVICE);
+    localStorage.setItem(SUB_KEY, 'push-old');
+    localStorage.setItem(RELAY_BASE_KEY, 'https://old-relay.example');
+    const client = makeClient([sub('push-old', THIS_DEVICE)]);
+    installFetch({});
+
+    expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(true);
+    expect(client.createPushSubscription).toHaveBeenCalledTimes(1);
+    expect(await resyncWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false })).toBe(false);
+    expect(client.createPushSubscription).toHaveBeenCalledTimes(1);
+  });
+});
+
 describe('resyncWebPush', () => {
   it('repairs a lost browser subscription for an account that previously opted in', async () => {
     localStorage.setItem(DEVICE_KEY, THIS_DEVICE);

lib/web-push.ts

@@ -8,7 +8,7 @@ import type { IJMAPClient } from '@/lib/jmap/client-interface';
 import type { AccountInfo, EmailPushConfig, Mailbox } from '@/lib/jmap/types';
 import { isJmapSetError, jmapErrorType } from '@/lib/jmap/jmap-error';
 import { DEFAULT_RELAY_BASE_URL } from '@/lib/push-relays';
-import { relayAllowed } from '@/lib/outgoing-hosts';
+import { isBuiltInHost, relayAllowed } from '@/lib/outgoing-hosts';
 import { noThirdPartyTraffic } from '@/lib/third-party-mode';
 import { IS_LITE } from '@/lib/lite';
 import {
@@ -39,6 +39,16 @@ function subscriptionIdKey(accountId: string): string {
   return SUBSCRIPTION_ID_PREFIX + accountId;
 }
 
+/**
+ * The relay base a registration was made with. The server's PushSubscription
+ * points at it, so a later change of relay (the admin's list, or the built-in
+ * one going away with no third-party traffic, #903) needs a new record.
+ */
+const RELAY_BASE_PREFIX = 'bulwark.push.relayBase.v1.';
+function relayBaseKey(accountId: string): string {
+  return RELAY_BASE_PREFIX + accountId;
+}
+
 function emailPushRefusedKey(accountId: string): string {
   return EMAIL_PUSH_REFUSED_PREFIX + accountId;
 }
@@ -851,6 +861,7 @@ export async function enableWebPush(
   const verificationCode = await pollVerificationCode(relayBaseUrl, deviceClientId);
   await params.client.verifyPushSubscription(serverAssignedId, verificationCode);
   localStorage.setItem(subIdKey, serverAssignedId);
+  localStorage.setItem(relayBaseKey(accountId), relayBaseUrl);
   if (replacedServerId) {
     await params.client.destroyPushSubscription(replacedServerId).catch(() => undefined);
   }
@@ -894,6 +905,7 @@ export async function disableWebPush(params: DisableWebPushParams): Promise<void
     await params.client.destroyPushSubscription(id).catch(() => undefined);
   }
   localStorage.removeItem(subIdKey);
+  localStorage.removeItem(relayBaseKey(accountId));
   // Enabling again starts from the full emailPush map.
   localStorage.removeItem(emailPushRefusedKey(accountId));
 
@@ -1083,9 +1095,21 @@ async function runResync(accountId: string, params: ResyncWebPushParams): Promis
       await disableWebPush({ client: params.client });
       return false;
     }
-    if (!params.force && !(await resyncDue(accountId))) return false;
+    // The registration points at another relay than the one to use now (the
+    // admin changed the list, or the built-in relay went away with no
+    // third-party traffic): make a new server record that points here. A
+    // registration from before the relay was remembered counts as the
+    // built-in one in that mode.
+    const current = (params.relayBaseUrl ?? DEFAULT_RELAY_BASE_URL).replace(/\/+$/, '');
+    const registeredWith = localStorage.getItem(relayBaseKey(accountId));
+    const moved = !!current && (
+      (registeredWith !== null && registeredWith !== current)
+      || (noThirdPartyTraffic() && (registeredWith === null || isBuiltInHost(registeredWith)) && !isBuiltInHost(current))
+    );
+    if (!moved && !params.force && !(await resyncDue(accountId))) return false;
     await enableWebPush({
       client: params.client,
+      forceRecreate: moved,
       relayBaseUrl: params.relayBaseUrl,
       accountLabel: params.accountLabel,
       inboxOnly: params.inboxOnly,