From a29cf27ff8ea28d43bd27235b415d7682d6d245c Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 25 Aug 2026 18:50:41 +0200 Subject: [PATCH] fix: route multi-account email lookups and invitation parsing by source account #847 --- .../calendar-invitation-banner.test.tsx | 84 +++++++++++++++++++ .../email/calendar-invitation-banner.tsx | 34 ++++++-- components/mail/mail-app.tsx | 57 +++++++++---- lib/__tests__/clear-cached-data.test.ts | 2 + lib/clear-cached-data.ts | 1 + stores/email-store.ts | 25 ++++-- 6 files changed, 173 insertions(+), 30 deletions(-) diff --git a/components/email/__tests__/calendar-invitation-banner.test.tsx b/components/email/__tests__/calendar-invitation-banner.test.tsx index 6e2c017a4..de6be7fcf 100644 --- a/components/email/__tests__/calendar-invitation-banner.test.tsx +++ b/components/email/__tests__/calendar-invitation-banner.test.tsx @@ -12,9 +12,20 @@ const mocks = vi.hoisted(() => { queryCalendarEvents: vi.fn(async () => []), }; + const sourceClientMock = { + parseCalendarEvents: vi.fn(), + fetchBlob: vi.fn(), + getCalendarsAccountId: vi.fn(() => 'source-calendar-account'), + }; + const authState = { client: clientMock, primaryIdentity: { email: 'user@example.com' }, + getClientForAccount: vi.fn((id: string) => (id === 'login-b' ? sourceClientMock : undefined)), + }; + + const emailState = { + viewingAccountId: null as string | null, }; const settingsState = { @@ -48,7 +59,9 @@ const mocks = vi.hoisted(() => { return { pushMock, clientMock, + sourceClientMock, authState, + emailState, settingsState, calendarState, useCalendarStoreMock, @@ -118,6 +131,10 @@ vi.mock('@/stores/auth-store', () => ({ useAuthStore: (selector: (state: typeof mocks.authState) => unknown) => selector(mocks.authState), })); +vi.mock('@/stores/email-store', () => ({ + useEmailStore: (selector: (state: typeof mocks.emailState) => unknown) => selector(mocks.emailState), +})); + vi.mock('@/stores/settings-store', () => ({ useSettingsStore: (selector: (state: typeof mocks.settingsState) => unknown) => selector(mocks.settingsState), })); @@ -162,6 +179,11 @@ describe('CalendarInvitationBanner', () => { mocks.calendarState.updateEvent.mockReset(); mocks.calendarState.setSelectedDate.mockReset(); mocks.calendarState.events = []; + mocks.sourceClientMock.parseCalendarEvents.mockReset(); + mocks.sourceClientMock.fetchBlob.mockReset(); + mocks.sourceClientMock.fetchBlob.mockRejectedValue(new Error('no blob')); + mocks.authState.getClientForAccount.mockClear(); + mocks.emailState.viewingAccountId = null; }); it('does not parse invitations when invitation parsing is disabled', () => { @@ -173,6 +195,68 @@ describe('CalendarInvitationBanner', () => { expect(mocks.clientMock.parseCalendarEvents).not.toHaveBeenCalled(); }); + // #867: blobs are per-account, so a message from another signed-in account + // must be parsed through that account's client and against its owning + // JMAP account - not the globally active login. + it('parses invitations of a stamped (unified view) email through its source account', async () => { + mocks.sourceClientMock.parseCalendarEvents.mockResolvedValue([ + { uid: 'uid-b', title: 'Invite in account B', start: '2026-03-22T14:00:00Z', participants: {} }, + ]); + + render( + , + ); + + await waitFor(() => { + expect(screen.getByText('Invite in account B')).toBeInTheDocument(); + }); + expect(mocks.sourceClientMock.parseCalendarEvents).toHaveBeenCalledWith('jmap-b', 'blob-1'); + expect(mocks.sourceClientMock.fetchBlob).toHaveBeenCalledWith('blob-1', 'invite.ics', 'text/calendar', 'jmap-b'); + expect(mocks.clientMock.parseCalendarEvents).not.toHaveBeenCalled(); + }); + + it('parses invitations through the viewing account in the per-account sidebar view', async () => { + mocks.emailState.viewingAccountId = 'login-b'; + mocks.sourceClientMock.parseCalendarEvents.mockResolvedValue([ + { uid: 'uid-b', title: 'Invite in account B', start: '2026-03-22T14:00:00Z', participants: {} }, + ]); + + render(); + + await waitFor(() => { + expect(screen.getByText('Invite in account B')).toBeInTheDocument(); + }); + // Unstamped email: the owning account is the viewing client's own. + expect(mocks.sourceClientMock.parseCalendarEvents).toHaveBeenCalledWith('source-calendar-account', 'blob-1'); + expect(mocks.clientMock.parseCalendarEvents).not.toHaveBeenCalled(); + }); + + it('keeps using the active client for unstamped emails', async () => { + mocks.clientMock.parseCalendarEvents.mockResolvedValue([ + { uid: 'uid-a', title: 'Invite in account A', start: '2026-03-22T14:00:00Z', participants: {} }, + ]); + + render(); + + await waitFor(() => { + expect(screen.getByText('Invite in account A')).toBeInTheDocument(); + }); + expect(mocks.clientMock.parseCalendarEvents).toHaveBeenCalledWith('calendar-account', 'blob-1'); + expect(mocks.sourceClientMock.parseCalendarEvents).not.toHaveBeenCalled(); + }); + + it('surfaces the underlying error when parsing fails', async () => { + mocks.clientMock.parseCalendarEvents.mockRejectedValue(new Error('Failed to parse calendar file')); + + render(); + + await waitFor(() => { + expect(screen.getByText('Failed to parse calendar file')).toBeInTheDocument(); + }); + }); + it('does not show trust warnings on sent invitations from the current user', async () => { mocks.clientMock.parseCalendarEvents.mockResolvedValue([ { diff --git a/components/email/calendar-invitation-banner.tsx b/components/email/calendar-invitation-banner.tsx index 9b8b0fe44..fa280d983 100644 --- a/components/email/calendar-invitation-banner.tsx +++ b/components/email/calendar-invitation-banner.tsx @@ -22,6 +22,7 @@ import { useTranslations, useFormatter } from 'next-intl'; import { useRouter } from '@/i18n/navigation'; import { isDocumentRTL } from '@/i18n/direction'; import { useAuthStore } from '@/stores/auth-store'; +import { useEmailStore } from '@/stores/email-store'; import { useCalendarStore } from '@/stores/calendar-store'; import { useSettingsStore } from '@/stores/settings-store'; import type { Email, CalendarEvent } from '@/lib/jmap/types'; @@ -363,6 +364,17 @@ export function CalendarInvitationBanner({ email }: CalendarInvitationBannerProp const format = useFormatter(); const router = useRouter(); const client = useAuthStore((s) => s.client); + const getClientForAccount = useAuthStore((s) => s.getClientForAccount); + const viewingAccountId = useEmailStore((s) => s.viewingAccountId); + // Blobs are per-account, so the invitation must be parsed and fetched through + // the login the message is reachable from - its source stamp in aggregate + // views, the per-account sidebar's viewing account otherwise - and against its + // owning JMAP account. Against any other account Stalwart answers + // CalendarEvent/parse with an error. Calendar actions (import / RSVP) keep + // using the active client, whose calendars the calendar store holds. (#867) + const sourceClientId = email.sourceClientAccountId ?? viewingAccountId ?? null; + const parseClient = (sourceClientId ? getClientForAccount?.(sourceClientId) : undefined) ?? client; + const blobAccountId = email.sourceAccountId ?? parseClient?.getCalendarsAccountId(); const currentUserEmail = useAuthStore((s) => s.primaryIdentity?.email); const calendarInvitationParsingEnabled = useSettingsStore((s) => s.calendarInvitationParsingEnabled); const timeFormat = useSettingsStore((s) => s.timeFormat); @@ -380,23 +392,27 @@ export function CalendarInvitationBanner({ email }: CalendarInvitationBannerProp const [selectedCalendarId, setSelectedCalendarId] = useState(''); const [rawIcsMethod, setRawIcsMethod] = useState('unknown'); const [isCollapsed, setIsCollapsed] = useState(false); + // Underlying failure shown under the generic parse error, so "wrong account" + // or a network failure is distinguishable from an unparsable file. + const [parseError, setParseError] = useState(null); const attachment = findCalendarAttachment(email); const parseEvent = useCallback(async () => { - if (!client || !attachment || !calendarInvitationParsingEnabled) return; + if (!client || !parseClient || !blobAccountId || !attachment || !calendarInvitationParsingEnabled) return; setState('loading'); setActionNotice(null); setActionError(null); + setParseError(null); try { // JMAP strips parameters from Content-Type (RFC 8621), so method=REQUEST // is lost. Fetch raw ICS to extract METHOD as a reliable fallback - in // parallel with parsing to save a roundtrip. const [events, rawText] = await Promise.all([ - client.parseCalendarEvents(client.getCalendarsAccountId(), attachment.blobId), + parseClient.parseCalendarEvents(blobAccountId, attachment.blobId), (async () => { try { - const blob = await client.fetchBlob(attachment.blobId, 'invite.ics', 'text/calendar'); + const blob = await parseClient.fetchBlob(attachment.blobId, 'invite.ics', 'text/calendar', blobAccountId); return await blob.text(); } catch { return null; @@ -440,10 +456,11 @@ export function CalendarInvitationBanner({ email }: CalendarInvitationBannerProp .catch(() => { /* ignore lookup failure */ }); } } - } catch { + } catch (err) { + setParseError(err instanceof Error ? err.message : String(err)); setState('error'); } - }, [calendarInvitationParsingEnabled, client, attachment, supportsCalendar]); + }, [calendarInvitationParsingEnabled, client, parseClient, blobAccountId, attachment, supportsCalendar]); useEffect(() => { if (attachment && calendarInvitationParsingEnabled) { @@ -743,7 +760,12 @@ export function CalendarInvitationBanner({ email }: CalendarInvitationBannerProp
- {t('parse_error')} +
+ {t('parse_error')} + {parseError && ( + {parseError} + )} +
); } diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index b2aa388ca..c28c99888 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -3067,7 +3067,9 @@ export function MailApp({ linkSegments }: MailAppProps = {}) { const shouldHideHorizontalViewerPane = isHorizontalMailLayout && !hasViewerContent; // Handle email selection with mobile view switching - const handleEmailSelect = async (email: { id: string }) => { + const handleEmailSelect = async ( + email: { id: string } & Partial>, + ) => { if (!client || !email) return; // If composing, suspend the composer (unmount will trigger onSaveState) @@ -3078,7 +3080,19 @@ export function MailApp({ linkSegments }: MailAppProps = {}) { // Find the list-level email for metadata (accountId, scheduled flags, etc.) // but don't select it yet — wait for the full fetch to avoid a toolbar flash // caused by rendering with the stub (no bodyValues) then re-rendering with the full email. - const listEmail = activeEmails.find(e => e.id === email.id); + // + // JMAP email ids are only unique per account (RFC 8621 §1.3) and Stalwart + // hands out overlapping id ranges, so in aggregate views one bare id can name + // unrelated mails in different accounts. Prefer the clicked object itself (it + // is the list row) and only fall back to an id lookup that also matches the + // source stamps — never the first bare-id hit. (#847) + const listEmail: Partial | undefined = + activeEmails.find(e => e === email) + ?? activeEmails.find(e => + e.id === email.id + && (email.sourceClientAccountId === undefined || e.sourceClientAccountId === email.sourceClientAccountId) + && (email.sourceAccountId === undefined || e.sourceAccountId === email.sourceAccountId)) + ?? (email.sourceClientAccountId ? email : undefined); setLoadingEmail(true); @@ -3094,23 +3108,34 @@ export function MailApp({ linkSegments }: MailAppProps = {}) { // Fetch the full content try { - // In unified view each email carries its source reference: the login it is - // reachable through (`sourceClientAccountId`) and its owning JMAP account - // (`sourceAccountId`). Resolve both so we fetch from the server that actually - // owns it — works uniformly for personal and shared/group sources, since for + // Emails fetched through an aggregate view carry their source reference: + // the login they are reachable through (`sourceClientAccountId`) and their + // owning JMAP account (`sourceAccountId`). Honor the stamps whenever they + // are present — not only while `isUnifiedView` is set, since a stamped list + // can outlive that flag — so the fetch goes to the server that actually owns + // the mail. Works uniformly for personal and shared/group sources: for // personal the owning account equals the client's primary (no-op). (#281) - const sourceClientId = isUnifiedView ? listEmail?.sourceClientAccountId : undefined; - const perAccountClient = sourceClientId + // + // Unstamped emails belong to the account being browsed: the per-account + // sidebar's `viewingAccountId` when set, otherwise the active login. A + // shared folder on that client still needs its owner accountId. (#847) + const sourceClientId = listEmail?.sourceClientAccountId; + const sourceClient = sourceClientId ? useAuthStore.getState().getClientForAccount(sourceClientId) : undefined; - const fetchClient = perAccountClient ?? client; + if (sourceClientId && !sourceClient) { + // No silent fallback to the active client: with colliding ids that would + // render another account's mail under this row. + console.warn('[mail-app] No connected client for source account', sourceClientId); + return; + } + const fetchClient = sourceClient + ?? (viewingAccountId ? useAuthStore.getState().getClientForAccount(viewingAccountId) : undefined) + ?? client; - const accountId = isUnifiedView - ? listEmail?.sourceAccountId - : (() => { - // Non-unified: shared folders on the active client still need their - // owner accountId passed explicitly. - const mailbox = mailboxes.find(mb => mb.id === selectedMailbox); + const accountId = listEmail?.sourceAccountId + ?? (() => { + const mailbox = viewMailboxes.find(mb => mb.id === selectedMailbox); return mailbox?.isShared ? mailbox.accountId : undefined; })(); @@ -3126,7 +3151,7 @@ export function MailApp({ linkSegments }: MailAppProps = {}) { } // Re-stamp the source reference so later actions on the open email // resolve to the right account (the fetched object lacks these). - if (isUnifiedView && listEmail) { + if (listEmail?.sourceClientAccountId) { fullEmail.accountId = listEmail.accountId; fullEmail.accountLabel = listEmail.accountLabel; fullEmail.sourceClientAccountId = listEmail.sourceClientAccountId; diff --git a/lib/__tests__/clear-cached-data.test.ts b/lib/__tests__/clear-cached-data.test.ts index b29db7748..9c8ac8e02 100644 --- a/lib/__tests__/clear-cached-data.test.ts +++ b/lib/__tests__/clear-cached-data.test.ts @@ -24,6 +24,7 @@ describe('clearCachedData', () => { localStorage.setItem('calendar-storage', '1'); localStorage.setItem('identity-storage', '1'); localStorage.setItem('calendar-notification-storage', '1'); + localStorage.setItem('email-snapshot', '{"v":2}'); // Must survive — losing these is exactly the pain we're avoiding. localStorage.setItem('account-registry', 'accounts'); localStorage.setItem('auth-storage', 'session'); @@ -36,6 +37,7 @@ describe('clearCachedData', () => { expect(localStorage.getItem('calendar-storage')).toBeNull(); expect(localStorage.getItem('identity-storage')).toBeNull(); expect(localStorage.getItem('calendar-notification-storage')).toBeNull(); + expect(localStorage.getItem('email-snapshot')).toBeNull(); expect(localStorage.getItem('account-registry')).toBe('accounts'); expect(localStorage.getItem('auth-storage')).toBe('session'); diff --git a/lib/clear-cached-data.ts b/lib/clear-cached-data.ts index c9f3fffad..412a573c4 100644 --- a/lib/clear-cached-data.ts +++ b/lib/clear-cached-data.ts @@ -9,6 +9,7 @@ import { evictAll } from '@/lib/account-state-manager'; // - 'settings-storage' / 'theme-storage' / 'locale-storage' → user prefs // - 'template-storage' / 'smime-preferences' → user-created content const CACHE_STORAGE_KEYS = [ + 'email-snapshot', 'identity-storage', 'contact-storage', 'calendar-storage', diff --git a/stores/email-store.ts b/stores/email-store.ts index 2392181dd..09018d398 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -526,7 +526,11 @@ function resolveEmailActionContext( // both - no id-space guessing, no capability scan. For personal sources // `sourceAccountId` equals the client's primary, so passing it to JMAP is a // no-op (matches the previous `accountId: undefined` behavior exactly). - if (state.isUnifiedView && email.sourceClientAccountId && email.sourceAccountId) { + // The stamps are honored whenever present, not only while `isUnifiedView` + // is set: a stamped email (e.g. the open selection) can outlive the flag, and + // bare ids collide across accounts, so the active client is never a safe + // substitute for the stamped one. (#847) + if (email.sourceClientAccountId && email.sourceAccountId) { return { client: useAuthStore.getState().getClientForAccount(email.sourceClientAccountId) ?? resolveActionClient(passedClient), mailboxes: state.accountMailboxes[email.sourceAccountId] ?? state.mailboxes, @@ -2883,7 +2887,7 @@ export const useEmailStore = create((set, get) => ({ emailId, originalMailboxId: currentMailbox.originalId || currentMailbox.id, accountId, - sourceClientAccountId: get().isUnifiedView ? email.sourceClientAccountId : undefined, + sourceClientAccountId: email.sourceClientAccountId, }); try { @@ -2963,7 +2967,7 @@ export const useEmailStore = create((set, get) => ({ // account's. (#281) const listEmail = get().emails.find(e => e.id === emailId); let inboxMailboxes = mailboxes; - if (get().isUnifiedView && listEmail?.sourceClientAccountId && listEmail?.sourceAccountId) { + if (listEmail?.sourceClientAccountId && listEmail?.sourceAccountId) { undoClient = useAuthStore.getState().getClientForAccount(listEmail.sourceClientAccountId) ?? undoClient; accountId = listEmail.sourceAccountId; inboxMailboxes = get().accountMailboxes[listEmail.sourceAccountId] ?? mailboxes; @@ -4511,7 +4515,10 @@ useEmailStore.subscribe((state, prev) => { // { background: true } when the list is already populated). // --------------------------------------------------------------------------- const EMAIL_SNAPSHOT_KEY = 'email-snapshot'; -const EMAIL_SNAPSHOT_VERSION = 1; +// v2: unified / cross-account lists are no longer snapshotted (their rows carry +// per-account source stamps that a fresh boot ignores, so a colliding JMAP id +// opened the wrong account's mail - #847). Older snapshots are discarded. +const EMAIL_SNAPSHOT_VERSION = 2; const EMAIL_SNAPSHOT_MAX_EMAILS = 50; if (typeof window !== 'undefined') { @@ -4563,10 +4570,12 @@ if (typeof window !== 'undefined') { snapshotTimer = null; const s = useEmailStore.getState(); if (s.mailboxes.length === 0) return; - // Don't snapshot search results, keyword filters, the scheduled view or - // a secondary account's view - the previous plain listing stays in - // place for the next boot. - if (s.searchQuery || s.selectedKeyword || s.isScheduledView || s.viewingAccountId) return; + // Don't snapshot search results, keyword filters, the scheduled view, a + // secondary account's view or a unified/cross-account list - the previous + // plain listing stays in place for the next boot. A unified list mixes + // accounts whose email ids collide; restored without its view flags it + // would be read as the active account's mail. (#847) + if (s.searchQuery || s.selectedKeyword || s.isScheduledView || s.viewingAccountId || s.isUnifiedView) return; try { window.localStorage.setItem(EMAIL_SNAPSHOT_KEY, JSON.stringify({ v: EMAIL_SNAPSHOT_VERSION,