mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-02 08:15:55 +00:00
fix: stop acting on a message of a signed-out account through the active account
This commit is contained in:
@@ -8,6 +8,7 @@ import {
|
||||
resolveTabFilter,
|
||||
} from '@/stores/message-list-tabs-store';
|
||||
import { useEmailStore } from '@/stores/email-store';
|
||||
import { useAuthStore } from '@/stores/auth-store';
|
||||
import { messageListTabHooks } from '@/lib/plugin-hooks';
|
||||
import { jmapError } from '@/lib/jmap/jmap-error';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
@@ -191,6 +192,11 @@ describe('categorizeEmails', () => {
|
||||
from: [{ email: `${id}@example.com` }],
|
||||
...(sourceAccountId ? { accountId: 'login-1', sourceClientAccountId: 'login-1', sourceAccountId } : {}),
|
||||
}) as unknown as Email;
|
||||
// The stamped login is connected, through this client.
|
||||
const loginClient = (client: Partial<IJMAPClient>) => {
|
||||
useAuthStore.setState({ getClientForAccount: (id: string) => (id === 'login-1' ? client : undefined) } as never);
|
||||
return client as IJMAPClient;
|
||||
};
|
||||
const PATCH = { 'keywords/$category-updates': null, 'keywords/$category-promotions': true };
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -231,7 +237,7 @@ describe('categorizeEmails', () => {
|
||||
const emitted = vi.spyOn(messageListTabHooks.onEmailCategorize, 'emit');
|
||||
|
||||
await expect(useMessageListTabsStore.getState()
|
||||
.categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m1', 'm2'], 'promotions')).rejects.toBe(refusal);
|
||||
.categorizeEmails(loginClient({ batchUpdateKeywords }), ['m1', 'm2'], 'promotions')).rejects.toBe(refusal);
|
||||
|
||||
// Both accounts were written; only the one the server accepted moved.
|
||||
expect(batchUpdateKeywords).toHaveBeenCalledTimes(2);
|
||||
@@ -265,7 +271,7 @@ describe('categorizeEmails', () => {
|
||||
const batchUpdateKeywords = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
await useMessageListTabsStore.getState()
|
||||
.categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m2'], 'promotions');
|
||||
.categorizeEmails(loginClient({ batchUpdateKeywords }), ['m2'], 'promotions');
|
||||
|
||||
expect(batchUpdateKeywords).toHaveBeenCalledWith(['m2'], PATCH, 'shared');
|
||||
expect(batchUpdateKeywords).not.toHaveBeenCalledWith(expect.anything(), expect.anything(), 'login-1');
|
||||
|
||||
@@ -25,7 +25,10 @@ function email(mailboxIds: Record<string, boolean>, stamps: Partial<Email> = {})
|
||||
|
||||
describe('resolveQuickRuleTarget', () => {
|
||||
beforeEach(() => {
|
||||
useAuthStore.setState({ client, activeAccountId: 'login-a' } as never);
|
||||
useAuthStore.setState({
|
||||
client, activeAccountId: 'login-a',
|
||||
getClientForAccount: (id: string) => (id === 'login-a' ? client : undefined),
|
||||
} as never);
|
||||
useEmailStore.setState({ mailboxes, selectedMailbox: 'a', viewingAccountId: null, accountMailboxes: {} } as never);
|
||||
});
|
||||
|
||||
@@ -44,6 +47,10 @@ describe('resolveQuickRuleTarget', () => {
|
||||
expect(resolveQuickRuleTarget(email({ 'c:a': true }, { sourceAccountId: 'c', sourceClientAccountId: 'login-a' }))?.shared).toBe(true);
|
||||
});
|
||||
|
||||
it('offers no target for a message of a login that is gone', () => {
|
||||
expect(resolveQuickRuleTarget(email({ a: true }, { sourceAccountId: 'd', sourceClientAccountId: 'login-b' }))).toBeNull();
|
||||
});
|
||||
|
||||
it('finds the folder the message is in', () => {
|
||||
const target = resolveQuickRuleTarget(email({ h: true }))!;
|
||||
expect(sourceMailboxOf(email({ h: true }), target)?.id).toBe('h');
|
||||
|
||||
@@ -46,7 +46,12 @@ function clientKey(client: IJMAPClient): number {
|
||||
export function resolveQuickRuleTarget(email: Email): QuickRuleTarget | null {
|
||||
const auth = useAuthStore.getState();
|
||||
if (!auth.client) return null;
|
||||
const context = resolveEmailActionContext(email, auth.client);
|
||||
let context: ReturnType<typeof resolveEmailActionContext>;
|
||||
try {
|
||||
context = resolveEmailActionContext(email, auth.client);
|
||||
} catch {
|
||||
return null; // a message of a login that is gone
|
||||
}
|
||||
const client = context.client;
|
||||
const ownAccountId = client.getAccountId();
|
||||
const accountId = context.accountId ?? ownAccountId;
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useEmailStore, resolveEmailActionContext } from '../email-store';
|
||||
import { useAuthStore } from '../auth-store';
|
||||
import type { Email, Mailbox } from '@/lib/jmap/types';
|
||||
import type { IJMAPClient } from '@/lib/jmap/client-interface';
|
||||
|
||||
// A row stamped with a login that is no longer connected must not be written
|
||||
// through another login: email and account ids repeat across logins and
|
||||
// servers, so the active login's client would act on its own message with
|
||||
// the same id (GHSA-8x23 class).
|
||||
|
||||
function mailbox(id: string, role: string, accountId: string): Mailbox {
|
||||
return { id, originalId: id, name: id, role, accountId, isShared: false, sortOrder: 0, totalEmails: 0, unreadEmails: 0, totalThreads: 0, unreadThreads: 0, isSubscribed: true } as Mailbox;
|
||||
}
|
||||
|
||||
function makeClient(accountId: string) {
|
||||
return {
|
||||
getAccountId: () => accountId,
|
||||
markAsRead: vi.fn().mockResolvedValue(undefined),
|
||||
toggleStar: vi.fn().mockResolvedValue(undefined),
|
||||
moveEmail: vi.fn().mockResolvedValue(undefined),
|
||||
moveToTrash: vi.fn().mockResolvedValue(undefined),
|
||||
setKeyword: vi.fn().mockResolvedValue(undefined),
|
||||
batchUpdateKeywords: vi.fn().mockResolvedValue(undefined),
|
||||
batchMarkAsRead: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as IJMAPClient;
|
||||
}
|
||||
|
||||
// A message of login B (gone), with the same id and account id as one of the
|
||||
// active login A's own messages.
|
||||
const goneRow = {
|
||||
id: 'eaaaaab', threadId: 't', subject: 'B', receivedAt: '2026-10-26T10:00:00Z',
|
||||
keywords: {}, mailboxIds: { a: true },
|
||||
sourceClientAccountId: 'login-b', sourceAccountId: 'cf',
|
||||
} as unknown as Email;
|
||||
|
||||
describe('a row of a login that is no longer connected', () => {
|
||||
let active: IJMAPClient;
|
||||
|
||||
beforeEach(() => {
|
||||
active = makeClient('cf');
|
||||
useAuthStore.setState({
|
||||
activeAccountId: 'login-a',
|
||||
client: active as never,
|
||||
getClientForAccount: (id: string) => (id === 'login-a' ? active : undefined) as never,
|
||||
} as never);
|
||||
useEmailStore.setState({
|
||||
isUnifiedView: true, viewingAccountId: null, selectedMailbox: '',
|
||||
mailboxes: [mailbox('a', 'inbox', 'cf'), mailbox('b', 'trash', 'cf')],
|
||||
accountMailboxes: {},
|
||||
emails: [goneRow], selectedEmail: null, selectedEmailKeys: new Set(),
|
||||
processingReadStatus: new Set(),
|
||||
threadEmailsCache: new Map(), expandedThreadIds: new Set(),
|
||||
} as never);
|
||||
});
|
||||
|
||||
it('resolves to no client instead of the active one', () => {
|
||||
expect(() => resolveEmailActionContext(goneRow, active)).toThrow('Not connected');
|
||||
});
|
||||
|
||||
it('is not marked read through the active login', async () => {
|
||||
await expect(useEmailStore.getState().markAsRead(active, 'eaaaaab', true)).rejects.toThrow('Not connected');
|
||||
expect(active.markAsRead).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('is not starred through the active login', async () => {
|
||||
await expect(useEmailStore.getState().toggleStar(active, 'eaaaaab')).rejects.toThrow('Not connected');
|
||||
expect(active.toggleStar).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('is not moved through the active login', async () => {
|
||||
await expect(useEmailStore.getState().moveToMailbox(active, 'eaaaaab', 'b')).rejects.toThrow('Not connected');
|
||||
expect(active.moveEmail).not.toHaveBeenCalled();
|
||||
expect(useEmailStore.getState().emails).toEqual([goneRow]);
|
||||
});
|
||||
|
||||
it('gets no keyword through the active login', async () => {
|
||||
await expect(useEmailStore.getState().markEmailKeyword(active, 'eaaaaab', '$answered')).rejects.toThrow('Not connected');
|
||||
expect(active.setKeyword).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('is not moved back to the Inbox through the active login', async () => {
|
||||
(active as unknown as { undoSpam: unknown }).undoSpam = vi.fn().mockResolvedValue(undefined);
|
||||
await expect(useEmailStore.getState().undoSpam(active, 'eaaaaab')).rejects.toThrow('Not connected');
|
||||
expect((active as unknown as { undoSpam: ReturnType<typeof vi.fn> }).undoSpam).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('is not marked read with its expanded thread through the active login', async () => {
|
||||
useEmailStore.setState({ emails: [{ ...goneRow, threadId: 't' }] } as never);
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
await useEmailStore.getState().markThreadAsRead(active, 'login-b/cf:t');
|
||||
expect(active.batchMarkAsRead).not.toHaveBeenCalled();
|
||||
expect(useEmailStore.getState().emails[0].keywords.$seen).toBeUndefined();
|
||||
});
|
||||
});
|
||||
+25
-9
@@ -1010,6 +1010,16 @@ function resolveActionClient(passedClient: IJMAPClient): IJMAPClient {
|
||||
return c ?? passedClient;
|
||||
}
|
||||
|
||||
/**
|
||||
* The client of the login a row is stamped with. Never another login's once
|
||||
* that one is gone: its ids name other messages there.
|
||||
*/
|
||||
function stampedClient(clientAccountId: string): IJMAPClient {
|
||||
const client = useAuthStore.getState().getClientForAccount(clientAccountId);
|
||||
if (!client) throw new Error('Not connected');
|
||||
return client;
|
||||
}
|
||||
|
||||
/**
|
||||
* Thrown by archive actions when no archive folder exists in the target
|
||||
* account. Callers map it to the translated
|
||||
@@ -1434,10 +1444,10 @@ export function resolveEmailActionContext(
|
||||
// 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)
|
||||
// substitute for the stamped one, also not once that login is gone. (#847)
|
||||
if (email.sourceClientAccountId && email.sourceAccountId) {
|
||||
return {
|
||||
client: useAuthStore.getState().getClientForAccount(email.sourceClientAccountId) ?? resolveActionClient(passedClient),
|
||||
client: stampedClient(email.sourceClientAccountId),
|
||||
mailboxes: sourceMailboxes(state, email.sourceClientAccountId, email.sourceAccountId) ?? state.mailboxes,
|
||||
accountId: email.sourceAccountId,
|
||||
};
|
||||
@@ -5883,7 +5893,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
targetMailboxId = cachedData.originalMailboxId;
|
||||
accountId = cachedData.accountId;
|
||||
if (cachedData.sourceClientAccountId) {
|
||||
undoClient = useAuthStore.getState().getClientForAccount(cachedData.sourceClientAccountId) ?? undoClient;
|
||||
undoClient = stampedClient(cachedData.sourceClientAccountId);
|
||||
}
|
||||
get().spamUndoCache.delete(emailKey ?? emailId);
|
||||
} else {
|
||||
@@ -5894,7 +5904,7 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
const listEmail = get().emails.find(isTarget);
|
||||
let inboxMailboxes = mailboxes;
|
||||
if (listEmail?.sourceClientAccountId && listEmail?.sourceAccountId) {
|
||||
undoClient = useAuthStore.getState().getClientForAccount(listEmail.sourceClientAccountId) ?? undoClient;
|
||||
undoClient = stampedClient(listEmail.sourceClientAccountId);
|
||||
accountId = listEmail.sourceAccountId;
|
||||
inboxMailboxes = sourceMailboxes(get(), listEmail.sourceClientAccountId, listEmail.sourceAccountId) ?? mailboxes;
|
||||
} else {
|
||||
@@ -6666,11 +6676,17 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
|
||||
// or marks an unrelated message that happens to share the id. So a thread
|
||||
// in a shared or group folder has to name its account.
|
||||
const groups = new Map<IJMAPClient, Map<string | undefined, string[]>>();
|
||||
for (const id of unreadIds) {
|
||||
const { client: actionClient, accountId } = resolveEmailActionContext(allEmailMap.get(id)!, client);
|
||||
const byAccount = groups.get(actionClient) ?? new Map<string | undefined, string[]>();
|
||||
byAccount.set(accountId, [...(byAccount.get(accountId) ?? []), id]);
|
||||
groups.set(actionClient, byAccount);
|
||||
try {
|
||||
for (const id of unreadIds) {
|
||||
const { client: actionClient, accountId } = resolveEmailActionContext(allEmailMap.get(id)!, client);
|
||||
const byAccount = groups.get(actionClient) ?? new Map<string | undefined, string[]>();
|
||||
byAccount.set(accountId, [...(byAccount.get(accountId) ?? []), id]);
|
||||
groups.set(actionClient, byAccount);
|
||||
}
|
||||
} catch (error) {
|
||||
// A message of a login that is gone: the thread stays unread.
|
||||
console.error('Failed to mark thread as read:', error);
|
||||
return;
|
||||
}
|
||||
|
||||
// Mark all as read on the server. A message the server refused (a
|
||||
|
||||
Reference in New Issue
Block a user