fix: list contacts and files only for shared accounts that support them (bulwarkmail/webmail#1239)

src/api/__tests__/contacts.test.ts

@@ -1,6 +1,7 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 
 const session = {
+  capabilities: { 'urn:ietf:params:jmap:contacts': {} } as Record<string, unknown>,
   primaryAccounts: {} as Record<string, string>,
   accounts: {} as Record<string, { name: string; isPersonal: boolean; accountCapabilities?: Record<string, unknown> }>,
 };
@@ -60,6 +61,15 @@ describe('contacts operations', () => {
       };
       expect(getContactCapableAccountIds()).toEqual(['acc-1', 'acc-team', 'acc-ct']);
     });
+
+    it('leaves out a shared account that lists its capabilities without contacts (bulwarkmail/webmail#1239)', () => {
+      session.accounts = {
+        'acc-1': { name: 'me', isPersonal: true },
+        'acc-mail': { name: 'Mail only', isPersonal: false, accountCapabilities: { 'urn:ietf:params:jmap:mail': {} } },
+        'acc-team': { name: 'Team', isPersonal: false, accountCapabilities: { 'urn:ietf:params:jmap:contacts': {} } },
+      };
+      expect(getContactCapableAccountIds()).toEqual(['acc-1', 'acc-team']);
+    });
   });
 
   describe('getAddressBooks', () => {

src/api/__tests__/files.test.ts

@@ -60,8 +60,12 @@ const mockMaxObjectsInGet = jmapClient.getMaxObjectsInGet as ReturnType<typeof v
 const mockMaxCallsInRequest = jmapClient.getMaxCallsInRequest as ReturnType<typeof vi.fn>;
 const mockMaxObjectsInSet = jmapClient.getMaxObjectsInSet as ReturnType<typeof vi.fn>;
 
+// A session advertises FileNode unless the test says otherwise.
 function setSession(session: unknown) {
-  (jmapClient as { currentSession: unknown }).currentSession = session;
+  (jmapClient as { currentSession: unknown }).currentSession =
+    session && typeof session === 'object'
+      ? { capabilities: { [CAPABILITIES.FILES]: {} }, ...session }
+      : session;
 }
 
 beforeEach(() => {
@@ -268,12 +272,15 @@ describe('getAllFileNodesAcrossAccounts', () => {
     expect(child.parentId).toBe('acc-2:s1');
   });
 
-  it('skips personal accounts without the filenode capability', async () => {
+  it('skips accounts that list their capabilities without filenode (bulwarkmail/webmail#1239)', async () => {
     setSession({
+      capabilities: { [CAPABILITIES.FILES]: {}, [CAPABILITIES.MAIL]: {} },
       primaryAccounts: { [CAPABILITIES.FILES]: 'acc-1' },
       accounts: {
         'acc-1': { name: 'me@example.com', isPersonal: true },
-        'acc-3': { name: 'other personal', isPersonal: true },
+        'acc-3': { name: 'other personal', isPersonal: true, accountCapabilities: { [CAPABILITIES.MAIL]: {} } },
+        // A shared account used to count as capable whatever it listed.
+        'acc-4': { name: 'Team', isPersonal: false, accountCapabilities: { [CAPABILITIES.MAIL]: {} } },
       },
     });
     mockRequest.mockResolvedValue({
@@ -1177,7 +1184,10 @@ describe('FileNode/set refused by the server', () => {
 describe('accountSupportsFiles (#563)', () => {
   const caps = { [CAPABILITIES.FILES]: {} };
   it('requires the account capability on personal accounts', () => {
-    expect(accountSupportsFiles({ name: 'me', isPersonal: true, isReadOnly: false }, caps)).toBe(false);
+    expect(accountSupportsFiles(
+      { name: 'me', isPersonal: true, isReadOnly: false, accountCapabilities: { [CAPABILITIES.MAIL]: {} } },
+      caps,
+    )).toBe(false);
     expect(accountSupportsFiles(
       { name: 'me', isPersonal: true, isReadOnly: false, accountCapabilities: { [CAPABILITIES.FILES]: {} } },
       caps,
@@ -1189,6 +1199,17 @@ describe('accountSupportsFiles (#563)', () => {
     expect(accountSupportsFiles({ name: 'grp', isPersonal: false, isReadOnly: false }, {})).toBe(false);
     expect(accountSupportsFiles(undefined, caps)).toBe(false);
   });
+
+  it('leaves out a shared account that lists its capabilities without filenode (bulwarkmail/webmail#1239)', () => {
+    expect(accountSupportsFiles(
+      { name: 'grp', isPersonal: false, isReadOnly: false, accountCapabilities: { [CAPABILITIES.MAIL]: {} } },
+      caps,
+    )).toBe(false);
+    expect(accountSupportsFiles(
+      { name: 'grp', isPersonal: false, isReadOnly: false, accountCapabilities: {} },
+      caps,
+    )).toBe(true);
+  });
 });
 
 describe('supportsSharing', () => {

src/api/contacts.ts

@@ -4,6 +4,7 @@ import { CAPABILITIES } from './types';
 import type { ContactCard, ContactMedia, AddressBook } from './types';
 import { generateUUID } from '../lib/uuid';
 import { contactFromWire, contactToWire } from '../lib/contact-wire';
+import { accountSupports } from '../lib/account-capability';
 
 const USING = [CAPABILITIES.CORE, CAPABILITIES.CONTACTS];
 
@@ -55,18 +56,19 @@ export function getContactsAccountId(): string {
 
 /**
  * Every account that may hold address books: the contacts primary first, then
- * accounts that advertise the contacts capability or are non-personal (shared
- * / group) accounts - Stalwart doesn't always advertise capabilities on group
- * accounts even when they have contact resources. Mirrors the webmail's
- * getContactCapableAccountIds().
+ * the accounts the user may use contacts with (lib/account-capability: a
+ * shared account that lists its capabilities without contacts is left out,
+ * one that lists none falls back to the server-wide capability, webmail
+ * #1239). Mirrors the webmail's getContactCapableAccountIds().
  */
 export function getContactCapableAccountIds(): string[] {
   const primaryId = getContactsAccountId();
-  const accounts = jmapClient.currentSession?.accounts ?? {};
+  const session = jmapClient.currentSession;
+  const accounts = session?.accounts ?? {};
   const out: string[] = [];
   for (const [id, account] of Object.entries(accounts)) {
     if (id === primaryId) continue;
-    if (account.accountCapabilities?.[CAPABILITIES.CONTACTS] || account.isPersonal === false) {
+    if (accountSupports(account, session?.capabilities, CAPABILITIES.CONTACTS)) {
       out.push(id);
     }
   }

src/api/files.ts

@@ -6,6 +6,7 @@ import { batched, JMAPSetError, requireMethodResult } from './jmap-result';
 import { decodeFileNodeName, getUniqueName } from '../lib/filenode-name';
 import { acceptedFileName, fileNameRulesFrom, type FileNameRules } from '../lib/file-name-rules';
 import { mediaTypeEssence } from '../lib/media-type';
+import { accountSupports } from '../lib/account-capability';
 
 /**
  * What a FileNode is: a file, a folder or a symlink.
@@ -119,16 +120,15 @@ export function filesAccountId(): string {
 // capability. A server can advertise urn:ietf:params:jmap:filenode while a
 // specific account has its jmap-file-node-* permissions revoked, in which case
 // the capability is absent from that account's accountCapabilities and every
-// FileNode action fails with an authorization error (#563). Non-personal
-// (shared/group) accounts don't always advertise per-account, so treat those
-// as capable. Mirrors the webmail's `supportsFiles`.
+// FileNode action fails with an authorization error (#563). A shared account
+// that lists no capabilities at all falls back to the server-wide one; one
+// that lists them without FileNode is not capable (lib/account-capability,
+// webmail #1239). Mirrors the webmail's `supportsFiles`.
 export function accountSupportsFiles(
   account: JMAPAccountInfo | undefined,
   sessionCapabilities: Record<string, unknown> | undefined,
 ): boolean {
-  if (!sessionCapabilities || !(CAPABILITIES.FILES in sessionCapabilities)) return false;
-  if (!account) return false;
-  return account.accountCapabilities?.[CAPABILITIES.FILES] != null || !account.isPersonal;
+  return accountSupports(account, sessionCapabilities, CAPABILITIES.FILES);
 }
 
 export function supportsFiles(): boolean {
@@ -318,15 +318,13 @@ export async function getAllFileNodes(): Promise<FileNode[]> {
 }
 
 // Accounts (primary + shared/group) that can hold FileNodes: any non-primary
-// account that advertises the filenode capability or is non-personal, since
-// Stalwart doesn't always advertise capabilities on shared accounts.
-function filesCapableAccountIds(): string[] {
+// account the user may use FileNode with (accountSupportsFiles).
+export function filesCapableAccountIds(): string[] {
   const primaryId = filesAccountId();
-  const accounts = jmapClient.currentSession?.accounts ?? {};
+  const session = jmapClient.currentSession;
+  const accounts = session?.accounts ?? {};
   const rest = Object.entries(accounts)
-    .filter(([id, info]) =>
-      id !== primaryId &&
-      (info.accountCapabilities?.[CAPABILITIES.FILES] != null || !info.isPersonal))
+    .filter(([id, info]) => id !== primaryId && accountSupportsFiles(info, session?.capabilities))
     .map(([id]) => id);
   return [primaryId, ...rest];
 }