diff --git a/app/(main)/[locale]/page.tsx b/app/(main)/[locale]/page.tsx index 94d884e6..464b795c 100644 --- a/app/(main)/[locale]/page.tsx +++ b/app/(main)/[locale]/page.tsx @@ -110,7 +110,7 @@ export default function Home() { const [conversationEmails, setConversationEmails] = useState([]); const [isLoadingConversation, setIsLoadingConversation] = useState(false); const [rateLimitSecondsLeft, setRateLimitSecondsLeft] = useState(null); - const [previewAttachment, setPreviewAttachment] = useState<{ blobId: string; name: string; type?: string } | null>(null); + const [previewAttachment, setPreviewAttachment] = useState<{ blobId: string; name: string; type?: string; accountId?: string; clientAccountId?: string } | null>(null); const [pendingMailtoAccountChoice, setPendingMailtoAccountChoice] = useState(null); const [isProtocolAccountSwitching, setIsProtocolAccountSwitching] = useState(false); const markAsReadTimeoutRef = useRef(null); @@ -2292,41 +2292,64 @@ export default function Home() { }; }, []); + // Blobs are scoped per JMAP account. In the unified/All-Mail view the open + // message may belong to another login (route to its client) or to a delegated + // shared account (same client, but the owner's accountId in the download URL). + // Resolve both from the email's source so attachments on cross-account + // messages can be viewed/downloaded instead of 404ing against the active + // account. + const resolveBlobSource = useCallback((email: typeof selectedEmail) => { + const clientAccountId = isUnifiedView ? email?.sourceClientAccountId : undefined; + const blobClient = clientAccountId + ? (useAuthStore.getState().getClientForAccount(clientAccountId) ?? client) + : client; + const accountId = isUnifiedView ? email?.sourceAccountId : undefined; + return { blobClient, accountId, clientAccountId }; + }, [isUnifiedView, client]); + const handleDownloadAttachment = async (blobId: string, name: string, type?: string, forceDownload?: boolean) => { - if (!client) return; + const { blobClient, accountId, clientAccountId } = resolveBlobSource(selectedEmail); + if (!blobClient) return; try { const { mailAttachmentAction } = useSettingsStore.getState(); if (!forceDownload && mailAttachmentAction === 'preview' && isFilePreviewable(name, type)) { - setPreviewAttachment({ blobId, name, type }); + setPreviewAttachment({ blobId, name, type, accountId, clientAccountId }); return; } - await client.downloadBlob(blobId, name, type); + await blobClient.downloadBlob(blobId, name, type, accountId); } catch (error) { console.error("Failed to download attachment:", error); } }; - const handlePreviewAttachmentDownload = useCallback(async () => { - if (!client || !previewAttachment) return; + const previewBlobClient = useCallback(() => { + const id = previewAttachment?.clientAccountId; + return id ? (useAuthStore.getState().getClientForAccount(id) ?? client) : client; + }, [previewAttachment, client]); - await client.downloadBlob(previewAttachment.blobId, previewAttachment.name, previewAttachment.type); - }, [client, previewAttachment]); + const handlePreviewAttachmentDownload = useCallback(async () => { + const c = previewBlobClient(); + if (!c || !previewAttachment) return; + + await c.downloadBlob(previewAttachment.blobId, previewAttachment.name, previewAttachment.type, previewAttachment.accountId); + }, [previewBlobClient, previewAttachment]); const getPreviewAttachmentContent = useCallback(async () => { - if (!client || !previewAttachment) { + const c = previewBlobClient(); + if (!c || !previewAttachment) { throw new Error('No attachment selected'); } - const blob = await client.fetchBlob(previewAttachment.blobId, previewAttachment.name, previewAttachment.type); + const blob = await c.fetchBlob(previewAttachment.blobId, previewAttachment.name, previewAttachment.type, previewAttachment.accountId); return { blob, contentType: previewAttachment.type || blob.type || 'application/octet-stream', }; - }, [client, previewAttachment]); + }, [previewBlobClient, previewAttachment]); const handleQuickReply = async (body: string) => { if (!client || !selectedEmail) return; diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 4f12dab6..feee8aaf 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -3704,6 +3704,8 @@ export function EmailViewer({ )} title={`${opensPreview ? tFiles('preview') : t('download')} ${getAttachmentDisplayName(attachment.name, attachment.type)}`} onClick={() => handleEffectiveAttachmentOpen(attachment)} + data-testid="attachment" + data-attachment-name={attachment.name} draggable={dragProps.draggable} onPointerEnter={dragProps.onPointerEnter} onDragStart={dragProps.onDragStart} @@ -4478,6 +4480,8 @@ export function EmailViewer({ )} title={`${opensPreview ? tFiles('preview') : t('download')} ${getAttachmentDisplayName(attachment.name, attachment.type)}`} onClick={() => handleEffectiveAttachmentOpen(attachment)} + data-testid="attachment" + data-attachment-name={attachment.name} draggable={dragProps.draggable} onPointerEnter={dragProps.onPointerEnter} onDragStart={dragProps.onDragStart} @@ -4622,6 +4626,8 @@ export function EmailViewer({ )} title={`${opensPreview ? tFiles('preview') : t('download')} ${getAttachmentDisplayName(attachment.name, attachment.type)}`} onClick={() => handleEffectiveAttachmentOpen(attachment)} + data-testid="attachment" + data-attachment-name={attachment.name} draggable={dragProps.draggable} onPointerEnter={dragProps.onPointerEnter} onDragStart={dragProps.onDragStart} diff --git a/integration/README.md b/integration/README.md index c1fe6111..d6e6c50f 100644 --- a/integration/README.md +++ b/integration/README.md @@ -85,7 +85,9 @@ integration/ ├── 05-actions.spec.ts # context-menu read/unread, delete, spam (inbox) ├── 06-shared-folders.spec.ts # delegated folder: appears + read/unread/delete/spam ├── 07-drafts.spec.ts # multiple recipients, changed sender, continue-draft button - └── 08-shared-moves.spec.ts # moving mail across own/shared and shared/shared + ├── 08-shared-moves.spec.ts # moving mail across own/shared and shared/shared + ├── 09-live-counters.spec.ts # live unified/All-Mail counters (login + shared) + └── 10-attachments.spec.ts # cross-account attachment download from All Mail ``` ## Findings surfaced by the suite @@ -110,6 +112,10 @@ because the UI behaviour is currently incomplete. Worth a look: "Move to" submenu offers the shared folder, but clicking it is a no-op. Shared ⇆ shared (same owner) moves work. Pinned with `test.fail` in `08-shared-moves`. +- **Cross-account attachments (fixed).** Blobs are account-scoped, so viewing/ + downloading an attachment on an All-Mail message from another account 404'd + against the active account. The download/preview path now routes to the + message's owning client + accountId (`10-attachments`). ## How the tests work diff --git a/integration/tests/10-attachments.spec.ts b/integration/tests/10-attachments.spec.ts new file mode 100644 index 00000000..159e5fa8 --- /dev/null +++ b/integration/tests/10-attachments.spec.ts @@ -0,0 +1,77 @@ +import { test, expect } from '@playwright/test'; +import { ACCOUNTS } from './helpers/config'; +import { sendMail } from './helpers/smtp'; +import { JmapClient } from './helpers/jmap'; +import { + login, + addAccount, + switchAccount, + seedSettings, + folderRow, + openFolder, + emailItem, + expectEmailVisible, + forceSync, +} from './helpers/app'; + +/** + * Attachments on a message that belongs to a *different* account, opened from + * the cross-account All-Mail view. Blobs are account-scoped, so downloading one + * must route to the owning account's client + accountId — otherwise it 404s + * against the active account (the reported bug). + */ +const { alice, bob } = ACCOUNTS; +const ATT = { filename: 'report.bin', contentType: 'application/octet-stream', content: 'hello-attachment-content-12345' }; + +test.describe('Cross-account attachments', () => { + test.beforeEach(async () => { + for (const a of [alice, bob]) { + const j = await JmapClient.connect(a.email, a.password); + await j.reset(); + } + }); + + test('an attachment on another account\'s All-Mail message downloads correctly', async ({ page }) => { + const subject = `IT attach ${Date.now()}`; + // Deliver a message with an attachment to bob. + await sendMail({ from: bob.email, authPass: bob.password, to: bob.email, subject, body: 'see attachment', attachment: ATT }); + + // Cross-account All Mail + always download attachments (don't preview). + await seedSettings(page, { + enableUnifiedMailbox: true, + enableCrossAllView: true, + unifiedCrossAccount: true, + includeGroupInUnified: true, + mailAttachmentAction: 'download', + }); + + // Make alice the active account, with bob added, so bob's message is + // genuinely cross-account when opened. + await login(page, alice); + await addAccount(page, bob); + await switchAccount(page, alice.email); + await forceSync(page); + + // Open the All-Mail view and bob's message. + await expect(folderRow(page, { name: '__cross_all__' }).first()).toBeVisible(); + await openFolder(page, { name: '__cross_all__' }); + await forceSync(page); + await expectEmailVisible(page, subject); + await emailItem(page, subject).first().click(); + + // The attachment chip is present; clicking it downloads the blob from bob's + // account (pre-fix this 404s against alice and no download fires). + const chip = page.locator(`[data-testid="attachment"][data-attachment-name="${ATT.filename}"]`).first(); + await chip.waitFor({ state: 'visible', timeout: 15000 }); + + const [download] = await Promise.all([ + page.waitForEvent('download', { timeout: 15000 }), + chip.click(), + ]); + + const stream = await download.createReadStream(); + const chunks: Buffer[] = []; + for await (const c of stream) chunks.push(c as Buffer); + expect(Buffer.concat(chunks).toString()).toContain(ATT.content); + }); +}); diff --git a/integration/tests/helpers/smtp.ts b/integration/tests/helpers/smtp.ts index d81c6bcd..cd994206 100644 --- a/integration/tests/helpers/smtp.ts +++ b/integration/tests/helpers/smtp.ts @@ -24,6 +24,8 @@ interface SendOptions { body: string; /** Extra headers (e.g. custom Message-ID / In-Reply-To for threading). */ headers?: Record; + /** Optional single attachment (sent as multipart/mixed, base64). */ + attachment?: { filename: string; contentType: string; content: string }; } class SmtpError extends Error {} @@ -110,14 +112,38 @@ export async function sendMail(opts: SendOptions): Promise { From: opts.from, To: recipients.join(', '), Subject: opts.subject, - 'Content-Type': 'text/plain; charset=utf-8', ...opts.headers, }; + + let mime: string; + if (opts.attachment) { + const boundary = 'itmixed_boundary_0001'; + headers['MIME-Version'] = '1.0'; + headers['Content-Type'] = `multipart/mixed; boundary="${boundary}"`; + const b64 = Buffer.from(opts.attachment.content).toString('base64').replace(/(.{76})/g, '$1\r\n'); + mime = [ + `--${boundary}`, + 'Content-Type: text/plain; charset=utf-8', + '', + crlf(opts.body), + `--${boundary}`, + `Content-Type: ${opts.attachment.contentType}; name="${opts.attachment.filename}"`, + `Content-Disposition: attachment; filename="${opts.attachment.filename}"`, + 'Content-Transfer-Encoding: base64', + '', + b64, + `--${boundary}--`, + ].join('\r\n'); + } else { + headers['Content-Type'] = 'text/plain; charset=utf-8'; + mime = crlf(opts.body); + } + const headerBlock = Object.entries(headers) .map(([k, v]) => `${k}: ${v}`) .join('\r\n'); // Dot-stuff any line that begins with '.' - const safeBody = crlf(opts.body).replace(/\r\n\./g, '\r\n..'); + const safeBody = mime.replace(/\r\n\./g, '\r\n..'); send(`${headerBlock}\r\n\r\n${safeBody}\r\n.`); await waitReply('250'); send('QUIT'); diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index 5a7d4223..d55db3ec 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -222,9 +222,9 @@ export interface IJMAPClient { ): Promise<{ blobId: string; size: number; type: string }>; getBlobDownloadUrl(blobId: string, name?: string, type?: string, accountId?: string): string; fetchBlob(blobId: string, name?: string, type?: string, accountId?: string): Promise; - fetchBlobAsObjectUrl(blobId: string, name?: string, type?: string): Promise; - fetchBlobArrayBuffer(blobId: string, name?: string, type?: string): Promise; - downloadBlob(blobId: string, name?: string, type?: string): Promise; + fetchBlobAsObjectUrl(blobId: string, name?: string, type?: string, accountId?: string): Promise; + fetchBlobArrayBuffer(blobId: string, name?: string, type?: string, accountId?: string): Promise; + downloadBlob(blobId: string, name?: string, type?: string, accountId?: string): Promise; // ── Identities ──────────────────────────────────────────────── getIdentities(): Promise; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 4df7fa86..fccc1503 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -3382,8 +3382,8 @@ export class JMAPClient implements IJMAPClient { return response.blob(); } - async fetchBlobAsObjectUrl(blobId: string, name?: string, type?: string): Promise { - const blob = await this.fetchBlob(blobId, name, type); + async fetchBlobAsObjectUrl(blobId: string, name?: string, type?: string, accountId?: string): Promise { + const blob = await this.fetchBlob(blobId, name, type, accountId); return URL.createObjectURL(blob); } @@ -5662,8 +5662,8 @@ export class JMAPClient implements IJMAPClient { return created as FileNode; } - async downloadBlob(blobId: string, name?: string, type?: string): Promise { - const blob = await this.fetchBlob(blobId, name, type); + async downloadBlob(blobId: string, name?: string, type?: string, accountId?: string): Promise { + const blob = await this.fetchBlob(blobId, name, type, accountId); const blobUrl = URL.createObjectURL(blob); const a = document.createElement('a'); @@ -6153,8 +6153,8 @@ export class JMAPClient implements IJMAPClient { // ── S/MIME raw-email helpers ───────────────────────────────────── /** Fetch blob content as an ArrayBuffer (for S/MIME byte processing). */ - async fetchBlobArrayBuffer(blobId: string, name?: string, type?: string): Promise { - const url = this.getBlobDownloadUrl(blobId, name, type); + async fetchBlobArrayBuffer(blobId: string, name?: string, type?: string, accountId?: string): Promise { + const url = this.getBlobDownloadUrl(blobId, name, type, accountId); const response = await this.authenticatedFetch(url, {}); if (!response.ok) { throw new Error(`Failed to fetch blob: ${response.status}`);