diff --git a/components/email/__tests__/composer-discard.test.tsx b/components/email/__tests__/composer-discard.test.tsx index ae7e06c9d..6c3f4b946 100644 --- a/components/email/__tests__/composer-discard.test.tsx +++ b/components/email/__tests__/composer-discard.test.tsx @@ -227,7 +227,7 @@ describe('composer Discard button', () => { fireEvent.click(within(dialog).getByText('discard')); - expect(onDiscardDraft).toHaveBeenCalledWith('draft-1'); + expect(onDiscardDraft).toHaveBeenCalledWith('draft-1', undefined, undefined); }); it('closes an empty, never saved composer without asking', () => { diff --git a/components/email/__tests__/composer-draft-attachments.test.tsx b/components/email/__tests__/composer-draft-attachments.test.tsx index 460acd1de..c9b6f35d8 100644 --- a/components/email/__tests__/composer-draft-attachments.test.tsx +++ b/components/email/__tests__/composer-draft-attachments.test.tsx @@ -263,7 +263,9 @@ describe('re-opened draft attachments (#849)', () => { expect.objectContaining({ blobId: 'part-blob-v1', name: 'report.pdf', size: 1234 }), ]); // ... after which the blobId is re-resolved against the new version. - expect(getEmail).toHaveBeenCalledWith('draft-v2'); + // Re-resolved in the account the draft was saved in (primary here: the mock + // client has no resolveSendAccountId). + expect(getEmail).toHaveBeenCalledWith('draft-v2', undefined); // The re-resolution itself must not count as a change: no phantom // save loop from the blobId swap. diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 116a7639d..f1153ed83 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -185,6 +185,12 @@ export interface ComposerDraftData { mode: 'compose' | 'reply' | 'replyAll' | 'forward'; replyTo?: EmailComposerProps['replyTo']; draftId: string | null; + /** + * The account `draftId` lives in - a group's own Drafts for a group identity + * (#1090). Draft ids are only unique within one account, so replacing or + * discarding the draft must name it. Absent: the primary account. + */ + draftAccountId?: string | null; /** * Server-side attachments of a re-opened draft (blobId references, no local * File). Without these the composer starts with zero attachments and the @@ -230,6 +236,8 @@ interface EmailComposerProps { * account dropdown; parents should send through that account's * client instead of the currently-active one. */ localAccountId?: string; + /** The account `draftId` lives in (see ComposerDraftData.draftAccountId). */ + draftAccountId?: string; attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>; inReplyTo?: string[]; references?: string[]; @@ -255,7 +263,8 @@ interface EmailComposerProps { * Cancelling the dialog drops it, so the draft simply stays put. */ requestCloseRef?: React.MutableRefObject<((afterClose?: () => void) => void) | null>; - onDiscardDraft?: (draftId: string) => void; + /** `draftAccountId`: where the draft lives; `localAccountId`: the login that reaches it (non-active identity). */ + onDiscardDraft?: (draftId: string, draftAccountId?: string, localAccountId?: string) => void; onSaveState?: (data: ComposerDraftData) => void; className?: string; initialDraftText?: string; @@ -825,6 +834,9 @@ export function EmailComposer({ // setDraftId is async, so a queued saveDraft would otherwise see the old // value and try to destroy a draft that was just replaced. const draftIdRef = useRef(initialData?.draftId ?? null); + // Account the draft lives in; follows the From identity (a group's own + // Drafts for a group identity, #1090). Read with draftIdRef. + const draftAccountIdRef = useRef(initialData?.draftAccountId ?? null); const [saveStatus, setSaveStatus] = useState<'idle' | 'saving' | 'saved' | 'error'>('idle'); const saveTimeoutRef = useRef(null); const lastSavedDataRef = useRef(""); @@ -1549,6 +1561,7 @@ export function EmailComposer({ const s = stateRef.current; onSaveState({ ...s, + draftAccountId: draftAccountIdRef.current, mode, replyTo, priority: priorityRef.current, @@ -2469,7 +2482,12 @@ export function EmailComposer({ // Use the JMAP client and raw identity id for the *owning* account // - falls back to active client for single-account / same-account - // identities. See `composerClient` derivation above. + // identities. See `composerClient` derivation above. Within that login + // the draft goes to the account owning the From identity (a group's own + // Drafts), and the replaced version is destroyed where it lives (#1090). + const draftAccountId = composerClient.resolveSendAccountId + ? await composerClient.resolveSendAccountId(savedDraft.fromEmail) + : undefined; const savedDraftId = await composerClient.createDraft( savedDraft.to, savedDraft.subject, @@ -2484,12 +2502,13 @@ export function EmailComposer({ savedDraft.htmlBody, replyThreadingHeaders?.inReplyTo, replyThreadingHeaders?.references, - { priority }, + { priority, accountId: draftAccountId, previousDraftAccountId: draftAccountIdRef.current ?? undefined }, ); // Update the ref synchronously so a queued save sees the new id and // doesn't try to destroy the just-replaced draft. draftIdRef.current = savedDraftId; + draftAccountIdRef.current = draftAccountId ?? null; setDraftId(savedDraftId); lastSavedDataRef.current = currentData; savedVersionRef.current = versionAtStart; @@ -2504,7 +2523,7 @@ export function EmailComposer({ const savedBlobIds = new Set(uploadedAttachments.map(att => att.blobId)); if (savedBlobIds.size) { try { - const freshDraft = await composerClient.getEmail(savedDraftId); + const freshDraft = await composerClient.getEmail(savedDraftId, draftAccountId); const freshParts = (freshDraft?.attachments ?? []).filter(p => !!p.blobId); if (freshParts.length) { const remap = (list: ComposerAttachment[]): ComposerAttachment[] => { @@ -3164,7 +3183,7 @@ export function EmailComposer({ // The draft was created through the identity's owning account // (see saveDraft), so clean it up there too - not on the active // account, where the id doesn't resolve (#461). - composerClient?.deleteEmail(finalDraftId).catch((err) => { + composerClient?.deleteEmail(finalDraftId, draftAccountIdRef.current ?? undefined).catch((err) => { debug.warn('email', 'Plugin handled the send, but draft cleanup failed:', err); }); } @@ -3227,6 +3246,7 @@ export function EmailComposer({ body: outgoing.textBody, htmlBody: outgoing.htmlBody || undefined, draftId: finalDraftId || undefined, + draftAccountId: finalDraftId ? (draftAccountIdRef.current ?? undefined) : undefined, fromEmail, fromName, identityId: rawId, @@ -3264,6 +3284,7 @@ export function EmailComposer({ setSubject(""); setBody(""); draftIdRef.current = null; + draftAccountIdRef.current = null; setDraftId(null); setSubAddressTag(""); setValidationErrors({}); @@ -3392,7 +3413,7 @@ export function EmailComposer({ clearTimeout(saveTimeoutRef.current); } if (draftId && onDiscardDraft) { - onDiscardDraft(draftId); + onDiscardDraft(draftId, draftAccountIdRef.current ?? undefined, currentIdentityParts.localAccountId ?? undefined); } stateRef.current = { to: '', cc: '', bcc: '', subject: '', body: '', showCc: false, showBcc: false, selectedIdentityId: null, subAddressTag: '', draftId: null, fromOverrideEnabled: false, fromOverrideEmail: '', fromOverrideName: '', attachments: [], plainTextMode }; emitClose(); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 2bbc64707..3347edd5f 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -2047,6 +2047,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { /** Local account owning the selected identity, when it came from the * cross-account From dropdown (more than one connected account). */ localAccountId?: string; + /** Account the draft lives in (a group's own Drafts for a group identity). */ + draftAccountId?: string; envelopeMailFrom?: string; attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>; inReplyTo?: string[]; @@ -2078,7 +2080,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const effectiveMode = pendingDraft?.mode ?? composerMode; const originalEmailId = composerSourceEmail?.id; - const result = await sendEmail(sendClient, data.to, data.subject, data.body, data.cc, data.bcc, data.identityId, data.fromEmail, data.draftId, data.fromName, data.htmlBody, data.attachments, data.inReplyTo, data.references, data.delayedUntil, data.envelopeMailFrom, { requestReadReceipt: data.requestReadReceipt, requestDsn: data.requestDsn, requireTls: data.requireTls, priority: data.priority, localAccountId: data.localAccountId }); + const result = await sendEmail(sendClient, data.to, data.subject, data.body, data.cc, data.bcc, data.identityId, data.fromEmail, data.draftId, data.fromName, data.htmlBody, data.attachments, data.inReplyTo, data.references, data.delayedUntil, data.envelopeMailFrom, { requestReadReceipt: data.requestReadReceipt, requestDsn: data.requestDsn, requireTls: data.requireTls, priority: data.priority, localAccountId: data.localAccountId, draftAccountId: data.draftAccountId }); submitted = true; announceSent(result, data); setShowComposer(false); @@ -2175,11 +2177,17 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } }; - const handleDiscardDraft = async (draftId: string) => { + // The draft lives where the composer saved it: the account owning the From + // identity (a group's own Drafts, #1090), reached through that identity's + // login when it isn't the active one. + const handleDiscardDraft = async (draftId: string, draftAccountId?: string, localAccountId?: string) => { if (!client) return; + const draftClient = localAccountId + ? (useAuthStore.getState().getClientForAccount(localAccountId) ?? client) + : client; try { - await client.deleteEmail(draftId); + await draftClient.deleteEmail(draftId, draftAccountId); } catch (error) { console.error("Failed to discard draft:", error); } @@ -2280,14 +2288,22 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { let draft = draftCandidate; if (!draft) return; + // Where the draft lives: a group/shared Drafts folder browsed directly, or + // the source stamp in an aggregate view (e.g. unified Drafts). Saving it + // again replaces it there, and the fetch has to name that account. + const draftMailbox = mailboxes.find(mb => mb.id === selectedMailbox); + const draftAccountId = draft.sourceAccountId + ?? (draftMailbox?.isShared ? draftMailbox.accountId : undefined); + const draftClient = draft.sourceClientAccountId + ? (useAuthStore.getState().getClientForAccount(draft.sourceClientAccountId) ?? client) + : client; + // The email list only fetches limited properties (no bodyValues/htmlBody/bcc). // Fetch the full email so the composer gets all draft content. if (!draft.bodyValues) { - const mailbox = mailboxes.find(mb => mb.id === selectedMailbox); - const accountId = mailbox?.isShared ? mailbox.accountId : undefined; - const fullDraft = await client.getEmail(draft.id, accountId); + const fullDraft = await draftClient.getEmail(draft.id, draftAccountId); if (!fullDraft) return; - draft = fullDraft; + draft = { ...fullDraft, sourceClientAccountId: draft.sourceClientAccountId, sourceAccountId: draft.sourceAccountId }; } draft = await emailHooks.onBeforeEditDraft.transform(draft); @@ -2349,6 +2365,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { inReplyTo: draft.inReplyTo ?? undefined, references: draft.references ?? undefined, priority: messagePriorityFromHeaders(draft.headers), + draftAccountId: draftAccountId ?? null, // Existing server-side attachments must ride along, or the composer // starts empty and the next save/send silently rebuilds the draft // without them (#849). @@ -4430,8 +4447,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { } }} requestCloseRef={composerRequestCloseRef} - onDiscardDraft={(draftId) => { - handleDiscardDraft(draftId); + onDiscardDraft={(draftId, draftAccountId, localAccountId) => { + handleDiscardDraft(draftId, draftAccountId, localAccountId); setPendingDraft(null); }} /> diff --git a/components/pro/pro-compose-tab-body.tsx b/components/pro/pro-compose-tab-body.tsx index 84fdd5c90..f584eb779 100644 --- a/components/pro/pro-compose-tab-body.tsx +++ b/components/pro/pro-compose-tab-body.tsx @@ -87,7 +87,7 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) { sendData.references, sendData.delayedUntil, sendData.envelopeMailFrom, - { localAccountId: sendData.localAccountId }, + { localAccountId: sendData.localAccountId, draftAccountId: sendData.draftAccountId }, ); submitted = true; announceSent(result, sendData); @@ -160,10 +160,13 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) { }); }, [closeTab]); - const handleDiscardDraft = useCallback(async (draftId: string) => { + const handleDiscardDraft = useCallback(async (draftId: string, draftAccountId?: string, localAccountId?: string) => { if (!client) return; + const draftClient = localAccountId + ? (useAuthStore.getState().getClientForAccount(localAccountId) ?? client) + : client; try { - await client.deleteEmail(draftId); + await draftClient.deleteEmail(draftId, draftAccountId); } catch (error) { console.error('Failed to discard draft:', error); } diff --git a/integration/tests/18-group-sent-copy.spec.ts b/integration/tests/18-group-sent-copy.spec.ts new file mode 100644 index 000000000..9bcdf411c --- /dev/null +++ b/integration/tests/18-group-sent-copy.spec.ts @@ -0,0 +1,186 @@ +import { test, expect, type Page } from '@playwright/test'; +import { ACCOUNTS, GROUP } from './helpers/config'; +import { JmapClient } from './helpers/jmap'; +import { + login, + forceSync, + openComposer, + addRecipient, + setSubject, + selectComposerFrom, + selectedComposerFrom, + expandSharedFolders, + openFolder, + expectEmailVisible, + emailItem, + waitDraftSaved, +} from './helpers/app'; + +/** + * Issue #1090: a message sent as the group's identity must be filed in the + * GROUP account's Sent folder - for compose, reply and forward alike - not in + * the member's own Sent. + * + * carol is a member of the `team` group (pre-provisioned). Stalwart lists the + * group's team@ identity among carol's own identities too, which is how the + * send used to slip through carol's account whenever the group account could + * not be matched by name (e.g. a sub-addressed From, an alias identity, or an + * account named by its bare principal name). + */ +const { bob, carol } = ACCOUNTS; +const { team } = GROUP; + +let seq = 0; +const subj = (l: string) => `IT ${l} ${Date.now()}-${seq++}`; + +async function send(page: Page): Promise { + // Focus without a click: the recipient autocomplete can overlay the field. + await page.locator('[data-testid="composer-subject"]').first().focus(); + await page.keyboard.press('Control+Enter'); + await page.locator('[data-testid="email-composer"]').first().waitFor({ state: 'hidden', timeout: 20000 }); +} + +test.describe('Sent copy of a group identity lands in the group’s Sent (#1090)', () => { + let jc: JmapClient; + let teamId: string; + + test.beforeEach(async () => { + jc = await JmapClient.connect(carol.email, carol.password); + teamId = jc.accountIdByName(team.email); + await jc.reset(); + await jc.reset(teamId); + }); + + /** Subjects of every message in the Sent folder of `accountId`. */ + async function sentSubjects(accountId: string): Promise { + return roleSubjects('sent', accountId); + } + + /** Subjects of every message in the `role` folder of `accountId`. */ + async function roleSubjects(role: string, accountId: string): Promise { + const sent = (await jc.mailboxByRole(role, accountId))!.id; + const r = await jc.request([ + ['Email/query', { accountId, filter: { inMailbox: sent } }, '0'], + ['Email/get', { accountId, '#ids': { resultOf: '0', name: 'Email/query', path: '/ids' }, properties: ['subject'] }, '1'], + ]); + return (r.methodResponses[1][1].list as Array<{ subject: string }>).map((m) => m.subject); + } + + /** + * Poll until a message whose subject contains `subject` ("Re: …", "Fwd: …" + * included - Stalwart's subject filter only strips "Re:") is in the group's + * Sent, and assert carol's own Sent has none. + */ + async function expectInGroupSent(subject: string): Promise { + await expect + .poll(async () => (await sentSubjects(teamId)).some((x) => x.includes(subject)), { timeout: 20000, message: 'copy in team Sent' }) + .toBe(true); + expect((await sentSubjects(jc.accountId)).filter((x) => x.includes(subject)), 'no copy in carol’s own Sent').toEqual([]); + } + + /** Create a received message directly in the group's Inbox (carol has write access). */ + async function placeInGroupInbox(inboxId: string, subject: string): Promise { + const r = await jc.request([['Email/set', { + accountId: teamId, + create: { m: { + mailboxIds: { [inboxId]: true }, + from: [{ email: 'someone@external.test' }], + to: [{ email: team.email }], + subject, + receivedAt: new Date().toISOString(), + bodyValues: { b: { value: 'x' } }, + textBody: [{ partId: 'b', type: 'text/plain' }], + } }, + }, '0']]); + if (!r.methodResponses[0][1].created?.m) throw new Error(`seed failed: ${JSON.stringify(r.methodResponses[0][1])}`); + } + + async function composeAsTeam(page: Page, subject: string): Promise { + await login(page, carol); + await forceSync(page); + await openComposer(page); + await expect.poll(() => page.locator('[data-testid="composer-from"] option').allTextContents().then(t => t.join('|')), { timeout: 15000 }) + .toContain(team.email); + await selectComposerFrom(page, team.email); + await expect.poll(() => selectedComposerFrom(page)).toContain(team.email); + await addRecipient(page, bob.email); + await setSubject(page, subject); + // An empty body keeps Send disabled. + await page.locator('[data-testid="email-composer"] [contenteditable="true"]').first().click(); + await page.keyboard.type('Hello from the team'); + } + + test('compose as the group', async ({ page }) => { + const s = subj('grp-compose'); + await composeAsTeam(page, s); + await send(page); + await expectInGroupSent(s); + }); + + test('compose as the group with a sub-addressed From', async ({ page }) => { + const s = subj('grp-subaddr'); + await composeAsTeam(page, s); + await page.getByTitle('Use sub-address').first().click(); + const tag = page.getByPlaceholder('Enter tag (e.g., shopping)'); + await tag.fill('news'); + await tag.press('Enter'); + await send(page); + await expectInGroupSent(s); + }); + + for (const [mode, key] of [['reply', 'r'], ['forward', 'f']] as const) { + test(`${mode} from the group mailbox`, async ({ page }) => { + const original = subj(`grp-${mode}-in`); + const inbox = (await jc.mailboxByRole('inbox', teamId))!.id; + await placeInGroupInbox(inbox, original); + + await login(page, carol); + await expandSharedFolders(page, team.email); + await openFolder(page, { role: 'inbox', shared: true }); + await forceSync(page); + await expectEmailVisible(page, original); + await emailItem(page, original).first().click(); + await expect(emailItem(page, original).first()).toHaveAttribute('data-unread', 'false', { timeout: 15000 }); + await page.keyboard.press(key); + await expect.poll(() => selectedComposerFrom(page), { timeout: 15000 }).toContain(team.email); + if (mode === 'forward') await addRecipient(page, bob.email); + await send(page); + // The subject filter matches the base subject (no "Re:"/"Fwd:"); the + // original sits in the group Inbox, so look in Sent only. + await expectInGroupSent(original); + }); + } + + test.describe('drafts of a group identity live in the group’s Drafts', () => { + const draftsHave = async (accountId: string, subject: string) => + (await roleSubjects('drafts', accountId)).some((x) => x.includes(subject)); + + test('autosaved draft lands in the group’s Drafts and leaves it on send', async ({ page }) => { + const s = subj('grp-draft-send'); + await composeAsTeam(page, s); + await waitDraftSaved(page); + await expect.poll(() => draftsHave(teamId, s), { timeout: 20000, message: 'draft in team Drafts' }).toBe(true); + expect(await draftsHave(jc.accountId, s), 'no draft in carol’s Drafts').toBe(false); + + await send(page); + await expectInGroupSent(s); + await expect.poll(() => draftsHave(teamId, s), { timeout: 20000, message: 'draft removed after send' }).toBe(false); + expect(await draftsHave(jc.accountId, s)).toBe(false); + }); + + test('discarding removes the draft from the group’s Drafts', async ({ page }) => { + const s = subj('grp-draft-discard'); + await composeAsTeam(page, s); + await waitDraftSaved(page); + await expect.poll(() => draftsHave(teamId, s), { timeout: 20000, message: 'draft in team Drafts' }).toBe(true); + + // "Discard" opens the close dialog (dirty composer); its own "Discard" + // button discards the draft. + const discard = page.getByRole('button', { name: 'Discard', exact: true }); + await discard.first().click(); + await expect(discard).toHaveCount(2, { timeout: 5000 }); + await discard.last().click(); + await expect.poll(() => draftsHave(teamId, s), { timeout: 20000, message: 'draft discarded' }).toBe(false); + }); + }); +}); diff --git a/lib/__tests__/jmap-send-group-account.test.ts b/lib/__tests__/jmap-send-group-account.test.ts new file mode 100644 index 000000000..4a6b7a22e --- /dev/null +++ b/lib/__tests__/jmap-send-group-account.test.ts @@ -0,0 +1,193 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { JMAPClient } from '../jmap/client'; + +// #1090: a message sent as a group (shared) mailbox identity must be created, +// submitted and filed in Sent in the GROUP account, not the member's own. The +// account used to be found only by matching its session name against the From +// address, which misses a group whose account name is a bare principal name, +// an alias identity of the group, and a sub-addressed From. Stalwart also lists +// the group's identity among the member's own, so the send silently went +// through the member's account. + +type MethodCall = [string, Record, string]; + +const SUBMISSION = 'urn:ietf:params:jmap:submission'; + +const IDENTITIES: Record> = { + // The member's own list mirrors the group's send-as identity (Stalwart). + me: [{ id: 'b', email: 'team@example.org' }, { id: 'c', email: 'carol@example.org' }], + grp: [{ id: 'b', email: 'team@example.org' }, { id: 'x', email: 'info@example.org' }], + alice: [{ id: 'a', email: 'alice@example.org' }], + wild: [{ id: 'w', email: '*@lists.example.org' }], +}; + +function createClient(accounts: Record): JMAPClient { + const client = new JMAPClient('https://jmap.example.org', 'carol@example.org', 'pass'); + Object.assign(client, { + apiUrl: 'https://jmap.example.org/api', + accountId: 'me', + username: 'carol@example.org', + accounts: Object.fromEntries(Object.entries(accounts).map(([id, name]) => [id, { name }])), + session: { + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'me', [SUBMISSION]: 'me' }, + accounts: Object.fromEntries(Object.keys(accounts).map((id) => [id, { + accountCapabilities: { 'urn:ietf:params:jmap:mail': {}, [SUBMISSION]: {} }, + }])), + }, + }); + return client; +} + +interface Recorded { + emailSetAccount?: string; + createdMailbox?: string; + destroyAccount?: string; + submissionAccount?: string; + sentMailbox?: string; + identityId?: string; +} + +function mockServer(): Recorded { + const rec: Recorded = {}; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const { methodCalls } = JSON.parse((init as { body: string }).body) as { methodCalls: MethodCall[] }; + const methodResponses = methodCalls.map(([name, args, callId]) => { + const accountId = args.accountId as string; + if (name === 'Mailbox/get') { + return [name, { list: [ + { id: `${accountId}-drafts`, name: 'Drafts', role: 'drafts' }, + { id: `${accountId}-sent`, name: 'Sent', role: 'sent' }, + ] }, callId]; + } + if (name === 'Identity/get') return [name, { list: IDENTITIES[accountId] ?? [] }, callId]; + if (name === 'Email/set') { + if (args.create) { + rec.emailSetAccount = accountId; + const created = Object.values(args.create as Record }>)[0]; + rec.createdMailbox = Object.keys(created?.mailboxIds ?? {})[0]; + } + if (args.destroy) rec.destroyAccount = accountId; + const created = Object.keys((args.create ?? {}) as object); + return [name, { created: Object.fromEntries(created.map((id) => [id, { id: 'email-9' }])) }, callId]; + } + if (name === 'EmailSubmission/set') { + rec.submissionAccount = accountId; + rec.identityId = (args.create as Record)['1'].identityId; + const update = (args.onSuccessUpdateEmail as Record }>)['#1']; + rec.sentMailbox = Object.keys(update.mailboxIds ?? {})[0]; + return [name, { created: { '1': { id: 'sub-1' } } }, callId]; + } + return ['error', { type: 'unknownMethod' }, callId]; + }); + const payload = { methodResponses }; + return { + ok: true, + status: 200, + text: () => Promise.resolve(JSON.stringify(payload)), + json: () => Promise.resolve(payload), + } as Response; + }); + return rec; +} + +function send(client: JMAPClient, fromEmail: string, identityId = 'b') { + return client.sendEmail(['rcpt@example.net'], 'Subject', 'body', undefined, undefined, identityId, fromEmail); +} + +describe('JMAPClient.sendEmail picks the account that owns the From identity (#1090)', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + const expectAccount = (rec: Recorded, account: string) => { + expect(rec.emailSetAccount).toBe(account); + expect(rec.submissionAccount).toBe(account); + expect(rec.sentMailbox).toBe(`${account}-sent`); + }; + + it('sends a group identity through the group account when its name is a bare principal name', async () => { + const rec = mockServer(); + await send(createClient({ me: 'carol@example.org', grp: 'team' }), 'team@example.org'); + expectAccount(rec, 'grp'); + }); + + it('sends an alias identity of the group through the group account', async () => { + const rec = mockServer(); + await send(createClient({ me: 'carol@example.org', grp: 'team' }), 'info@example.org', 'x'); + expectAccount(rec, 'grp'); + expect(rec.identityId).toBe('x'); + }); + + it('sends a sub-addressed group From through the group account', async () => { + const rec = mockServer(); + await send(createClient({ me: 'carol@example.org', grp: 'team' }), 'team+news@example.org'); + expectAccount(rec, 'grp'); + }); + + it('matches a wildcard identity of a delegated account for its domain', async () => { + const rec = mockServer(); + await send(createClient({ me: 'carol@example.org', wild: 'lists' }), 'announce@lists.example.org', 'w'); + expectAccount(rec, 'wild'); + }); + + it('keeps the member’s own address (and its sub-addresses) on the primary account', async () => { + const own = mockServer(); + await send(createClient({ me: 'carol@example.org', grp: 'team', alice: 'alice@example.org' }), 'carol@example.org', 'c'); + expectAccount(own, 'me'); + + vi.restoreAllMocks(); + const sub = mockServer(); + await send(createClient({ me: 'carol-renamed', grp: 'team' }), 'carol+x@example.org', 'c'); + expectAccount(sub, 'me'); + }); + + it('still prefers an account named exactly like the From address', async () => { + const rec = mockServer(); + await send(createClient({ me: 'carol@example.org', grp: 'team@example.org' }), 'team@example.org'); + expectAccount(rec, 'grp'); + }); + + describe('drafts', () => { + const draft = (client: JMAPClient, fromEmail: string, draftId?: string, options?: { accountId?: string; previousDraftAccountId?: string }) => + client.createDraft(['rcpt@example.net'], 'Subject', 'body', undefined, undefined, 'b', fromEmail, draftId, + undefined, undefined, undefined, undefined, undefined, options); + + it('saves a group identity’s draft in the group account’s Drafts', async () => { + const rec = mockServer(); + await draft(createClient({ me: 'carol@example.org', grp: 'team' }), 'team@example.org'); + expect(rec.emailSetAccount).toBe('grp'); + expect(rec.createdMailbox).toBe('grp-drafts'); + }); + + it('keeps the member’s own drafts in the primary account', async () => { + const rec = mockServer(); + await draft(createClient({ me: 'carol@example.org', grp: 'team' }), 'carol@example.org'); + expect(rec.emailSetAccount).toBe('me'); + expect(rec.createdMailbox).toBe('me-drafts'); + }); + + it('destroys the replaced version in the account it lives in', async () => { + const rec = mockServer(); + // Identity switched from carol to team: the new version goes to the + // group, the old one is destroyed in carol's account. + await draft(createClient({ me: 'carol@example.org', grp: 'team' }), 'team@example.org', 'old-1', { previousDraftAccountId: 'me' }); + expect(rec.emailSetAccount).toBe('grp'); + expect(rec.destroyAccount).toBe('me'); + + vi.restoreAllMocks(); + const again = mockServer(); + await draft(createClient({ me: 'carol@example.org', grp: 'team' }), 'team@example.org', 'old-2', { previousDraftAccountId: 'grp' }); + expect(again.destroyAccount).toBe('grp'); + }); + + it('sendEmail destroys the sent draft in the account named by draftAccountId', async () => { + const rec = mockServer(); + await createClient({ me: 'carol@example.org', grp: 'team' }).sendEmail( + ['rcpt@example.net'], 'Subject', 'body', undefined, undefined, 'b', 'team@example.org', 'draft-7', + undefined, undefined, undefined, undefined, undefined, undefined, undefined, { draftAccountId: 'grp' }, + ); + expectAccount(rec, 'grp'); + expect(rec.destroyAccount).toBe('grp'); + }); + }); +}); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index e0c13be0b..bd37cf448 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -635,6 +635,10 @@ export class DemoJMAPClient implements IJMAPClient { // ── Compose / Send ──────────────────────────────────────────── + async resolveSendAccountId(_fromEmail?: string): Promise { + return this.getAccountId(); + } + async createDraft( to: string[], subject: string, diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index efab390a2..c4eca22d7 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -349,10 +349,25 @@ export interface IJMAPClient { htmlBody?: string, inReplyTo?: string[], references?: string[], - // Importance / X-Priority headers, so a re-opened draft keeps its priority (#970). - options?: { priority?: MessagePriority }, + /** + * `priority`: Importance / X-Priority headers, so a re-opened draft keeps + * its priority (#970). `accountId`: the account to save the draft in + * (default: the account that owns the From identity, see + * resolveSendAccountId). `previousDraftAccountId`: where `draftId` lives, + * so the replaced version is destroyed there (default: the primary + * account). Draft ids are only unique within one account. + */ + options?: { priority?: MessagePriority; accountId?: string; previousDraftAccountId?: string }, ): Promise; + /** + * The account a message From `fromEmail` is created, submitted and filed in: + * a group/shared account that owns an identity for the address, else the + * primary account (#1090). Optional: without it drafts stay in the primary + * account. + */ + resolveSendAccountId?(fromEmail?: string): Promise; + sendEmail( to: string[], subject: string, @@ -375,7 +390,9 @@ export interface IJMAPClient { // requestDsn / requireTls map to RFC 3461 / RFC 8689 envelope parameters // and need the matching `submissionExtensions` entry (see // supportsSubmissionExtension). - options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority }, + // `draftAccountId`: the account `draftId` lives in, so the sent draft is + // destroyed there (default: the primary account). + options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; draftAccountId?: string }, ): Promise; /** Whether the submission account advertises an SMTP extension ("DSN", "REQUIRETLS", …). */ supportsSubmissionExtension?(extension: string, accountId?: string): boolean; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 5032231b4..74f5f249f 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -976,6 +976,8 @@ export interface SubmissionSendOptions { requireTls?: boolean; /** Importance / X-Priority headers (#970); see lib/message-priority.ts. */ priority?: MessagePriority; + /** The account the replaced draft (`draftId`) lives in; default the primary. */ + draftAccountId?: string; } /** The MAIL FROM / RCPT TO parameters the options translate to. */ @@ -1019,6 +1021,25 @@ type SubmissionCapability = { submissionExtensions?: unknown; }; +/** An identity address that is exactly `from` (both lower-cased by the caller or here). */ +function identityEmailMatchesExactly(identityEmail: string | undefined, from: string): boolean { + return !!identityEmail && identityEmail.toLowerCase() === from; +} + +/** + * An identity address that covers `from` without being it: a sub-address of + * it (team+tag@ for team@, with the common "+" / "-" delimiters) or a + * wildcard identity for its domain (RFC 8621 §6, "*@example.com"). + */ +function identityEmailCovers(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) return false; + if (idLocal === '*') return true; + return fromLocal.startsWith(`${idLocal}+`) || fromLocal.startsWith(`${idLocal}-`); +} + export class JMAPClient implements IJMAPClient { private static readonly RATE_LIMIT_TOAST_THROTTLE_MS = 10_000; /** @@ -1060,6 +1081,8 @@ export class JMAPClient implements IJMAPClient { private pingFailureCount = 0; private pingSkipRemaining = 0; private accounts: Record = {}; + /** Identities per candidate account for resolveSendAccountId, briefly cached. */ + private sendIdentitiesCache: { key: string; identitiesByAccount: Array<[string, Identity[]]>; expires: number } | null = null; private eventSource: EventSource | null = null; private stateChangeCallback: ((change: StateChange) => void) | null = null; private lastStates: AccountStates = {}; @@ -3971,9 +3994,13 @@ export class JMAPClient implements IJMAPClient { htmlBody?: string, inReplyTo?: string[], references?: string[], - options?: { priority?: MessagePriority }, + options?: { priority?: MessagePriority; accountId?: string; previousDraftAccountId?: string }, ): Promise { - const mailboxes = await this.getMailboxes(); + // A draft for a group identity belongs in the group's Drafts, like its + // Sent copy (#1090). + const draftAccountId = options?.accountId ?? await this.resolveSendAccountId(fromEmail); + const mailboxResponse = await this.request([["Mailbox/get", { accountId: draftAccountId }, "0"]]); + const mailboxes = (mailboxResponse.methodResponses?.[0]?.[1]?.list || []) as Mailbox[]; const draftsMailbox = mailboxes.find(mb => mb.role === 'drafts'); if (!draftsMailbox) { throw new Error('No drafts mailbox found'); @@ -4045,7 +4072,7 @@ export class JMAPClient implements IJMAPClient { // copy of the draft. Worst case now is an orphaned old version. const response = await this.request([ ["Email/set", { - accountId: this.accountId, + accountId: draftAccountId, create: { [emailId]: emailData }, }, "0"], ]); @@ -4067,7 +4094,7 @@ export class JMAPClient implements IJMAPClient { if (draftId) { try { const destroyResponse = await this.request([ - ["Email/set", { accountId: this.accountId, destroy: [draftId] }, "0"], + ["Email/set", { accountId: options?.previousDraftAccountId ?? this.accountId, destroy: [draftId] }, "0"], ]); const destroyResult = destroyResponse.methodResponses?.[0]?.[1]; if (destroyResult?.notDestroyed) { @@ -4106,9 +4133,7 @@ export class JMAPClient implements IJMAPClient { ): Promise { const holdForSeconds = delayedUntil ? this.validateDelayedUntil(delayedUntil) : undefined; const emailId = `send-${Date.now()}`; - const targetAccountId = (fromEmail && Object.keys(this.accounts).find(id => - this.accounts[id]?.name?.toLowerCase() === fromEmail.toLowerCase() - )) || this.accountId; + const targetAccountId = await this.resolveSendAccountId(fromEmail); const mboxResp = await this.request([ ["Mailbox/get", { accountId: targetAccountId }, "0"] ]); @@ -4421,7 +4446,7 @@ export class JMAPClient implements IJMAPClient { if (draftId && createdEmailId) { try { const destroyResponse = await this.request([ - ["Email/set", { accountId: this.accountId, destroy: [draftId] }, "0"], + ["Email/set", { accountId: options?.draftAccountId ?? this.accountId, destroy: [draftId] }, "0"], ]); const destroyResult = destroyResponse.methodResponses?.[0]?.[1]; if (destroyResult?.notDestroyed && Object.keys(destroyResult.notDestroyed).length) { @@ -5376,6 +5401,68 @@ export class JMAPClient implements IJMAPClient { return submission?.envelope; } + /** + * The account a message sent From `fromEmail` belongs to: where it is + * created, submitted and filed in Sent (#1090). + * + * Matching the session account *name* against the address alone misses a + * group mailbox whose account name is not that address (a bare principal + * name such as "team", a display name), an alias identity of the group, and + * a sub-addressed From (team+tag@). Stalwart also lists a group's send-as + * identity among the member's own identities, so the send then silently went + * through the member's account and its Sent copy landed there. + * + * Order: an account named exactly `fromEmail`; then an account that owns an + * identity for the address - exact before sub-address/wildcard, and a + * delegated (group/shared) account before the primary one, since the + * primary may merely mirror the group's identity; else the primary account. + */ + async resolveSendAccountId(fromEmail?: string): Promise { + if (!fromEmail) return this.accountId; + const from = parseRecipientString(fromEmail).email.toLowerCase(); + const named = Object.keys(this.accounts).find(id => this.accounts[id]?.name?.toLowerCase() === from); + if (named) return named; + + const delegated = Object.keys(this.accounts).filter(id => + id !== this.accountId + && !!this.session?.accounts?.[id]?.accountCapabilities?.['urn:ietf:params:jmap:submission'], + ); + if (delegated.length === 0) return this.accountId; + + const candidates = [...delegated, this.accountId]; + let identitiesByAccount: Array<[string, Identity[]]>; + const cached = this.sendIdentitiesCache; + if (cached && cached.expires > Date.now() && cached.key === candidates.join('\u0000')) { + identitiesByAccount = cached.identitiesByAccount; + } else { + try { + const response = await this.request( + candidates.map((id, i) => ["Identity/get", { accountId: id }, String(i)] as JMAPMethodCall), + ["urn:ietf:params:jmap:core", "urn:ietf:params:jmap:submission"], + ); + identitiesByAccount = candidates.map((id, i) => { + const [name, result] = response.methodResponses?.[i] ?? []; + return [id, name === "Identity/get" ? ((result?.list ?? []) as Identity[]) : []]; + }); + // Draft autosave resolves on every save; identities rarely change. + this.sendIdentitiesCache = { + key: candidates.join('\u0000'), + identitiesByAccount, + expires: Date.now() + 60_000, + }; + } catch (err) { + debug.warn('email', '[sendEmail] could not load identities to pick the sending account', err); + return this.accountId; + } + } + + for (const matches of [identityEmailMatchesExactly, identityEmailCovers]) { + const hit = identitiesByAccount.find(([, list]) => list.some(identity => matches(identity.email, from))); + if (hit) return hit[0]; + } + return this.accountId; + } + private getSubmissionAccountId(accountId?: string): string { // The requested (mail) account may not host EmailSubmission objects — JMAP // allows submission to live in a separate account (session diff --git a/stores/email-store.ts b/stores/email-store.ts index 67d765405..da18565c8 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -319,7 +319,7 @@ interface EmailStore { loadMoreEmails: (client: IJMAPClient) => Promise; fetchEmailContent: (client: IJMAPClient, emailId: string) => Promise; fetchQuota: (client: IJMAPClient) => Promise; - sendEmail: (client: IJMAPClient, to: string[], subject: string, body: string, cc?: string[], bcc?: string[], identityId?: string, fromEmail?: string, draftId?: string, fromName?: string, htmlBody?: string, attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>, inReplyTo?: string[], references?: string[], delayedUntil?: string, envelopeMailFrom?: string, options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; localAccountId?: string }) => Promise; + sendEmail: (client: IJMAPClient, to: string[], subject: string, body: string, cc?: string[], bcc?: string[], identityId?: string, fromEmail?: string, draftId?: string, fromName?: string, htmlBody?: string, attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>, inReplyTo?: string[], references?: string[], delayedUntil?: string, envelopeMailFrom?: string, options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; localAccountId?: string; draftAccountId?: string }) => Promise; sendRawEmail: (client: IJMAPClient, rawMimeBlob: Blob, identityId: string, delayedUntil?: string, envelopeRecipients?: string[]) => Promise; /** `emailKey` (emailKeyFor of the row) picks the right row when the id repeats across accounts. */ deleteEmail: (client: IJMAPClient, emailId: string, forceDelete?: boolean, emailKey?: string) => Promise;