From e81f00714ca13df199bbfc22422e2d03474f675a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:56:19 +0100 Subject: [PATCH] fix: undo and send now of a delayed send only reach the login it was sent from --- locales/rn/en.json | 2 + src/components/email/QuickReplyBox.tsx | 7 +- src/screens/ComposeScreen.tsx | 5 ++ src/stores/__tests__/send-undo-store.test.ts | 79 +++++++++++++++++++- src/stores/send-undo-store.ts | 65 +++++++++++++++- 5 files changed, 151 insertions(+), 7 deletions(-) diff --git a/locales/rn/en.json b/locales/rn/en.json index a1042d59..1d5a6813 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -340,6 +340,8 @@ "remove_link": "Remove link" }, "undo_send_label": "Sending in {seconds}s…", + "undo_send_other_login": "This message was sent from another account", + "undo_send_other_login_body": "Switch back to {account} to undo it or send it now. It goes out when the delay ends.", "upload_failed_short": "Upload failed", "uploading": "Uploading...", "uploading_pct": "Uploading {pct}%", diff --git a/src/components/email/QuickReplyBox.tsx b/src/components/email/QuickReplyBox.tsx index acc22a5a..2da499b9 100644 --- a/src/components/email/QuickReplyBox.tsx +++ b/src/components/email/QuickReplyBox.tsx @@ -8,7 +8,7 @@ import { useSettingsStore } from '../../stores/settings-store'; import { useLocaleStore } from '../../stores/locale-store'; import { useEmailStore } from '../../stores/email-store'; import { toast } from '../../stores/toast-store'; -import { useSendUndoStore } from '../../stores/send-undo-store'; +import { currentSendLogin, useSendUndoStore } from '../../stores/send-undo-store'; import { sendEmail, patchKeywordsForEmails } from '../../api/email'; import { jmapClient } from '../../api/jmap-client'; import { pruneEmailSubmissions } from '../../api/submissions'; @@ -103,6 +103,9 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF }); const threading = computeReplyThreadingHeaders(email); const holdFor = jmapClient.undoSendHold(sendDelaySeconds, jmapAccountId); + // Where the reply goes, read before it goes: Undo must reach that login + // and account whatever the app switches to meanwhile. + const sentThrough = { login: currentSendLogin(), accountId: jmapAccountId ?? jmapClient.connectedAccountId ?? undefined }; const result = await sendEmail( { from: [{ name: identity.name, email: identity.email }], @@ -127,7 +130,7 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF // Recorded before the flag below so its round trip doesn't eat the window. useSendUndoStore.getState().recordHeldSend(result, holdFor, { identityId: identity.id, - accountId: jmapAccountId, + ...sentThrough, from: [{ name: identity.name, email: identity.email }], }); void pruneEmailSubmissions(); diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index 37798e8c..6e6ba072 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -2184,6 +2184,9 @@ export default function ComposeScreen({ route, navigation }: Props) { setSending(true); try { 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; const result = await sendEmail( outgoing, primaryIdentity.id, @@ -2237,6 +2240,8 @@ export default function ComposeScreen({ route, navigation }: Props) { // Undo-send window: the undo bar offers Undo / Send now. useSendUndoStore.getState().recordHeldSend(result, holdForSeconds, { identityId: primaryIdentity.id, + accountId: submissionAccountId, + login: ownerLoginRef.current, from: outgoing.from, to: [...outgoing.to, ...(outgoing.cc ?? []), ...(outgoing.bcc ?? [])], }); diff --git a/src/stores/__tests__/send-undo-store.test.ts b/src/stores/__tests__/send-undo-store.test.ts index 7ad20326..1a2a33e9 100644 --- a/src/stores/__tests__/send-undo-store.test.ts +++ b/src/stores/__tests__/send-undo-store.test.ts @@ -7,11 +7,31 @@ vi.mock('../../api/email', () => ({ getFullEmail: vi.fn(), })); +// The login the client serves: me@example.com on mail.example.com, whose +// own JMAP account is 'primary'. Tests switch it to another server. +const { client } = vi.hoisted(() => ({ + client: { + accountId: 'primary', + connectedAccountId: 'primary' as string | null, + isConnected: true, + username: 'me@example.com' as string | null, + serverUrl: 'https://mail.example.com' as string | null, + }, +})); vi.mock('../../api/jmap-client', () => ({ - jmapClient: { accountId: 'primary' }, + jmapClient: client, RequestTimeoutError: class RequestTimeoutError extends Error {}, })); +const LOGIN = 'me@example.com@mail.example.com'; +const OTHER_LOGIN = 'me@example.com@other.example.org'; + +/** Switch the app to the same user on another server, whose own account is also 'primary'. */ +function switchToOtherServer() { + useAccountStore.setState({ activeAccountId: OTHER_LOGIN }); + client.serverUrl = 'https://other.example.org'; +} + // Background work nobody waits for: it never settles here. vi.mock('../../api/submissions', () => ({ pruneEmailSubmissions: vi.fn(() => new Promise(() => undefined)), @@ -23,6 +43,7 @@ import { NoLongerScheduledError, RecipientsRefusedError, SubmissionQuotaError } import type { Email, Mailbox } from '../../api/types'; import { useSendUndoStore, restoreUndoneSend, type PendingUndoSend } from '../send-undo-store'; import { useToastStore } from '../toast-store'; +import { useAccountStore } from '../account-store'; const api = vi.mocked(emailApi); const prune = vi.mocked(submissions.pruneEmailSubmissions); @@ -70,6 +91,8 @@ beforeEach(() => { useSendUndoStore.setState({ pending: null, busy: false, restoredEmailId: null }); useToastStore.getState().clearToasts(); api.getFullEmail.mockResolvedValue({ id: 'mail-1', subject: 'Re: hi' } as Email); + client.serverUrl = 'https://mail.example.com'; + useAccountStore.setState({ activeAccountId: LOGIN }); }); afterEach(() => { @@ -93,9 +116,21 @@ describe('send-undo-store', () => { from: [SENDER], sendAt: HELD.sendAt, delaySeconds: 10, + login: LOGIN, }); }); + it('names the own account and the login of an own-account send', () => { + useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] }); + expect(useSendUndoStore.getState().pending).toMatchObject({ accountId: 'primary', login: LOGIN }); + }); + + it('keeps the login the sender sent through when a switch landed during the send', () => { + client.serverUrl = 'https://other.example.org'; + useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', login: LOGIN, accountId: 'primary' }); + expect(useSendUndoStore.getState().pending?.login).toBe(LOGIN); + }); + it('records nothing for a send that went out at once', () => { const store = useSendUndoStore.getState(); expect(store.recordHeldSend({ scheduled: false, emailSubmissionId: 'sub-1', emailId: 'mail-1' }, undefined, { identityId: 'id-1' })).toBe(false); @@ -121,9 +156,47 @@ describe('send-undo-store', () => { }); it('cancels an own-account send in the primary account', async () => { - useSendUndoStore.getState().setPending(entry()); + useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] }); await useSendUndoStore.getState().undo(); - expect(api.cancelScheduledSend).toHaveBeenCalledWith('sub-1', undefined); + expect(api.cancelScheduledSend).toHaveBeenCalledWith('sub-1', 'primary'); + }); + + // GHSA-8x23: the same user on another server has an account 'primary' + // too, with its own submission 'sub-1'. + it('cancels nothing in another login, and says where the message was sent from', async () => { + useAccountStore.setState({ + accounts: [{ id: LOGIN, email: 'me@example.com', displayName: 'Me' } as never], + }); + useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] }); + const pending = useSendUndoStore.getState().pending; + // The switch is still under way: the bar is up and the client serves the other server. + client.serverUrl = 'https://other.example.org'; + + await expect(useSendUndoStore.getState().undo()).resolves.toBe(false); + await expect(useSendUndoStore.getState().sendNow()).resolves.toBe(false); + + expect(api.cancelScheduledSend).not.toHaveBeenCalled(); + expect(api.rescheduleScheduledSend).not.toHaveBeenCalled(); + expect(useToastStore.getState().toasts[0]).toMatchObject({ + type: 'error', + message: expect.stringContaining('me@example.com'), + }); + expect(useSendUndoStore.getState().pending).toBe(pending); + await expect(restoreUndoneSend(pending!, MAILBOXES)).rejects.toThrow(); + expect(api.restoreEmailToDraft).not.toHaveBeenCalled(); + useAccountStore.setState({ accounts: [] }); + }); + + it('takes the undo bar away once another login is active', () => { + useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] }); + switchToOtherServer(); + expect(useSendUndoStore.getState().pending).toBeNull(); + }); + + it('keeps the undo bar while the same login stays active', () => { + useSendUndoStore.getState().recordHeldSend(HELD, 10, { identityId: 'id-1', from: [SENDER] }); + useAccountStore.setState({ defaultAccountId: LOGIN }); + expect(useSendUndoStore.getState().pending).not.toBeNull(); }); it('keeps the entry when the cancel fails, and says so', async () => { diff --git a/src/stores/send-undo-store.ts b/src/stores/send-undo-store.ts index 02fef51c..1bf0b2e8 100644 --- a/src/stores/send-undo-store.ts +++ b/src/stores/send-undo-store.ts @@ -14,7 +14,9 @@ import { NoLongerScheduledError } from '../api/jmap-result'; import { pruneEmailSubmissions } from '../api/submissions'; import type { EmailAddress, Mailbox } from '../api/types'; import { draftContextFromEmail } from '../lib/draft-context'; +import { composerLoginMoved, servedLoginOf } from '../lib/draft-login'; import { mailboxesOfAccount } from '../lib/mailbox-tree'; +import { useAccountStore } from './account-store'; import { partlyRefusedWarning, rescheduleFailureNotice } from '../lib/send-failure-message'; import type { ComposeDraftContext } from '../navigation/types'; import { t } from './locale-store'; @@ -24,8 +26,19 @@ export interface PendingUndoSend { emailSubmissionId: string; emailId: string; identityId: string; - /** Account holding the submission: a shared account for a send from its identity. */ + /** + * JMAP account holding the submission: a shared account for a send from + * its identity, else the login's own. Recorded either way: submission ids + * repeat across accounts and servers. + */ accountId?: string; + /** + * The login (registry id, lib/draft-login) the message was sent through. + * Undo and Send now act through that login only: after a switch the same + * submission id names another message on the other server (webmail + * GHSA-8x23). + */ + login?: string | null; from?: EmailAddress[]; to?: EmailAddress[]; /** ISO time the server will release the message. */ @@ -36,7 +49,32 @@ export interface PendingUndoSend { } /** What the sender knows about a held send beyond the server's answer. */ -export type HeldSendDetails = Pick; +export type HeldSendDetails = Pick; + +/** The login the JMAP client works for now, or the registry's active one while it connects. */ +export function currentSendLogin(): string | null { + return servedLoginOf(jmapClient) ?? useAccountStore.getState().activeAccountId; +} + +/** Whether Undo or Send now of `entry` would now go through another login. */ +export function pendingSendLoginMoved(entry: Pick): boolean { + return composerLoginMoved(entry.login, { + activeAccountId: useAccountStore.getState().activeAccountId, + servedLogin: servedLoginOf(jmapClient), + }); +} + +function refuseOtherLogin(entry: PendingUndoSend): void { + const owner = entry.login ? useAccountStore.getState().getAccountById(entry.login) : undefined; + toast.error( + t('email_composer.undo_send_other_login', 'This message was sent from another account'), + t( + 'email_composer.undo_send_other_login_body', + 'Switch back to {account} to undo it or send it now. It goes out when the delay ends.', + { account: owner?.email || owner?.displayName || '' }, + ), + ); +} interface SendUndoState { pending: PendingUndoSend | null; @@ -71,6 +109,10 @@ export const useSendUndoStore = create((set, get) => ({ if (!holdForSeconds || !result.scheduled || !result.emailSubmissionId || !result.emailId) return false; get().setPending({ ...details, + accountId: details.accountId ?? jmapClient.connectedAccountId ?? undefined, + // The sender reads the login before it sends; a switch may land while + // the request is out. + login: details.login !== undefined ? details.login : currentSendLogin(), emailSubmissionId: result.emailSubmissionId, emailId: result.emailId, sendAt: result.sendAt, @@ -84,6 +126,10 @@ export const useSendUndoStore = create((set, get) => ({ undo: async () => { const entry = get().pending; if (!entry || get().busy) return false; + if (pendingSendLoginMoved(entry)) { + refuseOtherLogin(entry); + return false; + } set({ busy: true }); try { await cancelScheduledSend(entry.emailSubmissionId, entry.accountId); @@ -105,6 +151,10 @@ export const useSendUndoStore = create((set, get) => ({ sendNow: async () => { const entry = get().pending; if (!entry || get().busy) return false; + if (pendingSendLoginMoved(entry)) { + refuseOtherLogin(entry); + return false; + } set({ busy: true }); try { const result = await rescheduleScheduledSend( @@ -151,6 +201,8 @@ export async function restoreUndoneSend( entry: PendingUndoSend, mailboxes: Mailbox[], ): Promise { + // The ids name other messages in another login (see pendingSendLoginMoved). + if (pendingSendLoginMoved(entry)) throw new Error('The account was switched'); const shared = !!entry.accountId && entry.accountId !== jmapClient.accountId; const accountId = shared ? entry.accountId : undefined; const scope = mailboxesOfAccount(mailboxes, accountId); @@ -166,3 +218,12 @@ export async function restoreUndoneSend( } return draftContextFromEmail(await getFullEmail(entry.emailId, accountId), accountId); } + +// A switch to another login takes the undo bar away, since its actions +// would reach the other server. The held message still goes out when the +// delay ends; back in its account, Scheduled lists it until then. +useAccountStore.subscribe((state, prev) => { + if (state.activeAccountId === prev.activeAccountId) return; + const { pending } = useSendUndoStore.getState(); + if (pending?.login && pending.login !== state.activeAccountId) useSendUndoStore.setState({ pending: null }); +});