fix: keep push notifications working when a mailbox is shared with you

Stalwart refuses a push subscription's whole emailPush map as a
`forbidden` SetError when it names a mailbox shared by ACL (0.16.16 and
later, and 1.0), and 1.0 stops listing emailpush on such accounts. The
app narrowed the map on any `forbidden`, including a refused method
call, and put every account with folders in the first attempt, so 1.0
cost one refused call per install. Worse, a failed listing counted as
"the server has no subscription" and dropped the key of a working
registration, and any failed refresh - a network error, a timeout, rate
limiting - started a replacement.

- `jmap-result` gains `JMAPSetError` (a `JMAPMethodError` with the same
  name, carrying `properties`) and `isJMAPSetError`. `assertSetResult`
  throws it, so every refused object can be told apart from a refused
  call; existing `instanceof` checks and the outbox's name-based error
  classification are unchanged.
- The emailPush map holds the primary account and the accounts whose
  `accountCapabilities` list `urn:ietf:params:jmap:emailpush`, which
  leaves ACL shares out up front on 1.0. On 0.16.16-0.16.23 the refused
  map still narrows to the accounts that look owned, then to the
  primary account, but only on a `forbidden` SetError.
- A subscription ends only with a verified replacement, a user disable
  or the server's verdict: a failed listing aborts the run, and only a
  SetError on the refresh starts a replacement. Other failures leave
  the subscription and its key alone and the next run tries again.
- `forbidden` and `overQuota` read as plain text in settings and the
  onboarding prompt, and settings says that mailboxes others share
  don't send background notifications. New strings in all 27 languages.
