mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
fix: undo and send now of a delayed send only reach the login it was sent from
This commit is contained in:
@@ -340,6 +340,8 @@
|
|||||||
"remove_link": "Remove link"
|
"remove_link": "Remove link"
|
||||||
},
|
},
|
||||||
"undo_send_label": "Sending in {seconds}s…",
|
"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",
|
"upload_failed_short": "Upload failed",
|
||||||
"uploading": "Uploading...",
|
"uploading": "Uploading...",
|
||||||
"uploading_pct": "Uploading {pct}%",
|
"uploading_pct": "Uploading {pct}%",
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ import { useSettingsStore } from '../../stores/settings-store';
|
|||||||
import { useLocaleStore } from '../../stores/locale-store';
|
import { useLocaleStore } from '../../stores/locale-store';
|
||||||
import { useEmailStore } from '../../stores/email-store';
|
import { useEmailStore } from '../../stores/email-store';
|
||||||
import { toast } from '../../stores/toast-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 { sendEmail, patchKeywordsForEmails } from '../../api/email';
|
||||||
import { jmapClient } from '../../api/jmap-client';
|
import { jmapClient } from '../../api/jmap-client';
|
||||||
import { pruneEmailSubmissions } from '../../api/submissions';
|
import { pruneEmailSubmissions } from '../../api/submissions';
|
||||||
@@ -103,6 +103,9 @@ export function QuickReplyBox({ email, jmapAccountId, onMoreOptions, onSent, onF
|
|||||||
});
|
});
|
||||||
const threading = computeReplyThreadingHeaders(email);
|
const threading = computeReplyThreadingHeaders(email);
|
||||||
const holdFor = jmapClient.undoSendHold(sendDelaySeconds, jmapAccountId);
|
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(
|
const result = await sendEmail(
|
||||||
{
|
{
|
||||||
from: [{ name: identity.name, email: identity.email }],
|
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.
|
// Recorded before the flag below so its round trip doesn't eat the window.
|
||||||
useSendUndoStore.getState().recordHeldSend(result, holdFor, {
|
useSendUndoStore.getState().recordHeldSend(result, holdFor, {
|
||||||
identityId: identity.id,
|
identityId: identity.id,
|
||||||
accountId: jmapAccountId,
|
...sentThrough,
|
||||||
from: [{ name: identity.name, email: identity.email }],
|
from: [{ name: identity.name, email: identity.email }],
|
||||||
});
|
});
|
||||||
void pruneEmailSubmissions();
|
void pruneEmailSubmissions();
|
||||||
|
|||||||
@@ -2184,6 +2184,9 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
|||||||
setSending(true);
|
setSending(true);
|
||||||
try {
|
try {
|
||||||
const outgoing = buildOutgoing(primaryIdentity, liveBodyHtml, { forDraft: false });
|
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(
|
const result = await sendEmail(
|
||||||
outgoing,
|
outgoing,
|
||||||
primaryIdentity.id,
|
primaryIdentity.id,
|
||||||
@@ -2237,6 +2240,8 @@ export default function ComposeScreen({ route, navigation }: Props) {
|
|||||||
// Undo-send window: the undo bar offers Undo / Send now.
|
// Undo-send window: the undo bar offers Undo / Send now.
|
||||||
useSendUndoStore.getState().recordHeldSend(result, holdForSeconds, {
|
useSendUndoStore.getState().recordHeldSend(result, holdForSeconds, {
|
||||||
identityId: primaryIdentity.id,
|
identityId: primaryIdentity.id,
|
||||||
|
accountId: submissionAccountId,
|
||||||
|
login: ownerLoginRef.current,
|
||||||
from: outgoing.from,
|
from: outgoing.from,
|
||||||
to: [...outgoing.to, ...(outgoing.cc ?? []), ...(outgoing.bcc ?? [])],
|
to: [...outgoing.to, ...(outgoing.cc ?? []), ...(outgoing.bcc ?? [])],
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -7,11 +7,31 @@ vi.mock('../../api/email', () => ({
|
|||||||
getFullEmail: vi.fn(),
|
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', () => ({
|
vi.mock('../../api/jmap-client', () => ({
|
||||||
jmapClient: { accountId: 'primary' },
|
jmapClient: client,
|
||||||
RequestTimeoutError: class RequestTimeoutError extends Error {},
|
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.
|
// Background work nobody waits for: it never settles here.
|
||||||
vi.mock('../../api/submissions', () => ({
|
vi.mock('../../api/submissions', () => ({
|
||||||
pruneEmailSubmissions: vi.fn(() => new Promise<number>(() => undefined)),
|
pruneEmailSubmissions: vi.fn(() => new Promise<number>(() => undefined)),
|
||||||
@@ -23,6 +43,7 @@ import { NoLongerScheduledError, RecipientsRefusedError, SubmissionQuotaError }
|
|||||||
import type { Email, Mailbox } from '../../api/types';
|
import type { Email, Mailbox } from '../../api/types';
|
||||||
import { useSendUndoStore, restoreUndoneSend, type PendingUndoSend } from '../send-undo-store';
|
import { useSendUndoStore, restoreUndoneSend, type PendingUndoSend } from '../send-undo-store';
|
||||||
import { useToastStore } from '../toast-store';
|
import { useToastStore } from '../toast-store';
|
||||||
|
import { useAccountStore } from '../account-store';
|
||||||
|
|
||||||
const api = vi.mocked(emailApi);
|
const api = vi.mocked(emailApi);
|
||||||
const prune = vi.mocked(submissions.pruneEmailSubmissions);
|
const prune = vi.mocked(submissions.pruneEmailSubmissions);
|
||||||
@@ -70,6 +91,8 @@ beforeEach(() => {
|
|||||||
useSendUndoStore.setState({ pending: null, busy: false, restoredEmailId: null });
|
useSendUndoStore.setState({ pending: null, busy: false, restoredEmailId: null });
|
||||||
useToastStore.getState().clearToasts();
|
useToastStore.getState().clearToasts();
|
||||||
api.getFullEmail.mockResolvedValue({ id: 'mail-1', subject: 'Re: hi' } as Email);
|
api.getFullEmail.mockResolvedValue({ id: 'mail-1', subject: 'Re: hi' } as Email);
|
||||||
|
client.serverUrl = 'https://mail.example.com';
|
||||||
|
useAccountStore.setState({ activeAccountId: LOGIN });
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -93,9 +116,21 @@ describe('send-undo-store', () => {
|
|||||||
from: [SENDER],
|
from: [SENDER],
|
||||||
sendAt: HELD.sendAt,
|
sendAt: HELD.sendAt,
|
||||||
delaySeconds: 10,
|
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', () => {
|
it('records nothing for a send that went out at once', () => {
|
||||||
const store = useSendUndoStore.getState();
|
const store = useSendUndoStore.getState();
|
||||||
expect(store.recordHeldSend({ scheduled: false, emailSubmissionId: 'sub-1', emailId: 'mail-1' }, undefined, { identityId: 'id-1' })).toBe(false);
|
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 () => {
|
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();
|
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 () => {
|
it('keeps the entry when the cancel fails, and says so', async () => {
|
||||||
|
|||||||
@@ -14,7 +14,9 @@ import { NoLongerScheduledError } from '../api/jmap-result';
|
|||||||
import { pruneEmailSubmissions } from '../api/submissions';
|
import { pruneEmailSubmissions } from '../api/submissions';
|
||||||
import type { EmailAddress, Mailbox } from '../api/types';
|
import type { EmailAddress, Mailbox } from '../api/types';
|
||||||
import { draftContextFromEmail } from '../lib/draft-context';
|
import { draftContextFromEmail } from '../lib/draft-context';
|
||||||
|
import { composerLoginMoved, servedLoginOf } from '../lib/draft-login';
|
||||||
import { mailboxesOfAccount } from '../lib/mailbox-tree';
|
import { mailboxesOfAccount } from '../lib/mailbox-tree';
|
||||||
|
import { useAccountStore } from './account-store';
|
||||||
import { partlyRefusedWarning, rescheduleFailureNotice } from '../lib/send-failure-message';
|
import { partlyRefusedWarning, rescheduleFailureNotice } from '../lib/send-failure-message';
|
||||||
import type { ComposeDraftContext } from '../navigation/types';
|
import type { ComposeDraftContext } from '../navigation/types';
|
||||||
import { t } from './locale-store';
|
import { t } from './locale-store';
|
||||||
@@ -24,8 +26,19 @@ export interface PendingUndoSend {
|
|||||||
emailSubmissionId: string;
|
emailSubmissionId: string;
|
||||||
emailId: string;
|
emailId: string;
|
||||||
identityId: 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;
|
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[];
|
from?: EmailAddress[];
|
||||||
to?: EmailAddress[];
|
to?: EmailAddress[];
|
||||||
/** ISO time the server will release the message. */
|
/** 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. */
|
/** What the sender knows about a held send beyond the server's answer. */
|
||||||
export type HeldSendDetails = Pick<PendingUndoSend, 'identityId' | 'accountId' | 'from' | 'to'>;
|
export type HeldSendDetails = Pick<PendingUndoSend, 'identityId' | 'accountId' | 'from' | 'to' | 'login'>;
|
||||||
|
|
||||||
|
/** 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<PendingUndoSend, 'login'>): 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 {
|
interface SendUndoState {
|
||||||
pending: PendingUndoSend | null;
|
pending: PendingUndoSend | null;
|
||||||
@@ -71,6 +109,10 @@ export const useSendUndoStore = create<SendUndoState>((set, get) => ({
|
|||||||
if (!holdForSeconds || !result.scheduled || !result.emailSubmissionId || !result.emailId) return false;
|
if (!holdForSeconds || !result.scheduled || !result.emailSubmissionId || !result.emailId) return false;
|
||||||
get().setPending({
|
get().setPending({
|
||||||
...details,
|
...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,
|
emailSubmissionId: result.emailSubmissionId,
|
||||||
emailId: result.emailId,
|
emailId: result.emailId,
|
||||||
sendAt: result.sendAt,
|
sendAt: result.sendAt,
|
||||||
@@ -84,6 +126,10 @@ export const useSendUndoStore = create<SendUndoState>((set, get) => ({
|
|||||||
undo: async () => {
|
undo: async () => {
|
||||||
const entry = get().pending;
|
const entry = get().pending;
|
||||||
if (!entry || get().busy) return false;
|
if (!entry || get().busy) return false;
|
||||||
|
if (pendingSendLoginMoved(entry)) {
|
||||||
|
refuseOtherLogin(entry);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
set({ busy: true });
|
set({ busy: true });
|
||||||
try {
|
try {
|
||||||
await cancelScheduledSend(entry.emailSubmissionId, entry.accountId);
|
await cancelScheduledSend(entry.emailSubmissionId, entry.accountId);
|
||||||
@@ -105,6 +151,10 @@ export const useSendUndoStore = create<SendUndoState>((set, get) => ({
|
|||||||
sendNow: async () => {
|
sendNow: async () => {
|
||||||
const entry = get().pending;
|
const entry = get().pending;
|
||||||
if (!entry || get().busy) return false;
|
if (!entry || get().busy) return false;
|
||||||
|
if (pendingSendLoginMoved(entry)) {
|
||||||
|
refuseOtherLogin(entry);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
set({ busy: true });
|
set({ busy: true });
|
||||||
try {
|
try {
|
||||||
const result = await rescheduleScheduledSend(
|
const result = await rescheduleScheduledSend(
|
||||||
@@ -151,6 +201,8 @@ export async function restoreUndoneSend(
|
|||||||
entry: PendingUndoSend,
|
entry: PendingUndoSend,
|
||||||
mailboxes: Mailbox[],
|
mailboxes: Mailbox[],
|
||||||
): Promise<ComposeDraftContext> {
|
): Promise<ComposeDraftContext> {
|
||||||
|
// 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 shared = !!entry.accountId && entry.accountId !== jmapClient.accountId;
|
||||||
const accountId = shared ? entry.accountId : undefined;
|
const accountId = shared ? entry.accountId : undefined;
|
||||||
const scope = mailboxesOfAccount(mailboxes, accountId);
|
const scope = mailboxesOfAccount(mailboxes, accountId);
|
||||||
@@ -166,3 +218,12 @@ export async function restoreUndoneSend(
|
|||||||
}
|
}
|
||||||
return draftContextFromEmail(await getFullEmail(entry.emailId, accountId), accountId);
|
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 });
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user