fix: preserve message ownership in Pro blob downloads

This commit is contained in:
VasilisNtovolos
2026-10-20 14:33:42 +03:00
parent 640d4b8c2e
commit 41b70853fa
2 changed files with 174 additions and 7 deletions
@@ -75,6 +75,8 @@ vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: ({ o
const { MailApp } = await import('../mail-app');
const { ConversationEmailBody } = await import('@/components/email/thread-conversation-view');
const { EmailViewer } = await import('@/components/email/email-viewer');
const { ProEmailTabBody } = await import('@/components/pro/pro-email-tab-body');
const { ProFolderTabBody } = await import('@/components/pro/pro-folder-tab-body');
const sharedMailbox = { id: 'nrichmond:inbox', originalId: 'inbox', name: 'Shared', isShared: true, accountId: 'nrichmond' } as Mailbox;
const primaryMailbox = { id: 'inbox', name: 'Inbox', role: 'inbox' } as Mailbox;
@@ -602,3 +604,132 @@ describe('conversation blob routing', () => {
expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:owner-a');
});
});
// Pro hosts must carry the same message-scoped context as MailApp, including
// when the global mail selection is absent or belongs to another message.
type ProBlobAction = 'download' | 'preview click' | 'CID' | 'export';
const proBlobActions: ProBlobAction[] = ['download', 'preview click', 'CID', 'export'];
const proRoutes = [
{ label: 'direct shared folder', secondary: false, owner: 'nrichmond', selection: 'null', unified: false },
{ label: 'secondary shared folder', secondary: true, owner: 'nrichmond', selection: 'unrelated', unified: false },
{ label: 'primary folder', secondary: false, owner: 'postal12345', selection: 'null', unified: false },
{ label: 'secondary primary folder', secondary: true, owner: 'secondary-primary', selection: 'unrelated', unified: false },
{ label: 'Unified tab, null selection', secondary: true, owner: 'nrichmond', selection: 'null', unified: true },
{ label: 'Unified tab, unrelated selection', secondary: true, owner: 'nrichmond', selection: 'unrelated', unified: true },
{ label: 'explicit secondary email tab', secondary: true, owner: 'nrichmond', selection: 'null', unified: false, emailTab: true },
];
async function openProMessage(route: typeof proRoutes[number], action: ProBlobAction) {
const { a, b, fetchMock } = await conversationClients();
const origin = route.secondary ? b : a;
const full = message({
subject: 'Pro message',
attachments: [{ ...cardAttachment, name: 'photo.png', ...(action === 'CID' ? { cid: 'test-image' } : {}) }],
...(action === 'CID' ? {
textBody: [], htmlBody: [{ partId: '1', blobId: 'html-blob', type: 'text/html', size: 30 }],
bodyValues: { '1': { value: '<img src="cid:test-image">', isTruncated: false, isEncodingProblem: false } },
} : {}),
});
const stub = { ...full, sourceClientAccountId: 'login-other', sourceAccountId: route.owner };
vi.spyOn(a, 'getEmail').mockResolvedValue(full);
vi.spyOn(b, 'getEmail').mockResolvedValue(full);
useSettingsStore.setState({ emailDownloadTemplate: '{subject}', mailAttachmentAction: action === 'preview click' ? 'preview' : 'download' });
useEmailStore.setState({
emails: route.unified ? [stub] : [],
selectedEmail: route.selection === 'null' ? null : message({ id: 'unrelated', sourceClientAccountId: 'login-primary', sourceAccountId: 'unrelated-owner' }),
isUnifiedView: route.unified, selectedMailbox: route.unified ? '__unified_inbox__' : sharedMailbox.id,
accountMailboxes: { 'login-primary': [primaryMailbox, sharedMailbox], 'login-other': [primaryMailbox, sharedMailbox] },
});
if (route.unified || route.emailTab) {
render(<ProEmailTabBody tabId="reader" data={{ accountId: route.owner, clientAccountId: route.emailTab ? 'login-other' : undefined, emailId: full.id, mailboxId: '__unified_inbox__', title: full.subject! }} />);
} else {
const mailbox = route.owner === 'nrichmond' ? sharedMailbox : primaryMailbox;
vi.spyOn(origin, 'getEmails').mockResolvedValue({ emails: [full], total: 1, hasMore: false });
render(<ProFolderTabBody tabId="folder" data={{ accountId: route.secondary ? 'login-other' : 'login-primary', mailboxId: mailbox.id, title: mailbox.name }} />);
fireEvent.click(await screen.findByText('Pro message'));
}
if (action === 'CID') await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
else await screen.findAllByText('photo.png');
return { a, b, origin, fetchMock, full };
}
async function actOnProBlob(action: ProBlobAction) {
if (action === 'download') await downloadAttachment();
if (action === 'preview click') {
fireEvent.click((await screen.findAllByText('photo.png'))[0]);
// Preserve Pro's existing action: Preview clicks download, with no modal.
expect(screen.queryByText('Preview fetch')).not.toBeInTheDocument();
}
if (action === 'export') await exportEmail();
}
function expectProBlob(route: typeof proRoutes[number], action: ProBlobAction, index = 0) {
const host = route.secondary ? 'secondary' : 'primary';
const blobPath = action === 'export' ? 'raw-blob/Pro%20message.eml?accept=message%2Frfc822' : 'same-blob/photo.png?accept=image%2Fpng';
expectBlobRequest(`https://${host}.example.com/download/${route.owner}/${blobPath}`, `${host}-login`, index);
}
describe('Pro blob routing', () => {
it.each(proRoutes.flatMap(route => proBlobActions.map(action => ({ ...route, action }))))(
'$label: $action uses the message owner and authenticated client', async route => {
const { origin, fetchMock } = await openProMessage(route, route.action);
await actOnProBlob(route.action);
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
expectProBlob(route, route.action);
if (route.owner === 'nrichmond') expect(origin.getEmail).toHaveBeenCalledWith('email', route.owner);
},
);
it.each([proRoutes[0], proRoutes[1], proRoutes[4]].flatMap(route => proBlobActions.map(action => ({ ...route, action }))))(
'$label: $action retains its source after account and selection changes', async route => {
const { a, b, fetchMock } = await openProMessage(route, route.action);
if (route.action === 'CID') await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
// Leave the loaded reader mounted while a different Mail pane navigates.
act(() => {
useEmailStore.setState({ emails: [], selectedEmail: message({ id: 'other', sourceClientAccountId: 'login-primary', sourceAccountId: 'unrelated-owner' }), selectedMailbox: 'inbox', isUnifiedView: false });
useAuthStore.setState({ client: route.secondary ? b : a, activeAccountId: route.secondary ? 'login-other' : 'login-primary' });
});
// Switch through null, as reconnecting account switches can do.
act(() => useAuthStore.setState({ client: null }));
act(() => useAuthStore.setState({ client: route.secondary ? a : b, activeAccountId: route.secondary ? 'login-primary' : 'login-other' }));
await act(async () => {});
await actOnProBlob(route.action);
await waitFor(() => expect(fetchMock).toHaveBeenCalled());
for (let index = 0; index < fetchMock.mock.calls.length; index++) expectProBlob(route, route.action, index);
},
);
it('retains an in-flight Unified load across account and selection changes', async () => {
const { a, b, fetchMock } = await conversationClients();
let finish!: (email: Email) => void;
vi.spyOn(b, 'getEmail').mockImplementation(() => new Promise(resolve => { finish = resolve; }));
const wrongClient = vi.spyOn(a, 'getEmail');
useEmailStore.setState({ emails: [message({ sourceClientAccountId: 'login-other', sourceAccountId: 'nrichmond' })], selectedEmail: null, isUnifiedView: true });
render(<ProEmailTabBody tabId="reader" data={{ accountId: 'nrichmond', emailId: 'email', mailboxId: null, title: 'Loading' }} />);
await waitFor(() => expect(b.getEmail).toHaveBeenCalledWith('email', 'nrichmond'));
act(() => {
useEmailStore.setState({ emails: [], selectedEmail: message({ id: 'unrelated' }), isUnifiedView: false, selectedMailbox: 'inbox' });
useAuthStore.setState({ client: b, activeAccountId: 'login-other' });
});
await act(async () => finish(message({ attachments: [{ ...cardAttachment, name: 'photo.png' }] })));
await downloadAttachment();
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
expectProBlob(proRoutes[4], 'download');
expect(wrongClient).not.toHaveBeenCalled();
expect(b.getEmail).toHaveBeenCalledTimes(1);
});
it('changes CID routing when a reused reader opens identical ids on another client/owner', async () => {
const { a, b, fetchMock } = await conversationClients();
const full = inlineMessage({ attachments: [{ ...cardAttachment, name: 'photo.png', cid: 'test-image' }] });
vi.spyOn(a, 'getEmail').mockResolvedValue(full);
vi.spyOn(b, 'getEmail').mockResolvedValue({ ...full });
const reader = (login: string, owner: string) => <ProEmailTabBody tabId="reader" data={{ accountId: owner, clientAccountId: login, emailId: full.id, mailboxId: null, title: full.subject! }} />;
const { rerender } = render(reader('login-primary', 'owner-a'));
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
expectBlobRequest('https://primary.example.com/download/owner-a/same-blob/photo.png?accept=image%2Fpng', 'primary-login');
rerender(reader('login-other', 'owner-b'));
await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2));
expectBlobRequest('https://secondary.example.com/download/owner-b/same-blob/photo.png?accept=image%2Fpng', 'secondary-login', 1);
});
});
+43 -7
View File
@@ -5,7 +5,7 @@ import { useTranslations } from "next-intl";
import { EmailViewer } from "@/components/email/email-viewer";
import { ErrorBoundary, EmailViewerErrorFallback } from "@/components/error";
import { useAuthStore } from "@/stores/auth-store";
import { useEmailStore } from "@/stores/email-store";
import { useEmailStore, resolveEmailBlobContext, type EmailBlobContext } from "@/stores/email-store";
import { useIdentityStore } from "@/stores/identity-store";
import { useProMultiAccountIdentities } from "@/hooks/use-pro-multi-account-identities";
import { findDraftIdentityId } from "@/lib/reply-identity";
@@ -100,6 +100,17 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
const openComposeTabNow = useProTabStore((s) => s.openComposeTab);
const [email, setEmail] = useState<Email | null>(null);
const [blobSource, setBlobSource] = useState<EmailBlobContext>({ client: null });
// A reader owns its load, including an in-flight one across account switches.
// Only a different message or explicit tab context starts a new load.
const loadRef = useRef<{
emailId: string;
clientOverride: typeof clientOverride;
accountId: typeof accountId;
blobSource: EmailBlobContext;
promise: Promise<Email | null>;
applied: boolean;
} | null>(null);
const [isLoading, setIsLoading] = useState(true);
const composerSessionIdRef = useRef(0);
@@ -137,11 +148,34 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
useEffect(() => {
let cancelled = false;
if (!client) return;
let load = loadRef.current;
if (!load || load.emailId !== emailId || load.clientOverride !== clientOverride || load.accountId !== accountId) {
const state = useEmailStore.getState();
const ref = state.selectedEmail?.id === emailId
? state.selectedEmail
: state.emails.find((e) => e.id === emailId);
// Folder/search panes supply their own context. A primary folder must
// not inherit an unrelated globally selected shared folder's owner.
const source = accountId || clientOverride
? { client, accountId: accountId ?? client.getAccountId() }
: resolveEmailBlobContext(ref ?? null, client);
load = {
emailId, clientOverride, accountId, blobSource: source, applied: false,
promise: accountId || clientOverride
? client.getEmail(emailId, source.accountId)
: fetchEmailContent(client, emailId),
};
loadRef.current = load;
}
const pending = load;
if (pending.applied) return;
setIsLoading(true);
(accountId ? client.getEmail(emailId, accountId) : fetchEmailContent(client, emailId))
pending.promise
.then((loaded) => {
if (cancelled) return;
pending.applied = true;
setEmail(loaded);
setBlobSource(pending.blobSource);
if (loaded) onLoadedRef.current?.(loaded);
})
.catch((err) => {
@@ -153,7 +187,7 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
return () => {
cancelled = true;
};
}, [client, emailId, accountId, fetchEmailContent]);
}, [client, clientOverride, emailId, accountId, fetchEmailContent]);
const currentMailboxRole = useMemo(() => {
if (!email) return undefined;
@@ -326,14 +360,15 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
}
}, [client, email, moveToMailbox, onClose]);
const handleDownloadAttachment = useCallback(async (blobId: string, name: string, type?: string) => {
if (!client) return;
const handleDownloadAttachment = useCallback(async (blobId: string, name: string, type?: string, _forceDownload?: boolean, source?: EmailBlobContext) => {
const context = source ?? blobSource;
if (!context.client) return;
try {
await client.downloadBlob(blobId, name, type);
await context.client.downloadBlob(blobId, name, type, context.accountId);
} catch (err) {
console.error('Download failed:', err);
}
}, [client]);
}, [blobSource]);
const handleQuickReply = useCallback(async (body: string) => {
handleReply(body);
@@ -394,6 +429,7 @@ export function ProEmailView({ emailId, client: clientOverride, accountId, onLoa
<ErrorBoundary fallback={EmailViewerErrorFallback}>
<EmailViewer
email={email}
blobSource={blobSource}
isLoading={isLoading}
onReply={handleReply}
onReplyAll={handleReplyAll}