From 31d8e4a4b76339f1cef785a518d512e0a3bd77f5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:40:42 +0100 Subject: [PATCH] feat: send and save drafts from a group or shared identity in that account --- src/api/__tests__/send-account.test.ts | 152 +++++++++++++++++++++++++ src/api/email.ts | 21 +++- src/api/identity.ts | 112 +++++++++++++++++- src/lib/compose-send-account.ts | 56 +++++++++ src/screens/ComposeScreen.tsx | 43 ++++++- 5 files changed, 373 insertions(+), 11 deletions(-) create mode 100644 src/api/__tests__/send-account.test.ts create mode 100644 src/lib/compose-send-account.ts diff --git a/src/api/__tests__/send-account.test.ts b/src/api/__tests__/send-account.test.ts new file mode 100644 index 00000000..9671e0fc --- /dev/null +++ b/src/api/__tests__/send-account.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// #1090: a message From a group identity is created, submitted and filed in +// the group's account. Stalwart lists the group's send-as identity among the +// member's own too, so the member's list alone sent it through the member's +// account and filed the Sent copy there. + +const { client } = vi.hoisted(() => ({ + client: { + accountId: 'me', + serverUrl: 'https://mail.example.com', + username: 'me@example.com', + request: vi.fn(), + getSubmissionAccountIds: vi.fn(() => ['me', 'team']), + currentSession: { accounts: { me: { name: 'me@example.com' }, team: { name: 'Team' } } } as Record, + getAccountName: vi.fn((id: string) => id), + hasAccountCapability: vi.fn(() => false), + getMaxCallsInRequest: vi.fn(() => 16), + getMaxObjectsInGet: vi.fn(() => 500), + getMaxObjectsInSet: vi.fn(() => 500), + }, +})); +vi.mock('../jmap-client', () => ({ jmapClient: client })); + +import { resetSendIdentities, resolveSendAccountId, sendIdentityIn } from '../identity'; +import { saveDraftVersion, sendEmail } from '../email'; +import { sendTargetFor } from '../../lib/compose-send-account'; +import type { Identity, Mailbox } from '../types'; + +const ME = { id: 'i-me', name: 'Me', email: 'me@example.com', mayDelete: false } as Identity; +// The member's list mirrors the group's identity, with the member's id for it. +const MIRRORED = { id: 'i-me-team', name: 'Team', email: 'team@example.com', mayDelete: false } as Identity; +const TEAM = { id: 'i-team', name: 'Team', email: 'team@example.com', mayDelete: false } as Identity; + +function identitiesAnswer(calls: Array<[string, { accountId: string }, string]>) { + return { + methodResponses: calls.map(([name, args, callId]) => { + if (name !== 'Identity/get') throw new Error(`unexpected ${name}`); + const list = args.accountId === 'team' ? [TEAM] : [ME, MIRRORED]; + return ['Identity/get', { accountId: args.accountId, list }, callId]; + }), + }; +} + +function mailbox(role: string, id: string, accountId?: string): Mailbox { + return { + id: accountId ? `${accountId}:${id}` : id, originalId: accountId ? id : undefined, role, name: role, + accountId, isShared: !!accountId, + } as Mailbox; +} +const MAILBOXES = [ + mailbox('drafts', 'own-drafts'), mailbox('sent', 'own-sent'), + mailbox('drafts', 'team-drafts', 'team'), mailbox('sent', 'team-sent', 'team'), +]; + +beforeEach(() => { + vi.clearAllMocks(); + resetSendIdentities(); + client.getSubmissionAccountIds.mockReturnValue(['me', 'team']); + client.request.mockImplementation(async (calls) => identitiesAnswer(calls)); +}); + +describe('resolveSendAccountId', () => { + it("sends a group identity's message from the group account, not the member's mirror", async () => { + await expect(resolveSendAccountId('team@example.com')).resolves.toBe('team'); + await expect(resolveSendAccountId('Team@Example.com')).resolves.toBe('team'); + }); + + it('keeps the own address, and a sub-address of it, in the own account', async () => { + await expect(resolveSendAccountId('me@example.com')).resolves.toBe('me'); + await expect(resolveSendAccountId('me+news@example.com')).resolves.toBe('me'); + }); + + it('takes a sub-address of the group address for the group', async () => { + await expect(resolveSendAccountId('team+sales@example.com')).resolves.toBe('team'); + }); + + it('asks nothing when there is no group or shared account', async () => { + client.getSubmissionAccountIds.mockReturnValue(['me']); + await expect(resolveSendAccountId('team@example.com')).resolves.toBe('me'); + expect(client.request).not.toHaveBeenCalled(); + }); + + it('reads the identities once for a burst of saves', async () => { + await resolveSendAccountId('team@example.com'); + await resolveSendAccountId('team@example.com'); + expect(client.request).toHaveBeenCalledTimes(1); + }); + + it("names the group account's own identity for the address", async () => { + await expect(sendIdentityIn('team', 'team@example.com')).resolves.toMatchObject({ id: 'i-team' }); + }); +}); + +describe('sendTargetFor', () => { + it("submits a group identity's message through the group's identity and files it in the group's folders", async () => { + await expect(sendTargetFor(MIRRORED, 'team@example.com', MAILBOXES)).resolves.toEqual({ + accountId: 'team', identityId: 'i-team', draftsMailboxId: 'team-drafts', sentMailboxId: 'team-sent', + }); + }); + + it('keeps an own message in the own folders', async () => { + await expect(sendTargetFor(ME, 'me@example.com', MAILBOXES)).resolves.toEqual({ + identityId: 'i-me', draftsMailboxId: 'own-drafts', sentMailboxId: 'own-sent', + }); + }); +}); + +describe('a draft that moves between accounts', () => { + const OUTGOING = { from: [{ email: 'team@example.com' }], to: [{ email: 'you@example.net' }], subject: 'Hi', textBody: 'Hi' }; + + it("destroys the previous version in the account it was saved in, never one with the same id in the new one", async () => { + client.request.mockReset(); + client.request + .mockResolvedValueOnce({ + methodResponses: [ + ['Email/set', { created: { draft: { id: 'd7' } } }, '0'], + ['Email/get', { list: [{ id: 'd7', attachments: [] }] }, '1'], + ], + }) + .mockResolvedValueOnce({ methodResponses: [['Email/set', { destroyed: ['d7'] }, '0']] }); + + // The own Drafts held version d7; the From is now the group's, whose + // account has a message d7 of its own. + await saveDraftVersion(OUTGOING, 'team-drafts', 'd7', 'team', 'me'); + + const [create] = client.request.mock.calls[0][0]; + expect(create[1].accountId).toBe('team'); + const [destroy] = client.request.mock.calls[1][0]; + expect(destroy[1]).toMatchObject({ accountId: 'me', destroy: ['d7'] }); + }); + + it('removes the old draft from its own account once the group send went out', async () => { + client.request.mockReset(); + client.request + .mockResolvedValueOnce({ + methodResponses: [ + ['Email/set', { created: { draft: { id: 'e1' } } }, '0'], + ['EmailSubmission/set', { created: { 'sub-1': { id: 's1' } } }, '1'], + ], + }) + .mockResolvedValueOnce({ methodResponses: [['Email/set', { destroyed: ['d7'] }, '0']] }); + + await sendEmail(OUTGOING, 'i-team', 'team-sent', undefined, { + draftsMailboxId: 'team-drafts', draftId: 'd7', draftAccountId: 'me', accountId: 'team', + }); + + expect(client.request.mock.calls[0][0][0][1].accountId).toBe('team'); + expect(client.request.mock.calls[0][0][1][1].accountId).toBe('team'); + expect(client.request.mock.calls[1][0][0][1]).toMatchObject({ accountId: 'me', destroy: ['d7'] }); + }); +}); diff --git a/src/api/email.ts b/src/api/email.ts index 0e60768a..61ba9fe8 100644 --- a/src/api/email.ts +++ b/src/api/email.ts @@ -1672,6 +1672,12 @@ export interface SendEmailOptions { draftsMailboxId?: string; /** Previous draft version to destroy once the submission succeeded (#849). */ draftId?: string; + /** + * Account `draftId` lives in, when not the submitting one: a draft saved + * in the own Drafts and sent from a group identity, or the other way + * round. Email ids repeat across accounts. + */ + draftAccountId?: string; /** Submitting account (shared/group account); defaults to the primary. */ accountId?: string; } @@ -1875,11 +1881,12 @@ export async function sendEmail( // rather than a failed send (#849). The submission record rides along, and // nothing about it may turn into a warning. if (opts?.draftId && emailSubmissionId) { + const draftAccountId = opts.draftAccountId ?? accountId; try { if (recordToDestroy) { const cleanup = await jmapClient.request( [ - ['Email/set', { accountId, destroy: [opts.draftId] }, '0'], + ['Email/set', { accountId: draftAccountId, destroy: [opts.draftId] }, '0'], submissionDestroyCall([recordToDestroy], accountId, '1'), ], SUBMISSION_USING, @@ -1887,7 +1894,7 @@ export async function sendEmail( readSubmissionDestroy(cleanup, '1'); assertEmailsDestroyed(requireMethodResult(cleanup, '0', 'Email/set')); } else { - await destroyEmails([opts.draftId], accountId); + await destroyEmails([opts.draftId], draftAccountId); } } catch (err) { filingWarning = filingWarning ?? `old draft cleanup failed: ${err instanceof Error ? err.message : String(err)}`; @@ -1936,6 +1943,11 @@ export async function saveDraftVersion( draftsMailboxId: string, previousDraftId?: string, accountIdOverride?: string, + /** + * Account the previous version lives in, when not this one: the draft + * moves between the own Drafts and a group's when its From does (#1090). + */ + previousDraftAccountId?: string, ): Promise { const accountId = accountIdOverride ?? jmapClient.accountId; const emailCreate = buildEmailCreate(email); @@ -1968,9 +1980,10 @@ export async function saveDraftVersion( } catch (err) { console.warn('[email] could not read back the saved draft parts:', err); } - if (previousDraftId && previousDraftId !== id) { + const previousAccountId = previousDraftAccountId ?? accountId; + if (previousDraftId && (previousDraftId !== id || previousAccountId !== accountId)) { try { - await destroyEmails([previousDraftId], accountId); + await destroyEmails([previousDraftId], previousAccountId); } catch (err) { console.warn('[email] failed to destroy previous draft version:', err); } diff --git a/src/api/identity.ts b/src/api/identity.ts index fb7f1f2a..c9c7fcd4 100644 --- a/src/api/identity.ts +++ b/src/api/identity.ts @@ -1,7 +1,7 @@ import { jmapClient } from './jmap-client'; import { assertSetResult, requireMethodResult } from './jmap-result'; import { CAPABILITIES } from './types'; -import type { Identity } from './types'; +import type { Identity, JMAPMethodCall } from './types'; import { sanitizeDisplayName } from '../lib/rfc5322-mailbox'; const USING = [CAPABILITIES.CORE, CAPABILITIES.SUBMISSION]; @@ -60,3 +60,113 @@ export async function deleteIdentity(id: string): Promise { ); assertSetResult(requireMethodResult(res, '0', 'Identity/set'), [id], 'identity'); } + +/** An address the identity matches exactly. */ +function identityEmailMatchesExactly(identityEmail: string | undefined, from: string): boolean { + return !!identityEmail && identityEmail.toLowerCase() === from; +} + +/** An identity address `from` is a sub-address of (team+tag@ for team@, with the common "+" / "-" delimiters). */ +function identityEmailIsSubAddressOf(identityEmail: string | undefined, from: string): boolean { + if (!identityEmail) return false; + const [idLocal, idDomain] = identityEmail.toLowerCase().split('@'); + const [fromLocal, fromDomain] = from.split('@'); + if (!idDomain || !fromDomain || idDomain !== fromDomain || idLocal === '*') return false; + return fromLocal.startsWith(`${idLocal}+`) || fromLocal.startsWith(`${idLocal}-`); +} + +/** A wildcard identity for the domain of `from` (RFC 8621 ยง6, "*@example.com"). */ +function identityEmailIsWildcardFor(identityEmail: string | undefined, from: string): boolean { + if (!identityEmail) return false; + const [idLocal, idDomain] = identityEmail.toLowerCase().split('@'); + return idLocal === '*' && !!idDomain && idDomain === from.split('@')[1]; +} + +const IDENTITY_MATCHERS = [identityEmailMatchesExactly, identityEmailIsSubAddressOf, identityEmailIsWildcardFor]; + +// Draft autosave resolves on every save; identities rarely change. +let sendIdentitiesCache: { key: string; byAccount: Array<[string, Identity[]]>; expires: number } | null = null; + +/** Forget the identities resolveSendAccountId read (tests, a switch of logins). */ +export function resetSendIdentities(): void { + sendIdentitiesCache = null; +} + +/** + * The identities of every account that can send, the delegated (group or + * shared) ones first and the login's own last. Null when there is no + * delegated account, or the identities can't be read. + */ +async function identitiesOfSendAccounts(): Promise | null> { + const own = jmapClient.accountId; + const delegated = jmapClient.getSubmissionAccountIds().filter((id) => id !== own); + if (delegated.length === 0) return null; + const candidates = [...delegated, own]; + // The server and login are part of the key: account ids repeat across them. + const key = [jmapClient.serverUrl, jmapClient.username, ...candidates].join('\u0000'); + if (sendIdentitiesCache && sendIdentitiesCache.key === key && sendIdentitiesCache.expires > Date.now()) { + return sendIdentitiesCache.byAccount; + } + try { + const res = await jmapClient.request( + candidates.map((accountId, i) => ['Identity/get', { accountId }, String(i)] as JMAPMethodCall), + USING, + ); + const byAccount = candidates.map((accountId, i): [string, Identity[]] => { + const [name, body] = res.methodResponses.find(([, , callId]) => callId === String(i)) ?? []; + return [accountId, name === 'Identity/get' ? ((body?.list ?? []) as Identity[]) : []]; + }); + sendIdentitiesCache = { key, byAccount, expires: Date.now() + 60_000 }; + return byAccount; + } catch (err) { + console.warn('[identity] could not read the identities to pick the sending account', err); + return null; + } +} + +/** + * The account a message sent From `fromEmail` belongs to: where it is + * created, submitted and filed in Sent (webmail resolveSendAccountId, + * #1090). Stalwart lists a group's send-as identity among the member's own + * identities too, so going by the identity list alone sent the message + * through the member's account and filed its Sent copy there. + * + * Order: an account named exactly `fromEmail`; then an account that owns an + * identity for the address - exact, then sub-address, then wildcard, and a + * delegated (group/shared) account before the own one, since the own one + * may merely mirror the group's identity; else the own account. A + * sub-address of the member's own address stays theirs even when a shared + * catch-all account has a wildcard identity for the domain. + */ +export async function resolveSendAccountId(fromEmail?: string): Promise { + const own = jmapClient.accountId; + const from = fromEmail?.trim().toLowerCase(); + if (!from) return own; + const accounts = jmapClient.currentSession?.accounts ?? {}; + const named = Object.keys(accounts).find((id) => accounts[id]?.name?.toLowerCase() === from); + if (named) return named; + const byAccount = await identitiesOfSendAccounts(); + if (!byAccount) return own; + for (const matches of IDENTITY_MATCHERS) { + const hit = byAccount.find(([, list]) => list.some((identity) => matches(identity.email, from))); + if (hit) return hit[0]; + } + return own; +} + +/** + * The identity of `accountId` a message From `fromEmail` is submitted + * through: identity ids are per account, so the one the composer picked + * from the login's list names nothing (or another identity) in a group + * account. Undefined when the account has none for the address. + */ +export async function sendIdentityIn(accountId: string, fromEmail: string): Promise { + const from = fromEmail.trim().toLowerCase(); + const byAccount = await identitiesOfSendAccounts(); + const list = byAccount?.find(([id]) => id === accountId)?.[1] ?? await getIdentities(accountId); + for (const matches of IDENTITY_MATCHERS) { + const hit = list.find((identity) => matches(identity.email, from)); + if (hit) return hit; + } + return undefined; +} diff --git a/src/lib/compose-send-account.ts b/src/lib/compose-send-account.ts new file mode 100644 index 00000000..9b1e4357 --- /dev/null +++ b/src/lib/compose-send-account.ts @@ -0,0 +1,56 @@ +// Where the composer creates, submits and files a message: the account its +// From belongs to (#1090). A group or shared identity sends from the group's +// account, with that account's identity id, and its draft and Sent copy live +// in the group's Drafts and Sent; everything else uses the login's own. + +import { getMailboxes } from '../api/email'; +import { jmapClient } from '../api/jmap-client'; +import { resolveSendAccountId, sendIdentityIn } from '../api/identity'; +import type { Identity, Mailbox } from '../api/types'; +import { mailboxesOfAccount } from './mailbox-tree'; + +export interface SendTarget { + /** The JMAP account; undefined for the login's own. */ + accountId?: string; + /** The identity to submit through, as that account names it. */ + identityId: string; + draftsMailboxId?: string; + sentMailboxId?: string; +} + +const roleIn = (list: Mailbox[], role: string) => { + const box = list.find((m) => m.role === role); + return box ? (box.originalId ?? box.id) : undefined; +}; + +/** + * The account, identity and folders for a message From `fromEmail` that the + * composer picked `identity` (one of the login's own) for. `mailboxes` are + * the folders the app has loaded, the shared ones among them; a group's are + * read from the server when they are not. + */ +export async function sendTargetFor(identity: Identity, fromEmail: string, mailboxes: Mailbox[]): Promise { + const ownTarget: SendTarget = { + identityId: identity.id, + draftsMailboxId: roleIn(mailboxesOfAccount(mailboxes, undefined), 'drafts'), + sentMailboxId: roleIn(mailboxesOfAccount(mailboxes, undefined), 'sent'), + }; + const accountId = await resolveSendAccountId(fromEmail); + if (accountId === jmapClient.accountId) return ownTarget; + // Submitting needs the account's own identity for the address; without + // one, the message goes out of the login's account as before. + const accountIdentity = await sendIdentityIn(accountId, fromEmail); + if (!accountIdentity) return ownTarget; + let scope = mailboxesOfAccount(mailboxes, accountId); + if (!scope.some((m) => m.role === 'sent')) { + try { + scope = await getMailboxes(accountId); + } catch (err) { + console.warn('[compose] could not read the folders of the sending account', err); + return ownTarget; + } + } + const sentMailboxId = roleIn(scope, 'sent'); + if (!sentMailboxId) return ownTarget; + return { accountId, identityId: accountIdentity.id, draftsMailboxId: roleIn(scope, 'drafts'), sentMailboxId }; +} diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index 764f6d24..d21ebe7c 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -67,6 +67,7 @@ import { } from '../lib/reply-identity'; import { shouldBlockEditorRemoteImages } from '../lib/editor-html'; import { placeholderWordsFor, unfilledPlaceholdersInMessage } from '../lib/placeholder-check'; +import { sendTargetFor } from '../lib/compose-send-account'; import { ComposerLoginMovedError, loginMovedFrom, uploadBytesForLogin, uploadFileForLogin, } from '../lib/compose-login'; @@ -1121,6 +1122,13 @@ export default function ComposeScreen({ route, navigation }: Props) { // Snapshot at the last successful draft save (null = never saved). const lastSavedRef = React.useRef(draft ? baselineRef.current : null); const draftIdRef = React.useRef(draft?.id ?? null); + // The account `draftIdRef` lives in, undefined for the login's own: a + // group identity's draft lives in the group's Drafts (#1090), and a send + // undone or a scheduled one cancelled for editing comes back in the + // account it was sent from. Email ids repeat across accounts, so the old + // version is destroyed there and nowhere else. + const draftAccountRef = React.useRef(draft?.jmapAccountId); + const draftAccount = () => draftAccountRef.current ?? jmapClient.accountId; const messageIdRef = React.useRef(draft?.messageId?.[0] ?? null); const inflightSaveRef = React.useRef | null>(null); const saveTimerRef = React.useRef | null>(null); @@ -1369,9 +1377,23 @@ export default function ComposeScreen({ route, navigation }: Props) { setDraftStatus('saving'); try { const outgoing = buildOutgoing(identity, html, { forDraft: true }); - const saved = await saveDraftVersion(outgoing, draftsMailbox.id, draftIdRef.current ?? undefined); + // A draft From a group identity is saved in the group's Drafts, like + // its Sent copy (#1090); one without a Drafts folder there in the own. + const target = await sendTargetFor(identity, outgoing.from[0]?.email ?? identity.email, mailboxes); + if (loginMoved()) return draftIdRef.current; + const saveIn = target.accountId && target.draftsMailboxId + ? { accountId: target.accountId, draftsMailboxId: target.draftsMailboxId } + : { accountId: undefined, draftsMailboxId: draftsMailbox.id }; + const saved = await saveDraftVersion( + outgoing, + saveIn.draftsMailboxId, + draftIdRef.current ?? undefined, + saveIn.accountId, + draftIdRef.current ? draftAccount() : undefined, + ); const id = saved.id; draftIdRef.current = id; + draftAccountRef.current = saveIn.accountId; // The previous version and its part blobs are gone: point every // attachment at the parts of the version just created. draftPartBlobsRef.current = rebaseDraftPartBlobs( @@ -1495,7 +1517,7 @@ export default function ComposeScreen({ route, navigation }: Props) { void (async () => { try { await inflightSaveRef.current; } catch { /* ignore */ } const id = draftIdRef.current ?? autosaved; - destroyEmails([id]).catch((err) => console.warn('[compose] discard failed', err)); + destroyEmails([id], draftAccountRef.current).catch((err) => console.warn('[compose] discard failed', err)); })(); } allowLeaveRef.current = true; @@ -2269,13 +2291,22 @@ export default function ComposeScreen({ route, navigation }: Props) { const outgoing = buildOutgoing(primaryIdentity, liveBodyHtml, { forDraft: false }); // The account the submission is held in, for Undo (read before the // request: a switch may land while it is out). - const submissionAccountId = jmapClient.connectedAccountId ?? undefined; + // A group or shared identity sends from its own account, and its Sent + // copy goes there (#1090). + const target = await sendTargetFor(primaryIdentity, outgoing.from[0]?.email ?? primaryIdentity.email, mailboxes); + if (refuseSendAfterSwitch()) return; + const submissionAccountId = target.accountId ?? jmapClient.connectedAccountId ?? undefined; const result = await sendEmail( outgoing, - primaryIdentity.id, - sentMailbox.id, + target.identityId, + target.sentMailboxId ?? sentMailbox.id, holdForSeconds, - { draftsMailboxId: draftsMailbox?.id, draftId: draftIdRef.current ?? undefined }, + { + draftsMailboxId: target.accountId ? target.draftsMailboxId : draftsMailbox?.id, + draftId: draftIdRef.current ?? undefined, + draftAccountId: draftIdRef.current ? draftAccount() : undefined, + accountId: target.accountId, + }, ); draftIdRef.current = null; lastSavedRef.current = null;