Merge pull request #677 from dealerweb/fix/calendar-fanout-access-denied

Fix: stop re-probing shared accounts without calendar access

lib/__tests__/jmap-calendar-access.test.ts

@@ -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);
+  });
+});

lib/jmap/client.ts

@@ -4566,6 +4566,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 {
@@ -4772,6 +4773,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 }>,
@@ -4784,6 +4790,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 {
@@ -4849,7 +4856,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 || [];
@@ -4893,6 +4905,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 [];
     }