Fix: stop re-probing shared accounts without calendar access
The calendar fan-out probes every shared/group account on suspicion,
because Stalwart does not always advertise calendar capability on group
accounts. A shared account that grants no calendar access at all rejects
that probe - and did so again on every calendar interaction: each range
change re-queried the account and logged a red console error
("You do not have access to account X") while working fine otherwise.
Remember the rejection instead: the thrown query error now carries the
JMAP error type, an access rejection for a probed secondary account is
logged once at debug level, and both fan-out loops (events and calendar
lists) skip the account for the rest of the session. Genuine failures on
the primary account keep the error log. Two regression tests cover the
probe-once behavior and the calendar-list skip.
This commit is contained in:
@@ -0,0 +1,97 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { JMAPClient } from '../jmap/client';
|
||||
|
||||
// The calendar fan-out probes shared accounts on suspicion (Stalwart does not
|
||||
// always advertise calendar capability on group accounts), so a shared account
|
||||
// without any calendar access answers every probe with an access rejection.
|
||||
// That rejection must be remembered and the account skipped afterwards -
|
||||
// before, every calendar interaction re-probed it and logged a console error.
|
||||
|
||||
function makeSession() {
|
||||
return {
|
||||
capabilities: { 'urn:ietf:params:jmap:core': {} },
|
||||
accounts: {
|
||||
'acct-1': { name: 'test', isPersonal: true, accountCapabilities: {} },
|
||||
// Shared account without calendar access: probed because it is
|
||||
// non-personal, rejected by the server.
|
||||
'ev': { name: 'shared', isPersonal: false, accountCapabilities: {} },
|
||||
},
|
||||
primaryAccounts: { 'urn:ietf:params:jmap:mail': 'acct-1' },
|
||||
apiUrl: 'https://mail.example.com/jmap/api',
|
||||
downloadUrl: 'https://mail.example.com/jmap/download/{accountId}/{blobId}/{name}',
|
||||
uploadUrl: 'https://mail.example.com/jmap/upload/{accountId}/',
|
||||
eventSourceUrl: '',
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(body: unknown): Response {
|
||||
return new Response(JSON.stringify(body), {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
});
|
||||
}
|
||||
|
||||
describe('calendar fan-out to shared accounts without access', () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn>;
|
||||
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
|
||||
let client: JMAPClient;
|
||||
|
||||
beforeEach(async () => {
|
||||
fetchSpy = vi.spyOn(globalThis, 'fetch');
|
||||
fetchSpy.mockResolvedValueOnce(jsonResponse(makeSession()));
|
||||
client = new JMAPClient('https://mail.example.com', 'user@test.com', 'pass123');
|
||||
await client.connect();
|
||||
fetchSpy.mockReset();
|
||||
|
||||
fetchSpy.mockImplementation(async (_url: RequestInfo | URL, init?: RequestInit) => {
|
||||
const body = JSON.parse(String(init?.body ?? '{}'));
|
||||
const [method, args] = body.methodCalls?.[0] ?? [];
|
||||
if (args?.accountId === 'ev') {
|
||||
return jsonResponse({
|
||||
methodResponses: [
|
||||
['error', { type: 'accountNotFound', description: 'You do not have access to account ev' }, '0'],
|
||||
],
|
||||
});
|
||||
}
|
||||
if (method === 'CalendarEvent/query') {
|
||||
return jsonResponse({ methodResponses: [['CalendarEvent/query', { ids: [] }, '0']] });
|
||||
}
|
||||
return jsonResponse({ methodResponses: [['Calendar/get', { list: [] }, '0']] });
|
||||
});
|
||||
consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
client.disconnect();
|
||||
consoleErrorSpy.mockRestore();
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
function callsForAccount(accountId: string): unknown[][] {
|
||||
return fetchSpy.mock.calls.filter((call: unknown[]) => {
|
||||
const body = (call[1] as RequestInit | undefined)?.body;
|
||||
return typeof body === 'string' && body.includes(`"accountId":"${accountId}"`);
|
||||
});
|
||||
}
|
||||
|
||||
it('probes a no-access shared account once, then skips it without console noise', async () => {
|
||||
await client.queryAllCalendarEvents({});
|
||||
await client.queryAllCalendarEvents({});
|
||||
|
||||
expect(callsForAccount('ev')).toHaveLength(1);
|
||||
// The primary account keeps being queried normally.
|
||||
expect(callsForAccount('acct-1')).toHaveLength(2);
|
||||
// An expected rejection is not an error worth red console output.
|
||||
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('getAllCalendars skips an account already known to be inaccessible', async () => {
|
||||
await client.queryAllCalendarEvents({});
|
||||
fetchSpy.mockClear();
|
||||
|
||||
await client.getAllCalendars();
|
||||
|
||||
expect(callsForAccount('ev')).toHaveLength(0);
|
||||
expect(callsForAccount('acct-1')).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
+25
-1
@@ -4547,6 +4547,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
for (const accountId of accountIds) {
|
||||
const isPrimary = accountId === primaryId;
|
||||
if (!isPrimary && this.calendarAccessDenied.has(accountId)) continue;
|
||||
const account = this.accounts[accountId];
|
||||
|
||||
try {
|
||||
@@ -4753,6 +4754,11 @@ export class JMAPClient implements IJMAPClient {
|
||||
.map((event) => normalizeCalendarEventLike(event));
|
||||
}
|
||||
|
||||
// Shared accounts the server rejected calendar access for - probed once,
|
||||
// then skipped for the rest of the session (see getCalendarCapableAccountIds
|
||||
// for why the fan-out has to probe on suspicion).
|
||||
private calendarAccessDenied = new Set<string>();
|
||||
|
||||
async queryAllCalendarEvents(
|
||||
filter: CalendarEventFilter,
|
||||
sort?: Array<{ property: string; isAscending: boolean }>,
|
||||
@@ -4765,6 +4771,7 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
for (const accountId of accountIds) {
|
||||
const isPrimary = accountId === primaryId;
|
||||
if (!isPrimary && this.calendarAccessDenied.has(accountId)) continue;
|
||||
const account = this.accounts[accountId];
|
||||
|
||||
try {
|
||||
@@ -4830,7 +4837,12 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
if (queryResponse.methodResponses?.[0]?.[0] === "error") {
|
||||
const error = queryResponse.methodResponses[0][1];
|
||||
throw new Error(error?.description || error?.type || "CalendarEvent/query failed");
|
||||
// Keep the JMAP error type so the catch below can tell an expected
|
||||
// access rejection apart from a genuine failure.
|
||||
throw Object.assign(
|
||||
new Error(error?.description || error?.type || "CalendarEvent/query failed"),
|
||||
{ jmapErrorType: error?.type },
|
||||
);
|
||||
}
|
||||
|
||||
const ids: string[] = queryResponse.methodResponses?.[0]?.[1]?.ids || [];
|
||||
@@ -4874,6 +4886,18 @@ export class JMAPClient implements IJMAPClient {
|
||||
|
||||
return filtered;
|
||||
} catch (error) {
|
||||
// The fan-out over shared accounts probes on suspicion (see
|
||||
// getCalendarCapableAccountIds) and may hit accounts that grant no
|
||||
// calendar access at all. Remember the rejection and go quiet instead
|
||||
// of re-probing - and re-logging - on every range change.
|
||||
const type = (error as { jmapErrorType?: string } | null)?.jmapErrorType;
|
||||
const denied = type === 'forbidden' || type === 'accountNotFound' ||
|
||||
/not have access/i.test(error instanceof Error ? error.message : '');
|
||||
if (targetAccountId && denied) {
|
||||
this.calendarAccessDenied.add(targetAccountId);
|
||||
debug.log('calendar', `No calendar access to account ${targetAccountId} - skipping it from now on`);
|
||||
return [];
|
||||
}
|
||||
console.error('Failed to query calendar events:', error);
|
||||
return [];
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user