mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 16:13:15 +00:00
fix: send via the identity's own account client so DKIM matches the From domain #461
This commit is contained in:
@@ -1979,7 +1979,10 @@ export function EmailComposer({
|
||||
const sendHandledByPlugin = (await emailHooks.onComposeSend.intercept(composeSendRequest)) === false;
|
||||
if (sendHandledByPlugin) {
|
||||
if (finalDraftId) {
|
||||
client?.deleteEmail(finalDraftId).catch((err) => {
|
||||
// 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) => {
|
||||
debug.warn('email', 'Plugin handled the send, but draft cleanup failed:', err);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1415,6 +1415,9 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
|
||||
fromEmail?: string;
|
||||
fromName?: string;
|
||||
identityId?: string;
|
||||
/** Local account owning the selected identity, when it came from the
|
||||
* cross-account From dropdown (Pro / embedded multi-account). */
|
||||
localAccountId?: string;
|
||||
envelopeMailFrom?: string;
|
||||
attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>;
|
||||
inReplyTo?: string[];
|
||||
@@ -1424,6 +1427,15 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
|
||||
}) => {
|
||||
if (!client) return;
|
||||
|
||||
// Sending from an identity that belongs to another connected account has
|
||||
// to go through that account's own client: the identity id is only valid
|
||||
// on its own server, and an unknown id makes the server fall back to the
|
||||
// active account's primary identity - which then signs the message with
|
||||
// the wrong DKIM key (#461). The draft was saved against the same account.
|
||||
const sendClient = data.localAccountId
|
||||
? (useAuthStore.getState().getClientForAccount(data.localAccountId) ?? client)
|
||||
: client;
|
||||
|
||||
// Separates "the submission failed" from "a follow-up step failed". Only the
|
||||
// former may propagate: the composer treats a resolved onSend as a delivered
|
||||
// message and wipes its fields, so swallowing a real send failure here lost
|
||||
@@ -1434,7 +1446,7 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
|
||||
const effectiveMode = pendingDraft?.mode ?? composerMode;
|
||||
const originalEmailId = selectedEmail?.id;
|
||||
|
||||
const result = await sendEmail(client, 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 });
|
||||
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, localAccountId: data.localAccountId });
|
||||
submitted = true;
|
||||
setShowComposer(false);
|
||||
if (result.filingError) {
|
||||
@@ -1664,6 +1676,11 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
|
||||
|
||||
lastUndoToastSubmissionRef.current = pendingUndoSend.submissionId;
|
||||
const pending = pendingUndoSend;
|
||||
// The submission lives on the account that owns the sending identity,
|
||||
// which is not always the active one in the multi-account shell (#461).
|
||||
const undoClient = pending.localAccountId
|
||||
? (useAuthStore.getState().getClientForAccount(pending.localAccountId) ?? client)
|
||||
: client;
|
||||
const undoDurationMs = Math.max(sendDelaySeconds, 8) * 1000;
|
||||
|
||||
toast.success(t('email_viewer.scheduled_send_created'), {
|
||||
@@ -1674,7 +1691,7 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
|
||||
onClick: () => {
|
||||
void (async () => {
|
||||
try {
|
||||
await client.rescheduleEmailSubmission(
|
||||
await undoClient.rescheduleEmailSubmission(
|
||||
pending.submissionId,
|
||||
pending.emailId!,
|
||||
pending.identityId!,
|
||||
@@ -1694,7 +1711,7 @@ export function MailApp({ linkSegments }: MailAppProps = {}) {
|
||||
onClick: () => {
|
||||
void (async () => {
|
||||
try {
|
||||
const restored = await cancelUndoSend(client, pending);
|
||||
const restored = await cancelUndoSend(undoClient, pending);
|
||||
if (restored && !pending.isSmime) {
|
||||
await handleEditDraft(restored);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
import { render, screen, fireEvent, waitFor } from '@testing-library/react';
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import React from 'react';
|
||||
|
||||
/**
|
||||
* #461 - the Pro "From" dropdown lists identities from every connected
|
||||
* account. Sending through the *active* account's client while using another
|
||||
* account's identity makes the server fall back to its own primary identity,
|
||||
* so the message goes out DKIM-signed with the wrong domain key. The compose
|
||||
* tab must submit through the client of the account owning the identity.
|
||||
*/
|
||||
|
||||
// The composer itself is irrelevant here - stub it down to a button that
|
||||
// fires the onSend payload the real composer builds for a cross-account
|
||||
// identity (raw identity id + owning local account id).
|
||||
const payloadRef = vi.hoisted(() => ({ current: {} as Record<string, unknown> }));
|
||||
|
||||
vi.mock('@/components/email/email-composer', () => ({
|
||||
EmailComposer: ({ onSend }: { onSend?: (d: Record<string, unknown>) => Promise<void> }) =>
|
||||
React.createElement(
|
||||
'button',
|
||||
{ onClick: () => { void onSend?.(payloadRef.current); } },
|
||||
'send',
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock('@/components/error', () => ({
|
||||
ErrorBoundary: ({ children }: { children: React.ReactNode }) => React.createElement(React.Fragment, null, children),
|
||||
ComposerErrorFallback: () => null,
|
||||
}));
|
||||
|
||||
vi.mock('next-intl', () => ({
|
||||
useTranslations: () => (key: string) => key,
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/debug', () => ({
|
||||
debug: { log: () => {}, warn: () => {}, error: () => {} },
|
||||
}));
|
||||
|
||||
vi.mock('@/stores/toast-store', () => ({
|
||||
toast: { error: () => {}, success: () => {}, warning: () => {} },
|
||||
}));
|
||||
|
||||
// vi.mock factories are hoisted above module-level consts, so the clients
|
||||
// have to be created in a hoisted block to be visible inside them.
|
||||
const { activeClient, otherClient } = vi.hoisted(() => {
|
||||
const makeClient = (label: string) => ({
|
||||
label,
|
||||
getAccountId: () => `jmap-${label}`,
|
||||
setKeyword: async () => {},
|
||||
deleteEmail: async () => {},
|
||||
getThreadEmails: async () => [],
|
||||
});
|
||||
return { activeClient: makeClient('account-1'), otherClient: makeClient('account-2') };
|
||||
});
|
||||
|
||||
vi.mock('@/stores/auth-store', () => {
|
||||
const state = {
|
||||
client: activeClient,
|
||||
activeAccountId: 'local-1',
|
||||
getClientForAccount: (id: string) => (id === 'local-2' ? otherClient : undefined),
|
||||
};
|
||||
const hook = (sel?: (s: typeof state) => unknown) =>
|
||||
typeof sel === 'function' ? sel(state) : state;
|
||||
hook.getState = () => state;
|
||||
return { useAuthStore: hook };
|
||||
});
|
||||
|
||||
const { sendEmail } = vi.hoisted(() => ({
|
||||
sendEmail: vi.fn(async (...args: unknown[]) => {
|
||||
void args;
|
||||
return { scheduled: false };
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('@/stores/email-store', () => {
|
||||
const state = {
|
||||
sendEmail: (...args: unknown[]) => sendEmail(...args),
|
||||
refreshCurrentMailbox: async () => {},
|
||||
fetchScheduledEmails: async () => {},
|
||||
refreshScheduledMetadata: async () => {},
|
||||
isScheduledView: false,
|
||||
emails: [],
|
||||
expandedThreadIds: new Set<string>(),
|
||||
threadEmailsCache: new Map(),
|
||||
};
|
||||
const hook = (sel?: (s: typeof state) => unknown) =>
|
||||
typeof sel === 'function' ? sel(state) : state;
|
||||
hook.getState = () => state;
|
||||
hook.setState = () => {};
|
||||
return { useEmailStore: hook };
|
||||
});
|
||||
|
||||
vi.mock('@/stores/pro-tab-store', () => {
|
||||
const state = {
|
||||
closeTab: () => {},
|
||||
updateTabTitle: () => {},
|
||||
updateComposeDraft: () => {},
|
||||
};
|
||||
const hook = (sel?: (s: typeof state) => unknown) =>
|
||||
typeof sel === 'function' ? sel(state) : state;
|
||||
hook.getState = () => state;
|
||||
return {
|
||||
useProTabStore: hook,
|
||||
registerProTabCloseInterceptor: () => () => {},
|
||||
};
|
||||
});
|
||||
|
||||
import { ProComposeTabBody } from '../pro-compose-tab-body';
|
||||
|
||||
const TAB_DATA = { sessionId: 1, mode: 'compose' as const, title: 'New message' };
|
||||
|
||||
const basePayload = {
|
||||
to: ['someone@example.org'],
|
||||
cc: [],
|
||||
bcc: [],
|
||||
subject: 'Hi',
|
||||
body: 'text',
|
||||
identityId: 'identity-on-account-2',
|
||||
fromEmail: 'alias@domain2.com',
|
||||
};
|
||||
|
||||
describe('Pro compose tab - cross-account send routing (#461)', () => {
|
||||
beforeEach(() => {
|
||||
sendEmail.mockClear();
|
||||
});
|
||||
|
||||
it('submits through the client of the account owning the selected identity', async () => {
|
||||
payloadRef.current = { ...basePayload, localAccountId: 'local-2' };
|
||||
render(<ProComposeTabBody tabId="tab-1" data={TAB_DATA} />);
|
||||
|
||||
fireEvent.click(screen.getByText('send'));
|
||||
|
||||
await waitFor(() => expect(sendEmail).toHaveBeenCalled());
|
||||
const args = sendEmail.mock.calls[0] as unknown[];
|
||||
expect(args[0]).toBe(otherClient);
|
||||
expect(args[6]).toBe('identity-on-account-2');
|
||||
// The owning account travels on to the store so undo-send can cancel the
|
||||
// submission on the right server.
|
||||
expect(args[args.length - 1]).toMatchObject({ localAccountId: 'local-2' });
|
||||
});
|
||||
|
||||
it('falls back to the active client for same-account identities', async () => {
|
||||
payloadRef.current = { ...basePayload };
|
||||
render(<ProComposeTabBody tabId="tab-1" data={TAB_DATA} />);
|
||||
|
||||
fireEvent.click(screen.getByText('send'));
|
||||
|
||||
await waitFor(() => expect(sendEmail).toHaveBeenCalled());
|
||||
expect(sendEmail.mock.calls[0][0]).toBe(activeClient);
|
||||
});
|
||||
|
||||
it('falls back to the active client when the owning account is no longer connected', async () => {
|
||||
payloadRef.current = { ...basePayload, localAccountId: 'local-gone' };
|
||||
render(<ProComposeTabBody tabId="tab-1" data={TAB_DATA} />);
|
||||
|
||||
fireEvent.click(screen.getByText('send'));
|
||||
|
||||
await waitFor(() => expect(sendEmail).toHaveBeenCalled());
|
||||
expect(sendEmail.mock.calls[0][0]).toBe(activeClient);
|
||||
});
|
||||
});
|
||||
@@ -52,13 +52,22 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) {
|
||||
|
||||
const handleSend = useCallback(async (sendData: Parameters<NonNullable<React.ComponentProps<typeof EmailComposer>['onSend']>>[0]) => {
|
||||
if (!client) return;
|
||||
// The Pro From dropdown lists identities from every connected account, so
|
||||
// the chosen identity may belong to a different account than the active
|
||||
// one. Submit through that account's own client - the identity id is only
|
||||
// valid on its own server, and submitting on the active account's session
|
||||
// makes the server sign with the wrong DKIM key (#461). The draft was
|
||||
// already saved against the same account by the composer.
|
||||
const sendClient = sendData.localAccountId
|
||||
? (useAuthStore.getState().getClientForAccount(sendData.localAccountId) ?? client)
|
||||
: client;
|
||||
// See the inline composer's handleEmailSend: a resolved onSend tells the
|
||||
// composer the message is gone and it clears itself, so a genuine send
|
||||
// failure has to propagate rather than be logged away (#702).
|
||||
let submitted = false;
|
||||
try {
|
||||
const result = await sendEmail(
|
||||
client,
|
||||
sendClient,
|
||||
sendData.to,
|
||||
sendData.subject,
|
||||
sendData.body,
|
||||
@@ -74,6 +83,7 @@ export function ProComposeTabBody({ tabId, data }: ProComposeTabBodyProps) {
|
||||
sendData.references,
|
||||
sendData.delayedUntil,
|
||||
sendData.envelopeMailFrom,
|
||||
{ localAccountId: sendData.localAccountId },
|
||||
);
|
||||
submitted = true;
|
||||
|
||||
|
||||
+21
-3
@@ -21,7 +21,16 @@ type ScheduledSubmissionMetadata = {
|
||||
|
||||
const VIRTUAL_SCHEDULED_MAILBOX_ID = '__scheduled__';
|
||||
|
||||
type PendingUndoSend = { submissionId: string; emailId?: string; identityId?: string; sendAt: string; isSmime: boolean };
|
||||
type PendingUndoSend = {
|
||||
submissionId: string;
|
||||
emailId?: string;
|
||||
identityId?: string;
|
||||
sendAt: string;
|
||||
isSmime: boolean;
|
||||
/** Local account that owns the submission, when the message was sent from
|
||||
* an identity belonging to a non-active account (#461). */
|
||||
localAccountId?: string;
|
||||
};
|
||||
|
||||
interface EmailStore {
|
||||
emails: Email[];
|
||||
@@ -156,7 +165,7 @@ interface EmailStore {
|
||||
loadMoreEmails: (client: IJMAPClient) => Promise<void>;
|
||||
fetchEmailContent: (client: IJMAPClient, emailId: string) => Promise<Email | null>;
|
||||
fetchQuota: (client: IJMAPClient) => Promise<void>;
|
||||
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 }) => Promise<SendEmailResult>;
|
||||
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; localAccountId?: string }) => Promise<SendEmailResult>;
|
||||
sendRawEmail: (client: IJMAPClient, rawMimeBlob: Blob, identityId: string, delayedUntil?: string, envelopeRecipients?: string[]) => Promise<SendEmailResult>;
|
||||
deleteEmail: (client: IJMAPClient, emailId: string, forceDelete?: boolean) => Promise<void>;
|
||||
markAsRead: (client: IJMAPClient, emailId: string, read: boolean) => Promise<void>;
|
||||
@@ -1362,7 +1371,16 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
set({
|
||||
isLoading: false,
|
||||
pendingUndoSend: result.scheduled && result.emailSubmissionId && result.sendAt
|
||||
? { submissionId: result.emailSubmissionId, emailId: result.emailId, identityId, sendAt: result.sendAt, isSmime: false }
|
||||
? {
|
||||
submissionId: result.emailSubmissionId,
|
||||
emailId: result.emailId,
|
||||
identityId,
|
||||
sendAt: result.sendAt,
|
||||
isSmime: false,
|
||||
// Cross-account send: undo/send-now must talk to the account that
|
||||
// holds the submission, not the active one (#461).
|
||||
localAccountId: options?.localAccountId,
|
||||
}
|
||||
: get().pendingUndoSend,
|
||||
});
|
||||
return result;
|
||||
|
||||
Reference in New Issue
Block a user