From 60167f752f791bb9fa0747a9f5f819eea9ef5894 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 28 Oct 2026 04:26:50 +0100 Subject: [PATCH] fix: label mail as External when any sender is outside or has no domain, and show the domain on a tap --- .../external-sender-chip-detail.test.tsx | 64 ++++++++++ .../__tests__/external-sender-chip.test.tsx | 4 +- components/email/email-viewer.tsx | 8 +- components/email/external-sender-chip.tsx | 115 ++++++++++++++---- components/email/thread-conversation-view.tsx | 2 +- lib/__tests__/external-mail.test.ts | 26 +++- lib/external-mail.ts | 29 ++++- locales/de/common.json | 3 +- locales/en/common.json | 3 +- 9 files changed, 212 insertions(+), 42 deletions(-) create mode 100644 components/email/__tests__/external-sender-chip-detail.test.tsx diff --git a/components/email/__tests__/external-sender-chip-detail.test.tsx b/components/email/__tests__/external-sender-chip-detail.test.tsx new file mode 100644 index 000000000..bd6ba9a3b --- /dev/null +++ b/components/email/__tests__/external-sender-chip-detail.test.tsx @@ -0,0 +1,64 @@ +import { render, fireEvent, cleanup, screen } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import type { Identity } from '@/lib/jmap/types'; +import { DEFAULT_ORGANISATION_POLICY } from '@/lib/admin/organisation-policy'; +import { DEFAULT_POLICY } from '@/lib/admin/types'; +import { usePolicyStore } from '@/stores/policy-store'; +import { useAuthStore } from '@/stores/auth-store'; + +// Strings with their values, so the domain can be seen in names and text. +vi.mock('next-intl', () => ({ + useTranslations: () => (key: string, vars?: Record) => + vars ? `${key}(${Object.values(vars).join(',')})` : key, + useLocale: () => 'en', +})); + +const { ExternalSenderChip } = await import('../external-sender-chip'); + +function setLabel(on: boolean) { + usePolicyStore.setState({ + policy: { + ...DEFAULT_POLICY, + organisation: { ...DEFAULT_ORGANISATION_POLICY, externalMail: { ...DEFAULT_ORGANISATION_POLICY.externalMail, senderLabel: on } }, + }, + }); +} + +beforeEach(() => { + setLabel(true); + useAuthStore.setState({ identities: [{ id: 'i1', name: 'Alice', email: 'alice@example.org' } as unknown as Identity] }); +}); +afterEach(() => cleanup()); + +describe('External chip on phones and keyboards', () => { + it('is a button whose name says the domain, and a tap shows it', () => { + render(); + const chip = screen.getByRole('button', { name: /org-two\.test/ }); + expect(chip.getAttribute('aria-expanded')).toBe('false'); + fireEvent.click(chip); + expect(chip.getAttribute('aria-expanded')).toBe('true'); + expect(document.querySelector('[data-external-sender-detail]')?.textContent).toBe('sender_tooltip(org-two.test)'); + // Escape and a tap elsewhere close it. + fireEvent.keyDown(chip, { key: 'Escape' }); + expect(document.querySelector('[data-external-sender-detail]')).toBeNull(); + fireEvent.click(chip); + fireEvent.pointerDown(document.body); + expect(document.querySelector('[data-external-sender-detail]')).toBeNull(); + }); + + it('shows for any external one of several From addresses', () => { + render(); + expect(screen.getByRole('button', { name: /evil\.example/ })).toBeTruthy(); + }); + + it('shows for a From without a domain', () => { + render(); + expect(screen.getByRole('button', { name: /sender_tooltip_no_domain/ })).toBeTruthy(); + }); + + it('stays a plain label inside another button', () => { + const { container } = render(); + expect(container.querySelectorAll('button')).toHaveLength(1); + expect(container.querySelector('[data-external-sender-chip]')?.getAttribute('title')).toBe('sender_tooltip(org-two.test)'); + }); +}); diff --git a/components/email/__tests__/external-sender-chip.test.tsx b/components/email/__tests__/external-sender-chip.test.tsx index a14269288..00e271fe2 100644 --- a/components/email/__tests__/external-sender-chip.test.tsx +++ b/components/email/__tests__/external-sender-chip.test.tsx @@ -63,7 +63,7 @@ beforeEach(() => { describe('External sender chip', () => { it('names the sender domain in its tooltip when the label is on', () => { setExternalMail({ senderLabel: true }); - const { container } = render(); + const { container } = render(); const chip = container.querySelector('[data-external-sender-chip]'); expect(chip?.textContent).toContain('sender_label'); expect(chip?.getAttribute('title')).toBe('sender_tooltip'); @@ -79,7 +79,7 @@ describe('External sender chip', () => { setExternalMail({ senderLabel: true }); const { container } = await openMessage('org-bob@example.org'); expect(container.querySelector('[data-external-sender-chip]')).toBeNull(); - const sub = render(); + const sub = render(); expect(sub.container.querySelector('[data-external-sender-chip]')).toBeNull(); }); diff --git a/components/email/email-viewer.tsx b/components/email/email-viewer.tsx index 504ff96a7..01fe12d50 100644 --- a/components/email/email-viewer.tsx +++ b/components/email/email-viewer.tsx @@ -856,7 +856,7 @@ export function MvExpandedConversationCard({ {!compact && sender?.name && sender.email && ( <{sender.email}> )} - + {compact ? formatDate(emailDisplayDate(email)) : mvSenderDate(emailDisplayDate(email), timeFormat)} @@ -5051,7 +5051,7 @@ export function EmailViewer({ {mvViewer && sender?.email && sender?.name && ( <{sender.email}> )} - + {shouldShowUnsubBanner && listHeaders?.listUnsubscribe && ( {t('unknown_sender')} )} - + {shouldShowUnsubBanner && listHeaders?.listUnsubscribe && ( - + {senderVerification && ( s.policy.organisation?.externalMail); const identities = useAuthStore((s) => s.identities); return useMemo(() => { - if (!externalMail?.senderLabel) return null; + if (!externalMail?.senderLabel) return []; const internal = internalDomains((identities ?? []).map((identity) => identity.email), externalMail.domains); - return externalSenderDomain(fromEmail, internal); - }, [externalMail, identities, fromEmail]); + return externalSenders(from, internal); + }, [externalMail, identities, from]); } /** * "External" next to the sender's name. A chip and not a banner, so it sits * beside the unverified-sender warning without adding a second banner, and * it keeps to one short word so the narrow phone header doesn't wrap more. + * + * It is a button: a tap (phones have no hover) or Enter opens a line naming + * the domain. Inside another button (a conversation card's header) it can't + * be one, so `interactive={false}` keeps it a label with a tooltip. */ -export function ExternalSenderChip({ fromEmail, className }: { fromEmail: string | null | undefined; className?: string }) { +export function ExternalSenderChip({ from, className, interactive = true }: { from: FromList; className?: string; interactive?: boolean }) { const t = useTranslations('external_mail'); - const domain = useExternalSenderDomain(fromEmail); - if (!domain) return null; - const tooltip = t('sender_tooltip', { domain }); - return ( - + const senders = useExternalSenders(from); + const [open, setOpen] = useState(false); + const wrapRef = useRef(null); + const popoverId = useId(); + + useEffect(() => { + if (!open) return; + const close = (e: Event) => { + if (wrapRef.current && !wrapRef.current.contains(e.target as Node)) setOpen(false); + }; + document.addEventListener('pointerdown', close); + document.addEventListener('focusin', close); + return () => { + document.removeEventListener('pointerdown', close); + document.removeEventListener('focusin', close); + }; + }, [open]); + + if (senders.length === 0) return null; + const domains = [...new Set(senders.map((s) => s.domain))]; + const named = domains.filter(Boolean).join(', '); + const description = named + ? t('sender_tooltip', { domain: named }) + : t('sender_tooltip_no_domain'); + + const chipClass = cn( + 'inline-flex flex-shrink-0 items-center gap-1 whitespace-nowrap rounded-md border px-1.5 py-0.5 text-xs font-medium leading-none', + 'border-warning/30 bg-warning/10 text-warning', + ); + const content = ( + <> - + {isStarred && ( )} diff --git a/lib/__tests__/external-mail.test.ts b/lib/__tests__/external-mail.test.ts index 57ba06cfd..06a408200 100644 --- a/lib/__tests__/external-mail.test.ts +++ b/lib/__tests__/external-mail.test.ts @@ -7,7 +7,7 @@ import { currentInternalDomains, domainMatches, externalRecipients, - externalSenderDomain, + externalSenders, internalDomains, isExternalAddress, } from '../external-mail'; @@ -114,11 +114,25 @@ describe('externalRecipients', () => { }); }); -describe('externalSenderDomain', () => { - it('names the domain of an external sender and nothing for internal ones', () => { - expect(externalSenderDomain('dave@org-two.test', ['example.org'])).toBe('org-two.test'); - expect(externalSenderDomain('bob@example.org', ['example.org'])).toBeNull(); - expect(externalSenderDomain(undefined, ['example.org'])).toBeNull(); +describe('externalSenders', () => { + const internal = ['example.org']; + it('names the external senders and nothing for internal ones', () => { + expect(externalSenders([{ email: 'dave@org-two.test' }], internal)).toEqual([{ address: 'dave@org-two.test', domain: 'org-two.test' }]); + expect(externalSenders([{ email: 'bob@example.org' }], internal)).toEqual([]); + expect(externalSenders(undefined, internal)).toEqual([]); + }); + + it('counts a message as external when any of several From addresses is', () => { + expect(externalSenders([{ email: 'bob@example.org' }, { email: 'eve@evil.example' }], internal)) + .toEqual([{ address: 'eve@evil.example', domain: 'evil.example' }]); + }); + + it('counts a From without a domain as external', () => { + expect(externalSenders([{ email: 'ceo' }], internal)).toEqual([{ address: 'ceo', domain: '' }]); + }); + + it('calls nobody external when no internal domain is known', () => { + expect(externalSenders([{ email: 'ceo' }, { email: 'dave@org-two.test' }], [])).toEqual([]); }); }); diff --git a/lib/external-mail.ts b/lib/external-mail.ts index f3ecc101f..8ae1f5113 100644 --- a/lib/external-mail.ts +++ b/lib/external-mail.ts @@ -101,10 +101,31 @@ export function externalRecipients( return out; } -/** The sender's domain when it is outside the organisation, else null. */ -export function externalSenderDomain(fromEmail: string | null | undefined, internal: readonly string[]): string | null { - if (!fromEmail || !isExternalAddress(fromEmail, internal)) return null; - return addressDomain(fromEmail); +export interface ExternalSender { + address: string; + /** ASCII domain; '' for an address without one. */ + domain: string; +} + +/** + * The From addresses that are outside the organisation. A message counts as + * external when any of them is, and an address without a domain + * ("CEO ") counts too: no organisation sends like that. A forged + * internal From is not caught here; the sender check warns about that. + */ +export function externalSenders( + from: readonly { email?: string | null }[] | null | undefined, + internal: readonly string[], +): ExternalSender[] { + if (!from || internal.length === 0) return []; + const out: ExternalSender[] = []; + for (const sender of from) { + const address = String(sender?.email ?? '').trim(); + const domain = addressDomain(address); + if (domain && domainMatches(domain, internal)) continue; + out.push({ address, domain }); + } + return out; } // ─── The active account ────────────────────────────────────── diff --git a/locales/de/common.json b/locales/de/common.json index 729bb80ca..61f24c102 100644 --- a/locales/de/common.json +++ b/locales/de/common.json @@ -661,7 +661,8 @@ }, "external_mail": { "sender_label": "Extern", - "sender_tooltip": "Von außerhalb Ihrer Organisation: {domain}", + "sender_tooltip": "Von {domain}, außerhalb Ihrer Organisation", + "sender_tooltip_no_domain": "Von einer Adresse ohne Domain, außerhalb Ihrer Organisation", "confirm_title": "Außerhalb Ihrer Organisation senden?", "confirm_message": "{count, plural, one {Dieser Empfänger ist} other {Diese # Empfänger sind}} außerhalb Ihrer Organisation. Senden Sie nur, wenn das so gewollt ist.", "field_to": "An", diff --git a/locales/en/common.json b/locales/en/common.json index b8c4ecb2c..dd844e682 100644 --- a/locales/en/common.json +++ b/locales/en/common.json @@ -661,7 +661,8 @@ }, "external_mail": { "sender_label": "External", - "sender_tooltip": "From outside your organisation: {domain}", + "sender_tooltip": "From {domain}, outside your organisation", + "sender_tooltip_no_domain": "From an address without a domain, outside your organisation", "confirm_title": "Send outside your organisation?", "confirm_message": "{count, plural, one {This recipient is} other {These # recipients are}} outside your organisation. Send only if you mean to.", "field_to": "To",