This commit is contained in:
Linus Rath
2026-09-25 20:56:57 +02:00
parent 3f4a81b7ad
commit e8a9c58d18
35 changed files with 733 additions and 55 deletions
+85
View File
@@ -0,0 +1,85 @@
import { describe, it, expect } from 'vitest';
import {
assertSetResult,
isJMAPSetError,
JMAPMethodError,
JMAPSetError,
requireMethodResult,
} from '../jmap-result';
import { isTransientNetworkError } from '../../lib/network-error';
// A SetError refuses one object of a /set call; a method error refuses the
// whole call. `forbidden` comes both ways, and only the SetError says
// something about the object. Shapes as Stalwart 0.16.16-0.16.23 and 1.0
// answer them (jmap-proto method/set.rs, error/set.rs).
const EMAIL_PUSH_REFUSED = {
type: 'forbidden',
description: 'No access to one of the accounts in the emailPush map.',
};
describe('JMAPSetError', () => {
it.each([
['notCreated', 'new', 'create'],
['notUpdated', 's1', 'update'],
['notDestroyed', 's1', 'destroy'],
])('is what assertSetResult throws for %s (Stalwart 0.16.16+ and 1.0)', (bucket, id, verb) => {
const body = { [bucket]: { [id]: EMAIL_PUSH_REFUSED } };
let thrown: unknown;
try {
assertSetResult(body, [id], 'push subscription');
} catch (err) {
thrown = err;
}
expect(isJMAPSetError(thrown)).toBe(true);
// Still a JMAPMethodError under that name, so older checks keep working.
expect(thrown).toBeInstanceOf(JMAPMethodError);
expect(thrown).toMatchObject({ name: 'JMAPMethodError', type: 'forbidden' });
expect((thrown as Error).message).toBe(
`Failed to ${verb} push subscription ${id}: forbidden – No access to one of the accounts in the emailPush map.`,
);
});
it('carries the properties the server named (Stalwart 1.0 keyword cap)', () => {
const body = {
notUpdated: {
m1: { type: 'invalidProperties', description: 'Keywords cannot be longer than 128 bytes.', properties: ['keywords'] },
},
};
expect(() => assertSetResult(body, ['m1'], 'email')).toThrow(JMAPSetError);
try {
assertSetResult(body, ['m1'], 'email');
} catch (err) {
expect((err as JMAPSetError).properties).toEqual(['keywords']);
expect((err as JMAPSetError).type).toBe('invalidProperties');
}
});
it('is not what a refused method call throws, even with the same type', () => {
const res = {
methodResponses: [['error', { type: 'forbidden', description: 'Not allowed' }, '0']] as Array<
[string, Record<string, unknown>, string]
>,
};
let thrown: unknown;
try {
requireMethodResult(res, '0', 'PushSubscription/set');
} catch (err) {
thrown = err;
}
expect(thrown).toBeInstanceOf(JMAPMethodError);
expect(isJMAPSetError(thrown)).toBe(false);
expect((thrown as JMAPMethodError).type).toBe('forbidden');
});
it('counts as a server refusal for the outbox, not a connectivity blip', () => {
// The outbox classifies by error name; a SetError must stay terminal.
expect(isTransientNetworkError(new JMAPSetError('forbidden', 'request timed out'))).toBe(false);
});
it('reads nothing else as a SetError', () => {
expect(isJMAPSetError(new JMAPMethodError('forbidden'))).toBe(false);
expect(isJMAPSetError(new Error('forbidden'))).toBe(false);
expect(isJMAPSetError({ type: 'forbidden' })).toBe(false);
expect(isJMAPSetError(undefined)).toBe(false);
});
});
+46
View File
@@ -21,6 +21,7 @@ import {
updatePushSubscription,
verifyPushSubscription,
} from '../push';
import { isJMAPSetError } from '../jmap-result';
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
const mockHasCapability = jmapClient.hasCapability as ReturnType<typeof vi.fn>;
@@ -191,6 +192,51 @@ describe('push operations', () => {
});
await expect(updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z' })).resolves.toBeUndefined();
});
// The push setup narrows the emailPush map, or replaces a subscription,
// only on the server's verdict about the subscription itself.
const rejection = (call: () => Promise<unknown>) => call().then(() => null, (err: unknown) => err);
it('throws a refused create as a SetError (Stalwart 0.16.16+ and 1.0)', async () => {
// As Stalwart answers a map naming a mailbox shared by ACL: no
// accountId, state or `created` key (jmap-proto method/set.rs).
mockRequest.mockResolvedValue({
methodResponses: [['PushSubscription/set', { notCreated: { new: forbidden } }, '0']],
});
const err = await rejection(() =>
createPushSubscription({ deviceClientId: 'd', url: 'https://relay/x', types: ['EmailDelivery'] }));
expect(isJMAPSetError(err)).toBe(true);
expect(err).toMatchObject({ type: 'forbidden' });
});
it('throws an update of a vanished subscription as a notFound SetError', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['PushSubscription/set', { notUpdated: { s1: { type: 'notFound' } } }, '0']],
});
const err = await rejection(() => updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z' }));
expect(isJMAPSetError(err)).toBe(true);
expect(err).toMatchObject({ type: 'notFound' });
});
it('throws a refused method call as a plain method error, even when forbidden', async () => {
mockRequest.mockResolvedValue({
methodResponses: [['error', { type: 'forbidden', description: 'Permission denied' }, '0']],
});
const create = await rejection(() =>
createPushSubscription({ deviceClientId: 'd', url: 'https://relay/x', types: ['EmailDelivery'] }));
const update = await rejection(() => updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z' }));
for (const err of [create, update]) {
expect(err).toMatchObject({ name: 'JMAPMethodError', type: 'forbidden' });
expect(isJMAPSetError(err)).toBe(false);
}
});
it('does not read an update that is neither applied nor refused as a SetError', async () => {
mockRequest.mockResolvedValue({ methodResponses: [['PushSubscription/set', {}, '0']] });
const err = await rejection(() => updatePushSubscription('s1', { expires: '2026-10-01T00:00:00Z' }));
expect(err).toMatchObject({ name: 'JMAPMethodError' });
expect(isJMAPSetError(err)).toBe(false);
});
});
describe('startPolling', () => {
+27 -20
View File
@@ -60,8 +60,31 @@ export function requireMethodResult<T = Record<string, any>>(
}
/**
* Throw when a /set response reports the given ids (or any id when `ids` is
* omitted) under `notCreated` / `notUpdated` / `notDestroyed`.
* A SetError (RFC 8620 §5.3): the server looked at one object of a /set call
* and refused it (`notCreated` / `notUpdated` / `notDestroyed`), as opposed
* to refusing the whole method call. The same type can come either way
* (`forbidden` is both), and only a SetError says something about that
* object. It is a `JMAPMethodError` with that name, so `instanceof` checks
* and the name-based classification in lib/network-error treat it as before.
*/
export class JMAPSetError extends JMAPMethodError {
/** The properties the server named (`invalidProperties`), if any. */
properties?: string[];
constructor(type: string, description?: string, properties?: string[]) {
super(type, description);
if (properties?.length) this.properties = properties;
}
}
/** Whether a thrown value is a SetError rather than a refused method call. */
export function isJMAPSetError(err: unknown): err is JMAPSetError {
return err instanceof JMAPSetError;
}
/**
* Throw a `JMAPSetError` when a /set response reports the given ids (or any
* id when `ids` is omitted) under `notCreated` / `notUpdated` /
* `notDestroyed`.
*/
export function assertSetResult(
body: Record<string, any>,
@@ -82,9 +105,10 @@ export function assertSetResult(
if (err?.properties?.length) detail.push(`properties=[${err.properties.join(', ')}]`);
if (err?.description) detail.push(err.description);
const more = entries.length > 1 ? ` (+${entries.length - 1} more)` : '';
throw new JMAPMethodError(
throw new JMAPSetError(
err?.type ?? 'setError',
`Failed to ${verb} ${what} ${id}${more}: ${detail.join(' – ')}`,
err?.properties,
);
}
}
@@ -134,23 +158,6 @@ export function batched<T>(items: T[], size: number): T[][] {
return result;
}
/**
* A SetError (RFC 8620 §5.3): the server looked at one object of a /set call
* and refused it (`notCreated` / `notUpdated` / `notDestroyed`), as opposed
* to refusing the whole method call. The same type can come either way
* (`forbidden` is both), and only a SetError says something about that
* object. It is a `JMAPMethodError` with that name, so `instanceof` checks
* and the name-based classification in lib/network-error treat it as before.
*/
export class JMAPSetError extends JMAPMethodError {
/** The properties the server named (`invalidProperties`), if any. */
properties?: string[];
constructor(type: string, description?: string, properties?: string[]) {
super(type, description);
if (properties?.length) this.properties = properties;
}
}
/**
* The properties a thrown SetError names, so a caller can tell
* `invalidProperties` on `media` from one on `name`; empty for anything else.
+13 -7
View File
@@ -20,6 +20,11 @@ function pushUsing(withEmailPush: boolean): string[] {
return withEmailPush ? [CAPABILITIES.CORE, CAPABILITIES.EMAIL_PUSH] : [CAPABILITIES.CORE];
}
/**
* Every PushSubscription of the account. Throws when the server can't be
* asked or refuses the call - never an empty list instead: to the push setup
* an empty list means "the server no longer has our subscription".
*/
export async function listPushSubscriptions(): Promise<PushSubscription[]> {
const withEmailPush = hasEmailPushCapability();
// `emailPush` is not in the server's default property set, so ask for it
@@ -41,9 +46,9 @@ export async function listPushSubscriptions(): Promise<PushSubscription[]> {
* Create a PushSubscription pointing the JMAP server at the given relay URL.
* Returns the server-assigned id (which the client also registers with the
* relay so the relay can route incoming pushes to an Expo token) and the
* expiry the server settled on. A refusal throws a JMAPMethodError carrying
* the server's SetError type, so callers can tell a `forbidden` emailPush map
* from other failures.
* expiry the server settled on. A refused subscription throws a JMAPSetError
* carrying the SetError type, so callers can tell a `forbidden` emailPush map
* from a refused method call and other failures.
*/
export async function createPushSubscription(params: {
deviceClientId: string;
@@ -88,10 +93,11 @@ export async function createPushSubscription(params: {
/**
* Push the subscription's expiry forward and re-sync its types or delivery
* filter (RFC 8620 §7.2.1). Throws a JMAPMethodError carrying the server's
* SetError type when it refuses the update - `notFound` once the
* subscription is gone, `forbidden` for an emailPush map naming an account
* the user may not subscribe to.
* filter (RFC 8620 §7.2.1). Throws a JMAPSetError carrying the SetError type
* when the server refuses the update - `notFound` once the subscription is
* gone, `forbidden` for an emailPush map naming an account the user may not
* subscribe to. An answer that neither updates nor refuses it is a plain
* JMAPMethodError: it says nothing about the subscription.
*/
export async function updatePushSubscription(
id: string,