mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 08:06:36 +00:00
fix: keep syncing when a shared account refuses access to its address books or calendars #34
Stalwart lists an account shared with the user for mail only with every capability, and answers `forbidden` when the user asks for its address books or calendars. The engine treated that like any failed request, so on the local test server every sync of usera ended with an I/O error and never reached the device, only because userb shares a mailbox. An account that refuses to list its collections (`forbidden`, `accountNotFound`, `accountNotSupportedByMethod`) is now left out of the run and reported as an item error; the other accounts sync as before. The rows written for such an account stay on the device: a refusal can pass, and dropping the account's contacts and events for one refused request would throw away device edits that are still waiting. Checked: three new engine tests (a shared account that never listed, one that stops listing its address books, one that stops listing its calendars), which fail without the fix, `npm run typecheck` and `npm test` on this commit, and a contacts sync of usera on the API 32 emulator, which now ends `ok` with one item error.
This commit is contained in:
@@ -9,6 +9,7 @@ import type { JmapInvocation, JmapResponse } from '../../types';
|
||||
import {
|
||||
addDeviceContact,
|
||||
addServerCards,
|
||||
CALENDAR_AUTHORITY,
|
||||
CONTACTS_AUTHORITY,
|
||||
createHarness,
|
||||
CrashError,
|
||||
@@ -608,3 +609,64 @@ describe('device sync engine: deletion guards', () => {
|
||||
expect(new CrashError('x').name).toBe('CrashError');
|
||||
});
|
||||
});
|
||||
|
||||
describe('device sync engine: accounts that refuse access', () => {
|
||||
// Stalwart lists an account shared for mail only with every capability,
|
||||
// and its /get calls answer `forbidden`.
|
||||
it('syncs the personal account when a shared one refuses to list its address books', async () => {
|
||||
const h = createHarness({ accounts: 'withShared' });
|
||||
addServerCards(h, ['Ada']);
|
||||
h.server.failNextMethod('AddressBook/get', { type: 'forbidden', description: 'You do not have access to account team' }, { accountId: 'team' });
|
||||
|
||||
const report = await h.run();
|
||||
|
||||
expect(report.outcome).toBe('ok');
|
||||
expect(report.itemErrors).toEqual([expect.objectContaining({ ref: 'team', side: 'download', type: 'forbidden' })]);
|
||||
expect(h.contactNamed('Ada')).toBeDefined();
|
||||
});
|
||||
|
||||
it('keeps the contacts of an account that stops listing its address books, and syncs it again once it does', async () => {
|
||||
const h = createHarness({ accounts: 'withShared' });
|
||||
const teamBook = h.server.all('AddressBook', 'team')[0].id as string;
|
||||
h.prefs.contactsSelection[`team/${teamBook}`] = true;
|
||||
const [mate] = addServerCards(h, ['Teammate'], 'team', teamBook);
|
||||
await h.run();
|
||||
expect(h.contactNamed('Teammate')).toBeDefined();
|
||||
|
||||
addServerCards(h, ['Ada']);
|
||||
h.server.serverUpdate('ContactCard', 'team', mate, { 'name/full': 'Team mate' });
|
||||
h.server.failNextMethod('AddressBook/get', { type: 'forbidden' }, { accountId: 'team' });
|
||||
const report = await h.run();
|
||||
|
||||
expect(report.outcome).toBe('ok');
|
||||
expect(h.contactNamed('Ada')).toBeDefined();
|
||||
expect(h.contactNamed('Teammate')).toBeDefined();
|
||||
|
||||
expect((await h.run()).itemErrors).toEqual([]);
|
||||
expect(h.contactNamed('Team mate')).toBeDefined();
|
||||
});
|
||||
|
||||
it('keeps the calendar and events of an account that refuses to list its calendars', async () => {
|
||||
const h = createHarness({ accounts: 'withShared' });
|
||||
const teamCalendar = h.server.all('Calendar', 'team')[0].id as string;
|
||||
h.prefs.calendarSelection[`team/${teamCalendar}`] = true;
|
||||
h.server.addEvent('team', {
|
||||
uid: 'uid-team',
|
||||
title: 'Team meeting',
|
||||
start: '2026-09-28T09:00:00',
|
||||
duration: 'PT1H',
|
||||
timeZone: 'Etc/UTC',
|
||||
calendarIds: { [teamCalendar]: true },
|
||||
});
|
||||
await h.run(CALENDAR_AUTHORITY);
|
||||
expect(h.events()).toHaveLength(1);
|
||||
|
||||
h.server.failNextMethod('Calendar/get', { type: 'forbidden' }, { accountId: 'team' });
|
||||
const report = await h.run(CALENDAR_AUTHORITY);
|
||||
|
||||
expect(report.outcome).toBe('ok');
|
||||
expect(report.itemErrors).toEqual([expect.objectContaining({ ref: 'team', type: 'forbidden' })]);
|
||||
expect(h.events()).toHaveLength(1);
|
||||
expect(h.device.rows('calendars').some((c) => c._sync_id === `team/${teamCalendar}`)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -219,7 +219,9 @@ export class CalendarSync extends ItemSync {
|
||||
this.readOnlyKeys.clear();
|
||||
const selected: Array<{ jmapAccountId: string; calendar: CalendarLike; readOnly: boolean; accountName: string }> = [];
|
||||
for (const account of this.env.accounts) {
|
||||
const { list, state } = await this.env.jmap.getContainers<CalendarLike>('Calendar', account.id, CALENDAR_PROPERTIES);
|
||||
const containers = await this.containersOf<CalendarLike>('Calendar', account.id, CALENDAR_PROPERTIES);
|
||||
if (!containers) continue;
|
||||
const { list, state } = containers;
|
||||
this.calendars.set(account.id, list);
|
||||
if (state) {
|
||||
this.collectionStates.set(account.id, state);
|
||||
|
||||
@@ -286,7 +286,9 @@ export class ContactsSync extends ItemSync {
|
||||
protected async collections(write: boolean): Promise<void> {
|
||||
this.syncedKeys.clear();
|
||||
for (const account of this.env.accounts) {
|
||||
const { list, state } = await this.env.jmap.getContainers<AddressBookLike>('AddressBook', account.id, ADDRESS_BOOK_PROPERTIES);
|
||||
const containers = await this.containersOf<AddressBookLike>('AddressBook', account.id, ADDRESS_BOOK_PROPERTIES);
|
||||
if (!containers) continue;
|
||||
const { list, state } = containers;
|
||||
this.books.set(account.id, list);
|
||||
for (const book of list) {
|
||||
if (isCollectionSelected(this.env.prefs.contactsSelection, account, CONTACTS_AUTHORITY, book)) {
|
||||
|
||||
@@ -66,6 +66,9 @@ type Removal = 'destroyed' | 'outside';
|
||||
/** Method-level refusals of a whole `/set` that concern the account, not the objects. */
|
||||
const ACCOUNT_REFUSALS = new Set(['accountReadOnly', 'forbidden', 'accountNotFound', 'accountNotSupportedByMethod']);
|
||||
|
||||
/** Refusals to list an account's collections: the account is not readable for this user. */
|
||||
const ACCOUNT_UNREADABLE = new Set(['forbidden', 'accountNotFound', 'accountNotSupportedByMethod']);
|
||||
|
||||
export abstract class ItemSync {
|
||||
/** Per account: object ids put on `stale` (true) or fetched fine again (false), for the next state op. */
|
||||
private readonly staleMarks = new Map<string, Map<string, boolean>>();
|
||||
@@ -75,6 +78,8 @@ export abstract class ItemSync {
|
||||
private readonly extraCreated = new Map<string, Set<string>>();
|
||||
/** Per account: `ifInState` of the next `/set` (the download's state, then our last `newState`). */
|
||||
private readonly setStates = new Map<string, string | null>();
|
||||
/** Accounts that refused to list their collections this run: left out of it (see `containersOf`). */
|
||||
private readonly unreadable = new Set<string>();
|
||||
|
||||
constructor(protected readonly env: RunEnv) {}
|
||||
|
||||
@@ -171,12 +176,37 @@ export abstract class ItemSync {
|
||||
}
|
||||
|
||||
protected accountIds(): string[] {
|
||||
return this.env.accounts.map((a) => a.id);
|
||||
return this.env.accounts.map((a) => a.id).filter((id) => !this.unreadable.has(id));
|
||||
}
|
||||
|
||||
/** The session's accounts plus accounts the rows were written for that are gone from it. */
|
||||
protected knownAccountIds(): string[] {
|
||||
return [...new Set([...this.accountIds(), ...Object.keys(this.env.store.committed.accounts)])];
|
||||
return [...new Set([...this.accountIds(), ...Object.keys(this.env.store.committed.accounts)])]
|
||||
.filter((id) => !this.unreadable.has(id));
|
||||
}
|
||||
|
||||
/**
|
||||
* One account's collections, or null when the account refuses to list them
|
||||
* (`forbidden`, `accountNotFound`, `accountNotSupportedByMethod`): Stalwart
|
||||
* lists an account shared for mail only with every capability, and access
|
||||
* can go away. Such an account is left out of the run and reported; the
|
||||
* rows written for it stay as they are, since a refusal can pass and must
|
||||
* not drop anything from the device.
|
||||
*/
|
||||
protected async containersOf<T>(
|
||||
type: 'AddressBook' | 'Calendar',
|
||||
accountId: string,
|
||||
properties: readonly string[],
|
||||
): Promise<{ list: T[]; state: string | null } | null> {
|
||||
try {
|
||||
return await this.env.jmap.getContainers<T>(type, accountId, properties);
|
||||
} catch (error) {
|
||||
if (!isMethodError(error) || !ACCOUNT_UNREADABLE.has(error.type)) throw error;
|
||||
this.unreadable.add(accountId);
|
||||
this.env.report.itemError({ ref: accountId, side: 'download', type: error.type, description: error.message });
|
||||
this.env.log(`account ${accountId} left out: ${error.type}`);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
// ── State ──
|
||||
|
||||
Reference in New Issue
Block a user