fix: show an error when the server refuses a push registration change

The PushSubscription calls read the first method response without
checking it, so an `error` response listed no devices, and a refused
verify or destroy (revoking another device) was reported as done. Route
every call through `requireMethodResult` and `assertSetResult`; a
destroy the server answers with `notFound` still counts as done.
This commit is contained in:
Linus Rath
2026-09-24 19:09:35 +02:00
parent ccaa24ecd3
commit ddfe85a5b2
2 changed files with 54 additions and 14 deletions
+40
View File
@@ -15,9 +15,11 @@ vi.mock('../jmap-client', () => ({
import { jmapClient } from '../jmap-client';
import {
createPushSubscription,
destroyPushSubscription,
listPushSubscriptions,
startPolling,
updatePushSubscription,
verifyPushSubscription,
} from '../push';
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
@@ -121,6 +123,44 @@ describe('push operations', () => {
.rejects.toMatchObject({ type: 'forbidden' });
});
it('throws when the verification code is refused', async () => {
mockRequest.mockResolvedValue({
methodResponses: [[
'PushSubscription/set',
{ notUpdated: { s1: { type: 'invalidProperties', description: 'Verification code does not match.' } } },
'0',
]],
});
await expect(verifyPushSubscription('s1', 'CODE')).rejects.toThrow('Verification code does not match.');
});
it('throws when a destroy is refused', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['PushSubscription/set', { notDestroyed: { s1: forbidden } }, '0']],
});
await expect(destroyPushSubscription('s1')).rejects.toMatchObject({ type: 'forbidden' });
});
it('treats destroying a subscription that is already gone as done', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['PushSubscription/set', { notDestroyed: { s1: { type: 'notFound' } } }, '0']],
});
await expect(destroyPushSubscription('s1')).resolves.toBeUndefined();
});
it.each([
['list', () => listPushSubscriptions()],
['create', () => createPushSubscription({ deviceClientId: 'd', url: 'https://relay/x', types: ['EmailDelivery'] })],
['update', () => updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z' })],
['verify', () => verifyPushSubscription('s1', 'CODE')],
['destroy', () => destroyPushSubscription('s1')],
])('%s throws on a method-level error response', async (_name, call) => {
mockRequest.mockResolvedValue({
methodResponses: [['error', { type: 'serverFail', description: 'Store unavailable' }, '0']],
});
await expect(call()).rejects.toMatchObject({ name: 'JMAPMethodError', type: 'serverFail' });
});
it('resolves when the update is applied', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['PushSubscription/set', { updated: { s1: null } }, '0']],
+14 -14
View File
@@ -1,5 +1,5 @@
import { jmapClient } from './jmap-client';
import { assertSetResult, JMAPMethodError } from './jmap-result';
import { assertSetResult, JMAPMethodError, requireMethodResult } from './jmap-result';
import { CAPABILITIES } from './types';
import type { EmailPushConfig, PushSubscription, StateChange } from './types';
@@ -33,8 +33,8 @@ export async function listPushSubscriptions(): Promise<PushSubscription[]> {
[['PushSubscription/get', args, '0']],
pushUsing(withEmailPush),
);
const [, body] = res.methodResponses[0] ?? [];
return (body?.list as PushSubscription[]) ?? [];
const body = requireMethodResult(res, '0', 'PushSubscription/get');
return (body.list as PushSubscription[]) ?? [];
}
/**
@@ -74,9 +74,9 @@ export async function createPushSubscription(params: {
],
pushUsing(withEmailPush),
);
const [, body] = res.methodResponses[0] ?? [];
const body = requireMethodResult(res, '0', 'PushSubscription/set');
assertSetResult(body, ['new'], 'push subscription');
const result = body?.created?.new as { id?: string } | undefined;
const result = body.created?.new as { id?: string } | undefined;
if (!result?.id) {
throw new Error(`PushSubscription/set create failed: ${JSON.stringify(body)}`);
}
@@ -107,9 +107,9 @@ export async function updatePushSubscription(
],
pushUsing(withEmailPush),
);
const [, body] = res.methodResponses[0] ?? [];
const body = requireMethodResult(res, '0', 'PushSubscription/set');
assertSetResult(body, [id], 'push subscription');
if (body?.updated?.[id] === undefined) {
if (body.updated?.[id] === undefined) {
throw new JMAPMethodError('notUpdated', `PushSubscription/set did not update ${id}`);
}
}
@@ -132,19 +132,19 @@ export async function verifyPushSubscription(
],
[CAPABILITIES.CORE],
);
const [, body] = res.methodResponses[0] ?? [];
if (body?.notUpdated?.[id]) {
throw new Error(
`PushSubscription verification failed: ${JSON.stringify(body.notUpdated[id])}`,
);
}
const body = requireMethodResult(res, '0', 'PushSubscription/set');
assertSetResult(body, [id], 'push subscription');
}
export async function destroyPushSubscription(id: string): Promise<void> {
await jmapClient.request(
const res = await jmapClient.request(
[['PushSubscription/set', { destroy: [id] }, '0']],
[CAPABILITIES.CORE],
);
const body = requireMethodResult(res, '0', 'PushSubscription/set');
// Already gone is what the caller asked for.
if (body.notDestroyed?.[id]?.type === 'notFound') return;
assertSetResult(body, [id], 'push subscription');
}
// ─── Live updates ────────────────────────────────────────