mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 08:13:50 +00:00
fix: route multi-account email lookups and invitation parsing by source account #847
This commit is contained in:
@@ -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(
|
||||
<CalendarInvitationBanner
|
||||
email={makeEmail({ sourceClientAccountId: 'login-b', sourceAccountId: 'jmap-b' })}
|
||||
/>,
|
||||
);
|
||||
|
||||
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(<CalendarInvitationBanner email={makeEmail()} />);
|
||||
|
||||
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(<CalendarInvitationBanner email={makeEmail()} />);
|
||||
|
||||
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(<CalendarInvitationBanner email={makeEmail()} />);
|
||||
|
||||
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([
|
||||
{
|
||||
|
||||
@@ -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<string>('');
|
||||
const [rawIcsMethod, setRawIcsMethod] = useState<InvitationMethod>('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<string | null>(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
|
||||
<div className="w-10 h-10 rounded-full bg-destructive/15 text-destructive flex items-center justify-center flex-shrink-0 shadow-sm">
|
||||
<AlertCircle className="w-5 h-5" />
|
||||
</div>
|
||||
<span className="text-sm text-destructive">{t('parse_error')}</span>
|
||||
<div className="flex flex-col min-w-0">
|
||||
<span className="text-sm text-destructive">{t('parse_error')}</span>
|
||||
{parseError && (
|
||||
<span className="text-xs text-muted-foreground truncate" title={parseError}>{parseError}</span>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<Pick<Email, 'sourceClientAccountId' | 'sourceAccountId'>>,
|
||||
) => {
|
||||
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<Email> | 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;
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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',
|
||||
|
||||
+17
-8
@@ -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<EmailStore>((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<EmailStore>((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,
|
||||
|
||||
Reference in New Issue
Block a user