starred/native
1
0
Fork 0
mirror of https://github.com/bulwarkmail/native.git synced 2026-10-31 08:06:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: never patch or judge another client's push subscription that reuses this device's subscription id

Browse Source
This commit is contained in:
Linus Rath
2026-10-29 02:40:40 +01:00
parent ce47c73f33
commit 77327ea856
2 changed files with 92 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/lib/__tests__/push-notifications.test.ts
+67 -4
View File
@@ -59,7 +59,13 @@ const { DETACHED, SYNC_SESSION } = vi.hoisted(() => {
};
return {
SYNC_SESSION,
DETACHED: { loaded: true, types: ['EmailDelivery'] as string[], log: [] as unknown[], session: SYNC_SESSION },
DETACHED: {
loaded: true,
types: ['EmailDelivery'] as string[],
deviceClientId: null as string | null,
log: [] as unknown[],
session: SYNC_SESSION,
},
};
});
@@ -82,7 +88,8 @@ vi.mock('../../api/jmap-client', () => ({
const [name, args, id] = calls[0];
DETACHED.log.push([name, args]);
if (name === 'PushSubscription/get') {
return { methodResponses: [[name, { list: [{ id: args.ids[0], types: DETACHED.types }] }, id]] };
const listed = { id: args.ids[0], types: DETACHED.types, ...(DETACHED.deviceClientId ? { deviceClientId: DETACHED.deviceClientId } : {}) };
return { methodResponses: [[name, { list: [listed] }, id]] };
}
return { methodResponses: [[name, { updated: { [Object.keys(args.update)[0]]: null } }, id]] };
}
@@ -666,7 +673,7 @@ describe('push types for device sync (#34)', () => {
await refreshPushSubscriptionTypes(OTHER);
expect(DETACHED.log).toEqual([
['load', OTHER],
['PushSubscription/get', { ids: ['bob-sub'], properties: ['id', 'types'] }],
['PushSubscription/get', { ids: ['bob-sub'], properties: ['id', 'deviceClientId', 'types'] }],
['PushSubscription/set', { update: { 'bob-sub': { types: ['EmailDelivery', 'CalendarEvent', 'Calendar'] } } }],
]);
// The singleton's subscriptions were not touched.
@@ -679,6 +686,40 @@ describe('push types for device sync (#34)', () => {
expect(DETACHED.log.map((entry) => (entry as unknown[])[0])).toEqual(['load', 'PushSubscription/get']);
});
it("leaves alone another client's subscription that reuses the recorded id", async () => {
// Stalwart gives a destroyed subscription's id to the next one, such as
// the webmail's on the same account.
await AsyncStorage.setItem(SUB_KEY, 'existing');
listMock.mockResolvedValue([{ ...healthy(['EmailDelivery']), deviceClientId: 'webmailwebmailwebmailwebmailwebm' }]);
await syncing({ [CONTACTS]: true });
await refreshPushSubscriptionTypes(ACCOUNT_ID);
expect(updateMock).not.toHaveBeenCalled();
// The same through another account's client.
const OTHER = 'bob@other.example.net';
await AsyncStorage.setItem(`push:subscriptionId:v2:${OTHER}`, 'b');
await AsyncStorage.setItem(deviceClientIdKey(OTHER), 'bobsphonebobsphonebobsphonebobsp');
await AsyncStorage.setItem(DEVICE_SYNC_KEY, JSON.stringify({
state: { accounts: { [OTHER]: { enabled: { [CALENDAR]: true } } } },
version: 0,
}));
DETACHED.log = [];
DETACHED.types = ['EmailDelivery'];
DETACHED.deviceClientId = 'webmailwebmailwebmailwebmailwebm';
try {
await refreshPushSubscriptionTypes(OTHER);
expect(DETACHED.log.map((entry) => (entry as unknown[])[0])).toEqual(['load', 'PushSubscription/get']);
// Its own subscription still gets the types.
DETACHED.log = [];
DETACHED.deviceClientId = 'bobsphonebobsphonebobsphonebobsp';
await refreshPushSubscriptionTypes(OTHER);
expect(DETACHED.log.map((entry) => (entry as unknown[])[0])).toEqual(['load', 'PushSubscription/get', 'PushSubscription/set']);
} finally {
DETACHED.deviceClientId = null;
}
});
it('leaves the subscription of an account that turned push off alone', async () => {
await AsyncStorage.setItem(SUB_KEY, 'existing');
await AsyncStorage.setItem(`push:optedOut:v1:${ACCOUNT_ID}`, '1');
@@ -1497,6 +1538,24 @@ describe('resyncPushNotifications', () => {
expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).not.toBeNull();
});
it("takes a recorded subscription whose id now names another client's for revoked", async () => {
// Revoked here, then the webmail on the same account subscribed and got
// the freed id.
await AsyncStorage.setItem(SUB_KEY, 'b');
await AsyncStorage.setItem(EXPIRES_KEY, inDays(5));
await AsyncStorage.setItem('push:accountIds:v1', JSON.stringify([ACCOUNT_ID]));
listMock.mockResolvedValue([
{ id: 'b', deviceClientId: 'webmailwebmailwebmailwebmailwebm', expires: inDays(7), types: ['EmailDelivery'] },
]);
const result = await resyncPushNotifications({ relayBaseUrl: RELAY });
expect(result).toBeNull();
expect(createMock).not.toHaveBeenCalled();
expect(destroyMock.mock.calls.map((c) => c[0])).not.toContain('b');
expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).not.toBeNull();
});
it('re-creates the subscription once the server behind the account was replaced (0.16.23 to 1.0)', async () => {
// A server reinstalled for Stalwart 1.0 holds none of the subscriptions,
// which looks exactly like a revocation from elsewhere; the fingerprint
@@ -1613,8 +1672,12 @@ describe('resyncPushNotifications', () => {
expect(await AsyncStorage.getItem(OPTED_OUT_KEY)).toBeNull();
expect(await AsyncStorage.getItem(EXPIRES_KEY)).not.toBeNull();
vi.clearAllMocks();
// Turning push off forgot the device client id, so the new subscription
// carries a new one.
const deviceClientId = await AsyncStorage.getItem(deviceClientIdKey(ACCOUNT_ID));
expect(deviceClientId).not.toBe(OUR_DCID);
listMock.mockResolvedValue([
{ id: 'new-server-id', deviceClientId: OUR_DCID, expires: inDays(7), types: ['EmailDelivery'] },
{ id: 'new-server-id', deviceClientId, expires: inDays(7), types: ['EmailDelivery'] },
]);
expect((await resyncPushNotifications({ relayBaseUrl: RELAY }))?.subscriptionId).toBe('new-server-id');
});
src/lib/push-notifications.ts
+25 -5
View File
@@ -1703,8 +1703,25 @@ async function wasRevokedOnServer(accountId: string): Promise<boolean> {
const expires = Date.parse((await AsyncStorage.getItem(subscriptionExpiresKey(accountId))) ?? '');
if (!storedServerId || !Number.isFinite(expires)) return false;
if (expires - Date.now() < REVOKED_EXPIRY_MARGIN_MS) return false;
const deviceClientId = await AsyncStorage.getItem(deviceClientIdKey(accountId));
const subs = await listPushSubscriptions();
return !subs.some((s) => s.id === storedServerId);
return !subs.some((s) => isOwnSubscription(s, storedServerId, deviceClientId));
}
/**
* Whether a subscription the server lists is the one this device recorded.
* Stalwart hands out short ids and gives a destroyed subscription's id to the
* next one created ("b" in every account), so the id alone can name another
* client's subscription on the same account, such as the webmail's. A
* subscription listed without a device client id, or one recorded before the
* device kept its own, is taken on the id.
*/
function isOwnSubscription(
s: { id: string; deviceClientId?: string | null },
subscriptionId: string,
deviceClientId: string | null,
): boolean {
return s.id === subscriptionId && (!s.deviceClientId || !deviceClientId || s.deviceClientId === deviceClientId);
}
/**
@@ -1781,10 +1798,11 @@ export async function refreshPushSubscriptionTypes(accountId: string): Promise<v
const username = jmapClient.username;
const serverUrl = jmapClient.serverUrl;
const served = !!username && !!serverUrl && generateAccountId(username, serverUrl) === accountId;
const deviceClientId = await AsyncStorage.getItem(deviceClientIdKey(accountId));
try {
if (served) {
const types = await pushTypesFor(accountId, jmapClient.currentSession);
const current = (await listPushSubscriptions()).find((s) => s.id === subscriptionId);
const current = (await listPushSubscriptions()).find((s) => isOwnSubscription(s, subscriptionId, deviceClientId));
if (!current || sameTypes(current.types, types)) return;
await updatePushSubscription(subscriptionId, { types: [...types] });
return;
@@ -1794,11 +1812,13 @@ export async function refreshPushSubscriptionTypes(accountId: string): Promise<v
const types = await pushTypesFor(accountId, client.currentSession);
const using = [CAPABILITIES.CORE];
const res = await client.request(
[['PushSubscription/get', { ids: [subscriptionId], properties: ['id', 'types'] }, '0']],
[['PushSubscription/get', { ids: [subscriptionId], properties: ['id', 'deviceClientId', 'types'] }, '0']],
using,
);
const current = (requireMethodResult(res, '0', 'PushSubscription/get').list as Array<{ id: string; types?: string[] | null }> | undefined)
?.find((s) => s.id === subscriptionId);
const list = requireMethodResult(res, '0', 'PushSubscription/get').list as
| Array<{ id: string; deviceClientId?: string | null; types?: string[] | null }>
| undefined;
const current = list?.find((s) => isOwnSubscription(s, subscriptionId, deviceClientId));
if (!current || sameTypes(current.types, types)) return;
await client.request(
[['PushSubscription/set', { update: { [subscriptionId]: { types: [...types] } } }, '0']],