From b945a789f96b1db3119ea5ff255fee7dbc25b80c Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 28 Oct 2026 04:05:46 +0100 Subject: [PATCH] fix: match internationalised domains in either form and name the domains an admin typed wrong --- .../__tests__/external-mail-section.test.tsx | 11 ++++ .../org/sections/external-mail-section.tsx | 26 ++++++--- components/email/external-sender-chip.tsx | 2 +- lib/__tests__/external-mail.test.ts | 34 +++++++++-- .../__tests__/organisation-policy.test.ts | 9 +++ lib/admin/organisation-policy.ts | 31 ++++++++-- lib/external-mail.ts | 57 ++++++++++++------- 7 files changed, 133 insertions(+), 37 deletions(-) diff --git a/components/admin/org/__tests__/external-mail-section.test.tsx b/components/admin/org/__tests__/external-mail-section.test.tsx index 6bc888151..efa4739f7 100644 --- a/components/admin/org/__tests__/external-mail-section.test.tsx +++ b/components/admin/org/__tests__/external-mail-section.test.tsx @@ -82,6 +82,17 @@ describe('External mail section', () => { fireEvent.change(field, { target: { value: 'partner.com, Sister.example\n@third.test' } }); fireEvent.blur(field); expect(latest!.orgValue('externalMail', 'domains').value).toEqual(['partner.com', 'sister.example', 'third.test']); + expect(screen.queryByTestId('org-external-domains-rejected')).toBeNull(); + }); + + it('stores an IDN in ASCII, shows it in Unicode and names entries that are not domains', async () => { + await renderSection(); + const field = screen.getByTestId('org-external-domains') as HTMLTextAreaElement; + fireEvent.change(field, { target: { value: 'bücher.de\nnot_a_domain, x' } }); + fireEvent.blur(field); + expect(latest!.orgValue('externalMail', 'domains').value).toEqual(['xn--bcher-kva.de']); + expect(screen.getByTestId('org-external-domains-rejected').textContent).toContain('not_a_domain, x'); + await waitFor(() => expect(field.value).toBe('bücher.de')); }); it('offers no import and no note without the plugin', async () => { diff --git a/components/admin/org/sections/external-mail-section.tsx b/components/admin/org/sections/external-mail-section.tsx index 3e50e5e24..e77c38efc 100644 --- a/components/admin/org/sections/external-mail-section.tsx +++ b/components/admin/org/sections/external-mail-section.tsx @@ -5,7 +5,8 @@ import { Download, Info } from '@/components/icons'; import { Button } from '@/components/ui/button'; import { SettingItem, SettingsSection, ToggleSwitch } from '@/components/settings/settings-section'; import { AdminLink } from '@/components/admin/admin-link'; -import { normalizeDomainList } from '@/lib/admin/organisation-policy'; +import { parseDomainList } from '@/lib/admin/organisation-policy'; +import { toUnicodeDomain } from '@/lib/idn'; import { apiFetch } from '@/lib/browser-navigation'; import { EXTERNAL_MAIL_WARNING_PLUGIN_ID } from '@/lib/plugin-sandbox/host-features'; import { InheritNote } from '../org-page'; @@ -59,13 +60,17 @@ export function ExternalMailSection({ draft }: { draft: OrgPolicyDraft }) { const anyOn = senderLabel.value || recipientWarning.value; // The list is typed as text and taken apart when the field is left, so a - // half-typed domain isn't thrown away on every key. - const joined = domains.value.join('\n'); + // half-typed domain isn't thrown away on every key. It is stored in ASCII + // (xn--) form and shown in Unicode; entries that aren't domains are named. + const stored = domains.value.join('\n'); + const joined = domains.value.map(toUnicodeDomain).join('\n'); const [text, setText] = useState(joined); + const [rejected, setRejected] = useState([]); useEffect(() => { setText(joined); }, [joined]); const commitDomains = (raw: string) => { - const next = normalizeDomainList(raw) ?? []; - if (next.join('\n') !== joined) draft.setOrgValue('externalMail', 'domains', next); + const parsed = parseDomainList(raw) ?? { domains: [], rejected: [] }; + setRejected(parsed.rejected); + if (parsed.domains.join('\n') !== stored) draft.setOrgValue('externalMail', 'domains', parsed.domains); else setText(joined); }; @@ -76,7 +81,9 @@ export function ExternalMailSection({ draft }: { draft: OrgPolicyDraft }) { const res = await apiFetch(`/api/admin/plugins/${EXTERNAL_MAIL_WARNING_PLUGIN_ID}/config`); if (!res.ok) throw new Error(String(res.status)); const config = (await res.json()) as Record; - const fromPlugin = normalizeDomainList(config.safeDomains) ?? []; + const parsed = parseDomainList(config.safeDomains) ?? { domains: [], rejected: [] }; + const fromPlugin = parsed.domains; + setRejected(parsed.rejected); const added = fromPlugin.filter((d) => !domains.value.includes(d)); if (added.length > 0) draft.setOrgValue('externalMail', 'domains', [...domains.value, ...added]); draft.setMessage({ @@ -137,10 +144,15 @@ export function ExternalMailSection({ draft }: { draft: OrgPolicyDraft }) { spellCheck={false} className="w-full rounded-md border border-input bg-background px-3 py-2 font-mono text-sm text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-ring" /> + {rejected.length > 0 && ( +

+ Left out, not a domain: {rejected.join(', ')} +

+ )} draft.clearOrgValue('externalMail', 'domains')} /> {plugin && ( diff --git a/components/email/external-sender-chip.tsx b/components/email/external-sender-chip.tsx index 621fd0a41..46107a44b 100644 --- a/components/email/external-sender-chip.tsx +++ b/components/email/external-sender-chip.tsx @@ -17,7 +17,7 @@ export function useExternalSenderDomain(fromEmail: string | null | undefined): s const identities = useAuthStore((s) => s.identities); return useMemo(() => { if (!externalMail?.senderLabel) return null; - const internal = internalDomains(externalMail.domains, (identities ?? []).map((identity) => identity.email)); + const internal = internalDomains((identities ?? []).map((identity) => identity.email), externalMail.domains); return externalSenderDomain(fromEmail, internal); }, [externalMail, identities, fromEmail]); } diff --git a/lib/__tests__/external-mail.test.ts b/lib/__tests__/external-mail.test.ts index 5f07e76ca..be3449906 100644 --- a/lib/__tests__/external-mail.test.ts +++ b/lib/__tests__/external-mail.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { addressDomain, + addressKey, contactAddresses, coreExternalWarningsOn, currentInternalDomains, @@ -52,13 +53,38 @@ describe('domainMatches', () => { describe('internalDomains', () => { it('always counts the identity domains and adds the admin list', () => { - expect(internalDomains([], ['me@example.org'])).toEqual(['example.org']); - expect(internalDomains(['partner.com'], ['me@example.org', 'alias@Example.org'])).toEqual(['partner.com', 'example.org']); + expect(internalDomains(['me@example.org'], [])).toEqual(['example.org']); + expect(internalDomains(['me@example.org', 'alias@Example.org'], ['partner.com'])).toEqual(['example.org', 'partner.com']); + }); + + it('keeps the identity domains when the admin list fills the cap', () => { + const full = Array.from({ length: 500 }, (_, i) => `partner${i}.example`); + const internal = internalDomains(['me@own.example'], full); + expect(internal).toHaveLength(500); + expect(internal[0]).toBe('own.example'); + }); +}); + +describe('IDN domains', () => { + it('compares Unicode and xn-- forms as the same domain, on both sides', () => { + // The admin typed it in Unicode, the server delivers xn--. + const internal = internalDomains([], ['bücher.de']); + expect(internal).toEqual(['xn--bcher-kva.de']); + expect(isExternalAddress('anna@xn--bcher-kva.de', internal)).toBe(false); + expect(isExternalAddress('anna@bücher.de', internal)).toBe(false); + expect(isExternalAddress('anna@eu.bücher.de', internal)).toBe(false); + expect(isExternalAddress('anna@buecher.de', internal)).toBe(true); + // An identity on an IDN domain counts in whichever form the address has. + expect(isExternalAddress('x@münchen.example', internalDomains(['me@xn--mnchen-3ya.example'], []))).toBe(false); + }); + + it('matches a contact written in Unicode against an xn-- recipient', () => { + expect(addressKey('Anna@Bücher.de')).toBe(addressKey('anna@xn--bcher-kva.de')); }); }); describe('isExternalAddress', () => { - const internal = internalDomains(['partner.com'], ['me@example.org']); + const internal = internalDomains(['me@example.org'], ['partner.com']); it('is external only outside the identity domains and the list, subdomains included', () => { expect(isExternalAddress('bob@example.org', internal)).toBe(false); expect(isExternalAddress('x@eu.partner.com', internal)).toBe(false); @@ -114,7 +140,7 @@ describe('the active account', () => { it('combines the policy domains with the identity domains', () => { setExternalMail({ domains: ['partner.com'] }); useAuthStore.setState({ identities: [identity('org-alice@example.org')] }); - expect(currentInternalDomains()).toEqual(['partner.com', 'example.org']); + expect(currentInternalDomains()).toEqual(['example.org', 'partner.com']); }); it('collects the addresses of every contact', () => { diff --git a/lib/admin/__tests__/organisation-policy.test.ts b/lib/admin/__tests__/organisation-policy.test.ts index 7d6b8ce25..58afdeb4d 100644 --- a/lib/admin/__tests__/organisation-policy.test.ts +++ b/lib/admin/__tests__/organisation-policy.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { DEFAULT_ORGANISATION_POLICY, normalizeDomainList, + parseDomainList, sanitizeOrganisationPolicy, } from '@/lib/admin/organisation-policy'; import { applyLitePolicy } from '@/lib/lite-config'; @@ -32,6 +33,14 @@ describe('sanitizeOrganisationPolicy', () => { expect(normalizeDomainList(['A.org', 'a.org', 'x'])).toEqual(['a.org']); expect(normalizeDomainList(42)).toBeUndefined(); }); + + it('takes IDN domains in ASCII form and names what it left out', () => { + expect(parseDomainList('Bücher.de, xn--bcher-kva.de, @partner.example, *.sub.example\nnot a domain, x')).toEqual({ + domains: ['xn--bcher-kva.de', 'partner.example', 'sub.example'], + rejected: ['not', 'a', 'domain', 'x'], + }); + expect(parseDomainList(['bad_domain.example', 'ok.example'])).toEqual({ domains: ['ok.example'], rejected: ['bad_domain.example'] }); + }); }); describe('applyLitePolicy', () => { diff --git a/lib/admin/organisation-policy.ts b/lib/admin/organisation-policy.ts index 77562da19..c61bcc89c 100644 --- a/lib/admin/organisation-policy.ts +++ b/lib/admin/organisation-policy.ts @@ -11,6 +11,7 @@ */ import type { EmailTemplate } from '@/lib/template-types'; +import { toAsciiDomain } from '@/lib/idn'; /** Details a user fills in once for the organisation signature. */ export const SIGNATURE_FIELDS = ['title', 'department', 'phone', 'mobile'] as const; @@ -101,18 +102,36 @@ function count(value: unknown, max: number): number | undefined { return Math.min(max, Math.max(0, Math.round(value))); } -/** Lowercased, trimmed, deduplicated domains; "@example.com" and "*.example.com" are taken as example.com. */ -export function normalizeDomainList(value: unknown): string[] | undefined { +/** + * A domain list taken apart: the usable domains, lowercased, deduplicated + * and in ASCII form (an IDN like bücher.de becomes xn--bcher-kva.de, the + * form mail servers use), and the entries that aren't domains. + * "@example.com" and "*.example.com" are taken as example.com. Undefined for + * anything that isn't a string or an array. + */ +export function parseDomainList(value: unknown): { domains: string[]; rejected: string[] } | undefined { const raw = typeof value === 'string' ? value.split(/[\s,;]+/) : Array.isArray(value) ? value : undefined; if (!raw) return undefined; const out = new Set(); + const rejected: string[] = []; for (const item of raw) { if (typeof item !== 'string') continue; - const domain = item.trim().toLowerCase().replace(/^\*\./, '').replace(/^@/, '').replace(/\.+$/, ''); - if (domain && DOMAIN_RE.test(domain)) out.add(domain); - if (out.size >= MAX_DOMAINS) break; + const entry = item.trim(); + if (!entry) continue; + const bare = entry.toLowerCase().replace(/^\*\./, '').replace(/^@/, '').replace(/\.+$/, ''); + const domain = bare ? toAsciiDomain(bare) : null; + if (domain && DOMAIN_RE.test(domain)) { + if (out.size < MAX_DOMAINS) out.add(domain); + } else { + rejected.push(entry); + } } - return [...out]; + return { domains: [...out], rejected }; +} + +/** The usable domains of a list (see parseDomainList). */ +export function normalizeDomainList(value: unknown): string[] | undefined { + return parseDomainList(value)?.domains; } function sanitizeLibrary(value: unknown): OrgTemplate[] | undefined { diff --git a/lib/external-mail.ts b/lib/external-mail.ts index d81add2b0..0fe756a9b 100644 --- a/lib/external-mail.ts +++ b/lib/external-mail.ts @@ -7,41 +7,60 @@ * covers eu.example.com). With no internal domain known at all nobody can * be told apart, so nothing counts as external. * + * Domains are compared in ASCII (punycode) form on both sides, so an IDN + * the admin typed in Unicode matches the xn-- form a server delivers. + * * The pure functions take everything as arguments; the `current*` helpers * read the active account's effective policy and identities. */ import { DEFAULT_ORGANISATION_POLICY, normalizeDomainList, type OrganisationPolicy } from '@/lib/admin/organisation-policy'; +import { toAsciiDomain, toAsciiEmail } from '@/lib/idn'; import { usePolicyStore } from '@/stores/policy-store'; import { useAuthStore } from '@/stores/auth-store'; import { useContactStore } from '@/stores/contact-store'; export type ExternalMailPolicy = OrganisationPolicy['externalMail']; -/** The lowercased domain of an address ("Name " or "a@b"); '' without one. */ -export function addressDomain(address: string | null | undefined): string { +/** The bare address of "Name " or "a@b". */ +function bareAddress(address: string | null | undefined): string { const raw = String(address ?? '').trim(); const bracket = raw.match(/<([^<>]*)>\s*$/); - const bare = (bracket ? bracket[1] : raw).trim(); - const at = bare.lastIndexOf('@'); - if (at < 0) return ''; - return bare.slice(at + 1).trim().toLowerCase().replace(/\.+$/, ''); + return (bracket ? bracket[1] : raw).trim(); } -/** True when `domain` is one of `internal` or a subdomain of one. */ +/** The domain of an address in lowercased ASCII form; '' without a usable one. */ +export function addressDomain(address: string | null | undefined): string { + const bare = bareAddress(address); + const at = bare.lastIndexOf('@'); + if (at < 0) return ''; + const domain = bare.slice(at + 1).trim().replace(/\.+$/, ''); + return (domain && toAsciiDomain(domain)) || ''; +} + +/** The address as a lookup key: the domain in ASCII form, all lowercased. */ +export function addressKey(address: string): string { + return toAsciiEmail(bareAddress(address)).toLowerCase(); +} + +/** True when `domain` (ASCII) is one of `internal` or a subdomain of one. */ export function domainMatches(domain: string, internal: readonly string[]): boolean { if (!domain) return false; const d = domain.toLowerCase(); return internal.some((entry) => d === entry || d.endsWith(`.${entry}`)); } -/** The admin's domains plus the domains of the user's identities, normalised and deduplicated. */ -export function internalDomains(policyDomains: readonly string[], identityEmails: readonly string[]): string[] { +/** + * The domains of the user's identities, then the admin's, in ASCII form and + * deduplicated. The identity domains come first so the list's size cap never + * cuts them. + */ +export function internalDomains(identityEmails: readonly string[], policyDomains: readonly string[]): string[] { const own = identityEmails.map(addressDomain).filter(Boolean); - return normalizeDomainList([...policyDomains, ...own]) ?? []; + return normalizeDomainList([...own, ...policyDomains]) ?? []; } -/** Whether mail from or to this address leaves the organisation. */ +/** Whether mail to this address leaves the organisation. An address without a domain isn't sendable and isn't counted. */ export function isExternalAddress(address: string, internal: readonly string[]): boolean { if (internal.length === 0) return false; const domain = addressDomain(address); @@ -58,20 +77,20 @@ export interface ExternalRecipient { /** * The recipients outside the organisation, each address once (in the first - * field it appears in). `trusted` is asked for the rest: with "trust - * contacts" on, an address in the user's contacts doesn't count. + * field it appears in). `trusted` is asked for the rest, with the address as + * addressKey makes it: with "trust contacts" on, a contact doesn't count. */ export function externalRecipients( recipients: { to: readonly string[]; cc: readonly string[]; bcc: readonly string[] }, internal: readonly string[], - trusted?: (address: string) => boolean, + trusted?: (key: string) => boolean, ): ExternalRecipient[] { const out: ExternalRecipient[] = []; const seen = new Set(); for (const field of ['to', 'cc', 'bcc'] as const) { for (const raw of recipients[field]) { const address = String(raw ?? '').trim(); - const key = address.toLowerCase(); + const key = addressKey(address); if (!address || seen.has(key)) continue; seen.add(key); if (!isExternalAddress(address, internal)) continue; @@ -103,16 +122,16 @@ export function coreExternalWarningsOn(policy: ExternalMailPolicy = currentExter /** The internal domains for the active account. */ export function currentInternalDomains(policy: ExternalMailPolicy = currentExternalMailPolicy()): string[] { const identities = useAuthStore.getState().identities ?? []; - return internalDomains(policy.domains, identities.map((identity) => identity.email)); + return internalDomains(identities.map((identity) => identity.email), policy.domains); } -/** Lowercased addresses of every loaded contact (all address books). */ +/** The addresses of every loaded contact (all address books), as addressKey makes them. */ export function contactAddresses(): Set { const out = new Set(); for (const contact of useContactStore.getState().contacts) { for (const entry of Object.values(contact.emails ?? {})) { - const address = entry?.address?.trim().toLowerCase(); - if (address) out.add(address); + const address = entry?.address?.trim(); + if (address) out.add(addressKey(address)); } } return out;