From 7b7236f05de5acc4ae0aeb9a32798258e1de2de9 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:33:16 +0100 Subject: [PATCH] fix: keep a rule made from a message out of a login switched to while the sheet was open --- locales/rn/de.json | 5 +++- locales/rn/en.json | 5 +++- src/api/__tests__/sieve-rules.test.ts | 17 +++++++++++++ src/api/sieve-rules.ts | 5 ++++ src/components/filters/QuickRuleSheet.tsx | 30 +++++++++++++++++++---- 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/locales/rn/de.json b/locales/rn/de.json index 2391303f..409969f4 100644 --- a/locales/rn/de.json +++ b/locales/rn/de.json @@ -266,7 +266,10 @@ }, "context_menu": { "color_tag": "Label", - "remove_color": "Label entfernen" + "remove_color": "Label entfernen", + "rules": { + "login_changed": "Sie haben das Konto gewechselt, daher wurde die Regel nicht gespeichert" + } }, "device_sync": { "auth_problem_text": "Der Server akzeptiert die Anmeldung von {account} nicht mehr, daher werden die Kontakte und Kalender dieses Kontos auf diesem Gerät nicht mehr synchronisiert. Melden Sie sich erneut an, um fortzufahren. Auf diesem Gerät vorgenommene Änderungen bleiben erhalten.", diff --git a/locales/rn/en.json b/locales/rn/en.json index a3e7c40c..7c59befb 100644 --- a/locales/rn/en.json +++ b/locales/rn/en.json @@ -275,7 +275,10 @@ }, "context_menu": { "color_tag": "Tag", - "remove_color": "Remove tag" + "remove_color": "Remove tag", + "rules": { + "login_changed": "You switched accounts, so the rule was not saved" + } }, "deep_link": { "open_failed": "Couldn't open this" diff --git a/src/api/__tests__/sieve-rules.test.ts b/src/api/__tests__/sieve-rules.test.ts index c9f3f3de..a4f665c7 100644 --- a/src/api/__tests__/sieve-rules.test.ts +++ b/src/api/__tests__/sieve-rules.test.ts @@ -90,4 +90,21 @@ describe('updateAccountFilters', () => { await expect(updateAccountFilters(() => [rule('new')], 'own')).rejects.toBeInstanceOf(FiltersLoginChangedError); expect(api.createSieveScript).not.toHaveBeenCalled(); }); + + // A rule made from a message and the folders of the login the sheet + // opened in, saved after a switch. + it('writes nothing into another login than the one the rule was built in', async () => { + api.getSieveScripts.mockResolvedValue([]); + const modify = vi.fn(() => [rule('new')]); + client.username = 'other@example.net'; + await expect(updateAccountFilters(modify, 'own', 'me@example.org@mail.example.org')) + .rejects.toBeInstanceOf(FiltersLoginChangedError); + expect(api.getSieveScripts).not.toHaveBeenCalled(); + expect(modify).not.toHaveBeenCalled(); + expect(api.createSieveScript).not.toHaveBeenCalled(); + + client.username = 'me@example.org'; + expect(await updateAccountFilters(modify, 'own', 'me@example.org@mail.example.org')).toBe(true); + expect(api.createSieveScript).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/api/sieve-rules.ts b/src/api/sieve-rules.ts index 454799b9..672faf77 100644 --- a/src/api/sieve-rules.ts +++ b/src/api/sieve-rules.ts @@ -39,14 +39,19 @@ export class FiltersLoginChangedError extends Error { * include, its forwarding and audience and the external rules. `modify` * returns null when there is nothing to write. A hand-edited script is * refused: it is never rewritten from a rule. Resolves to whether it wrote. + * + * `expectedLogin` is the login the rule was built in (from its message and + * folders); nothing is written unless the client still serves it. */ export async function updateAccountFilters( modify: (rules: FilterRule[]) => FilterRule[] | null, accountId: string = getSieveAccountId(), + expectedLogin?: string | null, ): Promise { // The client serves one login at a time; a switch while the script is read // would write these rules into the other login's account. const login = servedLoginOf(jmapClient); + if (expectedLogin !== undefined && login !== expectedLogin) throw new FiltersLoginChangedError(); const capabilities = getSieveCapabilities(accountId); const all = await getSieveScripts(accountId); const scripts = all.filter((s) => s.name !== VACATION_SCRIPT_NAME); diff --git a/src/components/filters/QuickRuleSheet.tsx b/src/components/filters/QuickRuleSheet.tsx index 1bb0f924..316d96b4 100644 --- a/src/components/filters/QuickRuleSheet.tsx +++ b/src/components/filters/QuickRuleSheet.tsx @@ -5,7 +5,9 @@ import { MoveSheet } from '../MoveSheet'; import { FilterRuleModal } from './FilterRuleModal'; import type { Email, Mailbox } from '../../api/types'; import { getSieveAccountId, getSieveCapabilities } from '../../api/sieve'; -import { OpaqueFiltersError, updateAccountFilters } from '../../api/sieve-rules'; +import { FiltersLoginChangedError, OpaqueFiltersError, updateAccountFilters } from '../../api/sieve-rules'; +import { jmapClient } from '../../api/jmap-client'; +import { servedLoginOf } from '../../lib/draft-login'; import { supportsPeriods } from '../../lib/sieve/period'; import { applyQuickRule, @@ -22,6 +24,7 @@ import { } from '../../lib/sieve/quick-rules'; import type { FilterRule } from '../../lib/sieve/types'; import { generateUUID } from '../../lib/uuid'; +import { useAccountStore } from '../../stores/account-store'; import { useFilterStore } from '../../stores/filter-store'; import { useKeywordsStore } from '../../stores/keywords-store'; import { useLocaleStore } from '../../stores/locale-store'; @@ -56,9 +59,20 @@ export function QuickRuleSheet({ visible, email, mailboxes, onClose }: QuickRule const identities = useSettingsStore((s) => s.identities); const keywords = useKeywordsStore((s) => s.keywords); const [step, setStep] = React.useState({ kind: 'menu' }); + // The login and Sieve account the sheet opened in. The rule is built from + // that login's message and folders; after a switch it must not go into the + // other login, where the same ids name other folders. + const openedIn = React.useRef<{ login: string | null; accountId: string | null }>({ login: null, accountId: null }); React.useEffect(() => { - if (visible) setStep({ kind: 'menu' }); + if (!visible) return; + setStep({ kind: 'menu' }); + let accountId: string | null = null; + try { accountId = getSieveAccountId(); } catch { /* not connected yet */ } + // Not connected yet (a cold start), the cached message is the active + // login's. + const login = servedLoginOf(jmapClient) ?? useAccountStore.getState().activeAccountId; + openedIn.current = { login, accountId }; }, [visible]); const ownAddresses = React.useMemo( @@ -85,11 +99,17 @@ export function QuickRuleSheet({ visible, email, mailboxes, onClose }: QuickRule /** Write `modify`'s rules into the own account's script and say how it went. */ const write = async (modify: (rules: FilterRule[]) => FilterRule[] | null) => { - const accountId = getSieveAccountId(); + const { login } = openedIn.current; + let accountId = openedIn.current.accountId; try { - await updateAccountFilters(modify, accountId); + // Read now only when the sheet opened before the session was up; + // updateAccountFilters refuses it unless the login is still the same. + accountId ??= getSieveAccountId(); + await updateAccountFilters(modify, accountId, login); } catch (err) { - if (err instanceof OpaqueFiltersError) { + if (err instanceof FiltersLoginChangedError) { + toast.error(t('context_menu.rules.login_changed', 'You switched accounts, so the rule was not saved')); + } else if (err instanceof OpaqueFiltersError) { toast.error(t('context_menu.rules.opaque_hint', 'Your filters were edited by hand. Open Filters settings')); } else { toast.error(t('notifications.filters_save_failed', 'Failed to save filters'), err instanceof Error ? err.message : undefined);