diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index 519b47ce..11db452c 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -43,10 +43,21 @@ const { CREATED } = vi.hoisted(() => ({ })); // A client of another signed-in account (device sync patches its push types -// without the singleton). -const { DETACHED } = vi.hoisted(() => ({ - DETACHED: { loaded: true, types: ['EmailDelivery'] as string[], log: [] as unknown[] }, -})); +// without the singleton), on a server with contacts and calendars unless a +// test says otherwise. +const { DETACHED, SYNC_SESSION } = vi.hoisted(() => { + const SYNC_SESSION = { + capabilities: { + 'urn:ietf:params:jmap:core': {}, + 'urn:ietf:params:jmap:contacts': {}, + 'urn:ietf:params:jmap:calendars': {}, + } as Record, + }; + return { + SYNC_SESSION, + DETACHED: { loaded: true, types: ['EmailDelivery'] as string[], log: [] as unknown[], session: SYNC_SESSION }, + }; +}); vi.mock('../../api/jmap-client', () => ({ jmapClient: { @@ -56,6 +67,9 @@ vi.mock('../../api/jmap-client', () => ({ currentSession: { capabilities: { 'urn:ietf:params:jmap:core': {} } }, }, JMAPClient: class { + get currentSession() { + return DETACHED.session; + } async loadAccount(id: string) { DETACHED.log.push(['load', id]); return DETACHED.loaded; @@ -112,7 +126,7 @@ import { } from '../../api/push'; import { getSharedMailboxes } from '../../api/email'; import { JMAPMethodError } from '../../api/jmap-result'; -import type { EmailPushConfig } from '../../api/types'; +import type { EmailPushConfig, JMAPSession } from '../../api/types'; import { jmapClient } from '../../api/jmap-client'; import { NativeModules } from 'react-native'; import { generateAccountId } from '../account-utils'; @@ -394,26 +408,103 @@ describe('push types for device sync (#34)', () => { })); } + // A server that does not know a type refuses the whole write. + const refusedTypes = (types: string[] | undefined) => { + const unknown = (types ?? []).filter((type) => type !== 'EmailDelivery'); + if (unknown.length > 0) throw new JMAPMethodError('invalidProperties', `Unknown types: ${unknown.join(', ')}`); + }; + beforeEach(async () => { vi.clearAllMocks(); await AsyncStorage.clear(); await AsyncStorage.setItem(deviceClientIdKey(ACCOUNT_ID), OUR_DCID); - (jmapClient as { currentSession: unknown }).currentSession = { - capabilities: { 'urn:ietf:params:jmap:core': {} }, - }; + (jmapClient as { currentSession: unknown }).currentSession = SYNC_SESSION; + DETACHED.session = SYNC_SESSION; listMock.mockResolvedValue([]); updateMock.mockResolvedValue(true); installFetch({}); }); + afterEach(() => { + (jmapClient as { currentSession: unknown }).currentSession = { + capabilities: { 'urn:ietf:params:jmap:core': {} }, + }; + DETACHED.session = SYNC_SESSION; + createMock.mockImplementation(async () => CREATED); + updateMock.mockImplementation(async () => undefined); + }); + it('adds the contact and calendar types of what the account syncs to the device', async () => { - expect(await pushTypesFor(ACCOUNT_ID)).toEqual(['EmailDelivery']); + expect(await pushTypesFor(ACCOUNT_ID, SYNC_SESSION as unknown as JMAPSession)).toEqual(['EmailDelivery']); await syncing({ [CONTACTS]: true, [CALENDAR]: false }); - expect(await pushTypesFor(ACCOUNT_ID)).toEqual(['EmailDelivery', 'ContactCard', 'AddressBook']); - expect(await pushTypesFor('someone@else.example')).toEqual(['EmailDelivery']); + expect(await pushTypesFor(ACCOUNT_ID, SYNC_SESSION as unknown as JMAPSession)) + .toEqual(['EmailDelivery', 'ContactCard', 'AddressBook']); + expect(await pushTypesFor('someone@else.example', SYNC_SESSION as unknown as JMAPSession)).toEqual(['EmailDelivery']); // Removed in Android Settings: nothing to route any more. await syncing({ [CONTACTS]: true }, { removedInAndroidSettings: true }); - expect(await pushTypesFor(ACCOUNT_ID)).toEqual(['EmailDelivery']); + expect(await pushTypesFor(ACCOUNT_ID, SYNC_SESSION as unknown as JMAPSession)).toEqual(['EmailDelivery']); + }); + + it('adds only the types of what the server offers', async () => { + await syncing({ [CONTACTS]: true, [CALENDAR]: true }); + const calendarsOnly = { capabilities: { 'urn:ietf:params:jmap:core': {}, 'urn:ietf:params:jmap:calendars': {} } }; + expect(await pushTypesFor(ACCOUNT_ID, calendarsOnly as unknown as JMAPSession)) + .toEqual(['EmailDelivery', 'CalendarEvent', 'Calendar']); + expect(await pushTypesFor(ACCOUNT_ID, null)).toEqual(['EmailDelivery']); + + // Sync was turned on while another account's server was served; this one + // has neither: its subscription stays mail-only. + (jmapClient as { currentSession: unknown }).currentSession = { + capabilities: { 'urn:ietf:params:jmap:core': {} }, + }; + createMock.mockImplementation(async (params: { types: string[] }) => { + refusedTypes(params.types); + return CREATED; + }); + const result = await setupPushNotifications({ relayBaseUrl: RELAY }); + expect(result.subscriptionId).toBe('new-server-id'); + expect(createMock.mock.calls.map((c) => c[0].types)).toEqual([['EmailDelivery']]); + }); + + it("does not add types another account's server does not offer", async () => { + const OTHER = 'bob@other.example.net'; + await AsyncStorage.setItem(`push:subscriptionId:v2:${OTHER}`, 'bob-sub'); + await AsyncStorage.setItem(DEVICE_SYNC_KEY, JSON.stringify({ + state: { accounts: { [OTHER]: { enabled: { [CONTACTS]: true } } } }, + version: 0, + })); + DETACHED.session = { capabilities: { 'urn:ietf:params:jmap:core': {}, 'urn:ietf:params:jmap:calendars': {} } }; + DETACHED.log = []; + DETACHED.types = ['EmailDelivery']; + await refreshPushSubscriptionTypes(OTHER); + expect(DETACHED.log.map((entry) => (entry as unknown[])[0])).toEqual(['load', 'PushSubscription/get']); + }); + + it('keeps mail push on a server that refuses the device sync types', async () => { + await syncing({ [CONTACTS]: true }); + createMock.mockImplementation(async (params: { types: string[] }) => { + refusedTypes(params.types); + return CREATED; + }); + updateMock.mockImplementation(async (_id: string, patch: { types?: string[] }) => refusedTypes(patch.types)); + + // A new subscription: mail-only rather than none. + const created = await setupPushNotifications({ relayBaseUrl: RELAY }); + expect(created).toEqual({ subscriptionId: 'new-server-id', verified: true }); + expect(createMock.mock.calls.map((c) => c[0].types)) + .toEqual([['EmailDelivery', 'ContactCard', 'AddressBook'], ['EmailDelivery']]); + + // The working subscription is kept and its expiry pushed forward, never + // replaced by one the server refuses as well. + vi.clearAllMocks(); + await AsyncStorage.setItem(SUB_KEY, 'existing'); + listMock.mockResolvedValue([healthy(['EmailDelivery'])]); + const refreshed = await setupPushNotifications({ relayBaseUrl: RELAY }); + expect(refreshed.subscriptionId).toBe('existing'); + expect(updateMock.mock.calls.map((c) => c[1].types)).toEqual([['EmailDelivery', 'ContactCard', 'AddressBook'], undefined]); + expect(updateMock.mock.calls[1][1].expires).toBeDefined(); + expect(createMock).not.toHaveBeenCalled(); + expect(destroyMock).not.toHaveBeenCalled(); }); it('subscribes with them', async () => { diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index 4151665b..97c48971 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -12,10 +12,16 @@ import { loadedMailboxes } from './mailbox-source'; import { jmapClient, JMAPClient } from '../api/jmap-client'; import { JMAPMethodError, requireMethodResult } from '../api/jmap-result'; import { CAPABILITIES } from '../api/types'; -import type { EmailPushConfig, JMAPAccountInfo, Mailbox } from '../api/types'; +import type { EmailPushConfig, JMAPAccountInfo, JMAPSession, Mailbox } from '../api/types'; import { generateAccountId } from './account-utils'; import { t } from '../stores/locale-store'; -import { DEVICE_SYNC_STORAGE_KEY, deviceSyncPushTypes, persistedAccounts } from '../device-sync/app/prefs'; +import { + authorityOfType, + DEVICE_SYNC_STORAGE_KEY, + deviceSyncPushTypes, + persistedAccounts, +} from '../device-sync/app/prefs'; +import { CALENDAR_AUTHORITY, CONTACTS_AUTHORITY, type Authority } from '../device-sync/types'; import { getUnifiedPushDistributors, isUnifiedPushSupported, @@ -207,19 +213,32 @@ export function isValidRelayUrl(value: string): boolean { // In-app sync uses the separate SSE channel and is unaffected. export const PUSH_TYPES = ['EmailDelivery'] as const; +// The capability a server advertises for the data types of an authority. +const DEVICE_SYNC_CAPABILITIES: Record = { + [CONTACTS_AUTHORITY]: CAPABILITIES.CONTACTS, + [CALENDAR_AUTHORITY]: CAPABILITIES.CALENDARS, +}; + /** * The push types of one account's subscription: `EmailDelivery`, plus the * contact and calendar types while the account syncs them to the device * (#34, docs/device-sync.md "Triggers"): the native push router turns those - * into device syncs, and a push carrying only them never notifies. + * into device syncs, and a push carrying only them never notifies. Only the + * types of a capability the account's session advertises: a server without + * contacts or calendars may refuse the whole subscription over them. * * Read from the device sync store's persisted JSON rather than the store, so * code that runs without it (the push task) never creates it. */ -export async function pushTypesFor(accountId: string): Promise { +export async function pushTypesFor(accountId: string, session: JMAPSession | null): Promise { let extra: string[] = []; try { - extra = deviceSyncPushTypes(persistedAccounts(await AsyncStorage.getItem(DEVICE_SYNC_STORAGE_KEY))[accountId]); + const capabilities = session?.capabilities ?? {}; + extra = deviceSyncPushTypes(persistedAccounts(await AsyncStorage.getItem(DEVICE_SYNC_STORAGE_KEY))[accountId]) + .filter((type) => { + const authority = authorityOfType(type); + return !!authority && DEVICE_SYNC_CAPABILITIES[authority] in capabilities; + }); } catch { extra = []; } @@ -404,6 +423,24 @@ async function writeWithEmailPush( } } +/** + * Run a subscription write with the account's push types and, when the server + * refuses it while they include device sync's, once more with the mail types + * alone: mail push never depends on device sync (#34). + */ +async function writeWithPushTypes( + types: readonly string[], + write: (types: readonly string[]) => Promise, +): Promise { + try { + return await write(types); + } catch (err) { + if (sameTypes(types, PUSH_TYPES)) throw err; + logPhase('jmap', 'push types refused, retrying with the mail types alone'); + return write(PUSH_TYPES); + } +} + async function readRefusedEmailPushAccounts(accountId: string): Promise { try { const parsed = JSON.parse((await AsyncStorage.getItem(emailPushRefusedKey(accountId))) ?? '[]'); @@ -947,7 +984,7 @@ async function setupPushNotificationsInner( : null; const subKey = subscriptionIdKey(accountId); const storedServerId = await AsyncStorage.getItem(subKey); - const types = await pushTypesFor(accountId); + const types = await pushTypesFor(accountId, jmapClient.currentSession); let jmapAccountId: string | null = null; try { jmapAccountId = jmapClient.accountId; @@ -1006,14 +1043,16 @@ async function setupPushNotificationsInner( let serverAssignedId: string; let serverExpires: string | null; try { - const created = await writeWithEmailPush(emailPush, (filter) => - createPushSubscription({ - deviceClientId, - url: buildRelayUrl(relayBaseUrl, `/api/push/jmap/${encodeURIComponent(deviceClientId)}`), - types: [...types], - expires: expiresFromNow(SUBSCRIPTION_EXPIRES_DAYS), - ...(filter ? { emailPush: filter } : {}), - }), + const created = await writeWithPushTypes(types, (wanted) => + writeWithEmailPush(emailPush, (filter) => + createPushSubscription({ + deviceClientId, + url: buildRelayUrl(relayBaseUrl, `/api/push/jmap/${encodeURIComponent(deviceClientId)}`), + types: [...wanted], + expires: expiresFromNow(SUBSCRIPTION_EXPIRES_DAYS), + ...(filter ? { emailPush: filter } : {}), + }), + ), ); serverAssignedId = created.result.id; serverExpires = created.result.expires; @@ -1063,8 +1102,9 @@ async function addPushAccountId(accountId: string): Promise { // from what this client wants (a subscription created by an older build still // listens to `Email`/`Mailbox`; device sync was turned on or off; a Junk // mailbox id can change under us). Returns false if the server rejects the -// update, which the caller treats as "replace"; otherwise the emailPush map -// the update installed (null when it left the filter alone). +// update, also with the mail types alone, which the caller treats as +// "replace"; otherwise the emailPush map the update installed (null when it +// left the filter alone). async function refreshSubscriptionExpires( sub: { id: string; @@ -1088,15 +1128,17 @@ async function refreshSubscriptionExpires( } } try { - const patch: { expires?: string; types?: string[] } = { - expires: expiresFromNow(SUBSCRIPTION_EXPIRES_DAYS), - }; - if (typesNeedUpdate) patch.types = [...types]; - const { emailPush } = await writeWithEmailPush( - emailPushNeedsUpdate ? desiredEmailPush : null, - (filter) => updatePushSubscription(sub.id, filter ? { ...patch, emailPush: filter } : patch), - ); - return { emailPush }; + return await writeWithPushTypes(types, async (wanted) => { + const patch: { expires?: string; types?: string[] } = { + expires: expiresFromNow(SUBSCRIPTION_EXPIRES_DAYS), + }; + if (!sameTypes(sub.types, wanted)) patch.types = [...wanted]; + const { emailPush } = await writeWithEmailPush( + emailPushNeedsUpdate ? desiredEmailPush : null, + (filter) => updatePushSubscription(sub.id, filter ? { ...patch, emailPush: filter } : patch), + ); + return { emailPush }; + }); } catch { return false; } @@ -1231,12 +1273,12 @@ export async function refreshPushSubscriptionTypes(accountId: string): Promise s.id === subscriptionId); if (!current || sameTypes(current.types, types)) return; await updatePushSubscription(subscriptionId, { types: [...types] }); @@ -1244,6 +1286,7 @@ export async function refreshPushSubscriptionTypes(accountId: string): Promise