diff --git a/src/api/__tests__/contacts.test.ts b/src/api/__tests__/contacts.test.ts index c919cf4a..c812c3c7 100644 --- a/src/api/__tests__/contacts.test.ts +++ b/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, primaryAccounts: {} as Record, accounts: {} as Record }>, }; @@ -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', () => { diff --git a/src/api/__tests__/files.test.ts b/src/api/__tests__/files.test.ts index 5bcad0d8..99f0e0ec 100644 --- a/src/api/__tests__/files.test.ts +++ b/src/api/__tests__/files.test.ts @@ -60,8 +60,12 @@ const mockMaxObjectsInGet = jmapClient.getMaxObjectsInGet as ReturnType; const mockMaxObjectsInSet = jmapClient.getMaxObjectsInSet as ReturnType; +// 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', () => { diff --git a/src/api/contacts.ts b/src/api/contacts.ts index 5330c0ec..826a1748 100644 --- a/src/api/contacts.ts +++ b/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); } } diff --git a/src/api/files.ts b/src/api/files.ts index 64c9c02a..806bcd1e 100644 --- a/src/api/files.ts +++ b/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 | 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 { } // 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]; }