fix: judge external mail by the message's own account and the sending account in several-account setups

This commit is contained in:
Linus Rath
2026-10-28 05:11:24 +01:00
parent 60167f752f
commit 1202a13720
11 changed files with 250 additions and 41 deletions
@@ -1,4 +1,4 @@
import { render, fireEvent, cleanup, screen } from '@testing-library/react';
import { render, fireEvent, cleanup, screen, waitFor } 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';
@@ -62,3 +62,45 @@ describe('External chip on phones and keyboards', () => {
expect(container.querySelector('[data-external-sender-chip]')?.getAttribute('title')).toBe('sender_tooltip(org-two.test)');
});
});
describe('a message of another signed-in account (unified inbox)', () => {
function signInDave(daveScope: Record<string, unknown> | null) {
usePolicyStore.setState({
rawPolicy: {
...DEFAULT_POLICY,
organisation: { ...DEFAULT_ORGANISATION_POLICY, externalMail: { ...DEFAULT_ORGANISATION_POLICY.externalMail, senderLabel: true } },
scopes: daveScope ? [{ id: 'two', label: 'Two', match: { domains: ['org-two.test'] }, policy: { organisation: { externalMail: daveScope } } }] : [],
},
});
const daveClient = {
getUsername: () => 'org-dave@org-two.test',
getSharedAccounts: () => [],
getAccountInfo: () => null,
getPrincipalTypes: async () => ({}),
getIdentities: async () => [{ id: 'd1', name: 'Dave', email: 'org-dave@org-two.test' }],
};
useAuthStore.setState({
activeAccountId: 'acc-alice',
getClientForAccount: (id: string) => (id === 'acc-dave' ? daveClient : undefined),
} as never);
}
it("is judged with that account's identities", async () => {
signInDave(null);
// For dave, a colleague at org-two.test is internal and alice is not.
const colleague = render(<ExternalSenderChip from={[{ email: 'colleague@org-two.test' }]} accountId="acc-dave" />);
const alice = render(<ExternalSenderChip from={[{ email: 'alice@example.org' }]} accountId="acc-dave" />);
await waitFor(() => expect(alice.container.querySelector('[data-external-sender-chip]')).not.toBeNull());
expect(colleague.container.querySelector('[data-external-sender-chip]')).toBeNull();
});
it("follows that account's scope, not the active one's", async () => {
signInDave({ senderLabel: false });
const { container } = render(<ExternalSenderChip from={[{ email: 'someone@elsewhere.example' }]} accountId="acc-dave" />);
// The active account has the label on; dave's scope turns it off.
await new Promise((r) => setTimeout(r, 20));
expect(container.querySelector('[data-external-sender-chip]')).toBeNull();
const active = render(<ExternalSenderChip from={[{ email: 'someone@elsewhere.example' }]} />);
expect(active.container.querySelector('[data-external-sender-chip]')).not.toBeNull();
});
});
+1
View File
@@ -3292,6 +3292,7 @@ export function EmailComposer({
fromEmail: (fromOverrideEnabled && fromOverrideEmail.trim()) || currentIdentity?.email || undefined,
identityId: currentIdentity?.id,
identityEmail: currentIdentity?.email || undefined,
localAccountId: currentIdentityParts.localAccountId ?? undefined,
mode,
plainText: plainTextMode,
};
+4 -4
View File
@@ -856,7 +856,7 @@ export function MvExpandedConversationCard({
{!compact && sender?.name && sender.email && (
<span className="text-xs text-muted-foreground truncate">&lt;{sender.email}&gt;</span>
)}
<ExternalSenderChip from={email.from} className="self-center" />
<ExternalSenderChip from={email.from} accountId={email.sourceClientAccountId} className="self-center" />
<span className="ms-auto ps-2 text-xs text-muted-foreground whitespace-nowrap">
{compact ? formatDate(emailDisplayDate(email)) : mvSenderDate(emailDisplayDate(email), timeFormat)}
</span>
@@ -5051,7 +5051,7 @@ export function EmailViewer({
{mvViewer && sender?.email && sender?.name && (
<span className="text-xs text-muted-foreground truncate">&lt;{sender.email}&gt;</span>
)}
<ExternalSenderChip from={email.from} />
<ExternalSenderChip from={email.from} accountId={email.sourceClientAccountId} />
<EmailIdentityBadge email={email} identities={identities} />
{shouldShowUnsubBanner && listHeaders?.listUnsubscribe && (
<UnsubscribeBanner
@@ -5341,7 +5341,7 @@ export function EmailViewer({
) : (
<span className="text-sm font-semibold text-foreground">{t('unknown_sender')}</span>
)}
<ExternalSenderChip from={email.from} />
<ExternalSenderChip from={email.from} accountId={email.sourceClientAccountId} />
<EmailIdentityBadge email={email} identities={identities} />
{shouldShowUnsubBanner && listHeaders?.listUnsubscribe && (
<UnsubscribeBanner
@@ -5529,7 +5529,7 @@ export function EmailViewer({
onViewContact={handleViewContactSidebar}
className="text-sm text-start"
/>
<ExternalSenderChip from={email.from} />
<ExternalSenderChip from={email.from} accountId={email.sourceClientAccountId} />
{senderVerification && (
<span
className={cn(
+48 -10
View File
@@ -4,24 +4,56 @@ import { useEffect, useId, useMemo, useRef, useState } from 'react';
import { useTranslations } from 'next-intl';
import { Building } from '@/components/icons';
import { cn } from '@/lib/utils';
import { externalSenders, internalDomains, type ExternalSender } from '@/lib/external-mail';
import {
externalMailContextFor,
externalSenders,
internalDomains,
isOtherAccount,
type ExternalMailContext,
type ExternalSender,
} from '@/lib/external-mail';
import { usePolicyStore } from '@/stores/policy-store';
import { useAuthStore } from '@/stores/auth-store';
type FromList = readonly { email?: string | null }[] | null | undefined;
/**
* The From addresses outside the organisation when the admin turned the
* "External" label on (lib/external-mail.ts); empty otherwise.
* The context of the account a message belongs to: the active one from the
* stores, another signed-in one (the unified inbox) resolved for that
* account, so its own scopes and identities decide.
*/
export function useExternalSenders(from: FromList): ExternalSender[] {
function useAccountContext(accountId: string | null | undefined): ExternalMailContext | null {
const externalMail = usePolicyStore((s) => s.policy.organisation?.externalMail);
const rawPolicy = usePolicyStore((s) => s.rawPolicy);
const identities = useAuthStore((s) => s.identities);
const other = useAuthStore(() => isOtherAccount(accountId));
const [otherContext, setOtherContext] = useState<{ accountId: string; context: ExternalMailContext } | null>(null);
useEffect(() => {
if (!other || !accountId) return;
let cancelled = false;
void externalMailContextFor(accountId).then((context) => {
if (!cancelled) setOtherContext({ accountId, context });
});
return () => { cancelled = true; };
}, [other, accountId, rawPolicy]);
return useMemo(() => {
if (!externalMail?.senderLabel) return [];
const internal = internalDomains((identities ?? []).map((identity) => identity.email), externalMail.domains);
return externalSenders(from, internal);
}, [externalMail, identities, from]);
if (other) return otherContext && otherContext.accountId === accountId ? otherContext.context : null;
if (!externalMail) return null;
return { policy: externalMail, identityEmails: (identities ?? []).map((identity) => identity.email) };
}, [other, otherContext, accountId, externalMail, identities]);
}
/**
* The From addresses outside the organisation when the admin turned the
* "External" label on for the message's account (lib/external-mail.ts);
* empty otherwise.
*/
export function useExternalSenders(from: FromList, accountId?: string | null): ExternalSender[] {
const context = useAccountContext(accountId);
return useMemo(() => {
if (!context?.policy.senderLabel) return [];
return externalSenders(from, internalDomains(context.identityEmails, context.policy.domains));
}, [context, from]);
}
/**
@@ -33,9 +65,15 @@ export function useExternalSenders(from: FromList): ExternalSender[] {
* 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({ from, className, interactive = true }: { from: FromList; className?: string; interactive?: boolean }) {
export function ExternalSenderChip({ from, accountId, className, interactive = true }: {
from: FromList;
/** The signed-in account the message belongs to (Email.sourceClientAccountId); the active one when not set. */
accountId?: string | null;
className?: string;
interactive?: boolean;
}) {
const t = useTranslations('external_mail');
const senders = useExternalSenders(from);
const senders = useExternalSenders(from, accountId);
const [open, setOpen] = useState(false);
const wrapRef = useRef<HTMLSpanElement>(null);
const popoverId = useId();
@@ -397,7 +397,7 @@ function EmailCard({
)}>
{sender?.name || (sender?.email && formatAddress(sender.email)) || "Unknown"}
</span>
<ExternalSenderChip from={email.from} interactive={false} />
<ExternalSenderChip from={email.from} accountId={email.sourceClientAccountId} interactive={false} />
{isStarred && (
<Star className="w-4 h-4 fill-amber-400 text-amber-400 flex-shrink-0" />
)}
@@ -21,6 +21,11 @@ const CROSS_ACCOUNT_IDENTITY_DELIMITER = '::';
// still refetches.
const remoteIdentityCache = new Map<string, Identity[]>();
/** One non-active account's identities, if fetched already (raw ids). */
export function cachedIdentitiesForAccount(localAccountId: string): Identity[] | undefined {
return remoteIdentityCache.get(localAccountId);
}
/** Every non-active account's identities fetched so far (raw ids). */
export function cachedRemoteIdentities(): Identity[] {
return [...remoteIdentityCache.values()].flat();
+36 -8
View File
@@ -3,8 +3,9 @@ import {
addressDomain,
addressKey,
contactAddresses,
coreExternalWarningsOn,
currentInternalDomains,
activeExternalMailContext,
contextInternalDomains,
externalMailContextFor,
domainMatches,
externalRecipients,
externalSenders,
@@ -139,22 +140,49 @@ describe('externalSenders', () => {
describe('the active account', () => {
beforeEach(() => {
setExternalMail({});
useAuthStore.setState({ identities: [] });
useAuthStore.setState({ identities: [], activeAccountId: 'acc-alice' });
useContactStore.setState({ contacts: [], addressBooks: [], trustedSendersBookId: null });
});
it('reads the switches from the effective policy', () => {
expect(coreExternalWarningsOn()).toBe(false);
expect(activeExternalMailContext().policy.senderLabel).toBe(false);
setExternalMail({ senderLabel: true });
expect(coreExternalWarningsOn()).toBe(true);
setExternalMail({ recipientWarning: true });
expect(coreExternalWarningsOn()).toBe(true);
expect(activeExternalMailContext().policy.senderLabel).toBe(true);
});
it('combines the policy domains with the identity domains', () => {
setExternalMail({ domains: ['partner.com'] });
useAuthStore.setState({ identities: [identity('org-alice@example.org')] });
expect(currentInternalDomains()).toEqual(['example.org', 'partner.com']);
expect(contextInternalDomains(activeExternalMailContext())).toEqual(['example.org', 'partner.com']);
});
it('resolves another signed-in account with its own scopes and identities', async () => {
// The raw policy: the label on for everyone, off for org-two.test.
usePolicyStore.setState({
rawPolicy: {
...DEFAULT_POLICY,
organisation: { ...DEFAULT_ORGANISATION_POLICY, externalMail: { ...DEFAULT_ORGANISATION_POLICY.externalMail, senderLabel: true } },
scopes: [{ id: 'two', label: 'Two', match: { domains: ['org-two.test'] }, policy: { organisation: { externalMail: { senderLabel: false } } } }],
},
});
setExternalMail({ senderLabel: true });
const daveClient = {
getUsername: () => 'org-dave@org-two.test',
getSharedAccounts: () => [],
getAccountInfo: () => null,
getPrincipalTypes: async () => ({}),
getIdentities: async () => [identity('org-dave@org-two.test')],
};
useAuthStore.setState({
identities: [identity('org-alice@example.org')],
getClientForAccount: (id: string) => (id === 'acc-dave' ? daveClient : undefined),
} as never);
const dave = await externalMailContextFor('acc-dave');
expect(dave.policy.senderLabel).toBe(false);
expect(dave.identityEmails).toEqual(['org-dave@org-two.test']);
// The active account, and an unknown one, read the stores.
expect((await externalMailContextFor('acc-alice')).identityEmails).toEqual(['org-alice@example.org']);
expect((await externalMailContextFor(undefined)).policy.senderLabel).toBe(true);
});
it('collects the addresses of every contact', () => {
+60 -13
View File
@@ -10,8 +10,8 @@
* 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.
* The pure functions take everything as arguments; the context helpers
* read one signed-in account's effective policy and identities.
*/
import { DEFAULT_ORGANISATION_POLICY, normalizeDomainList, type OrganisationPolicy } from '@/lib/admin/organisation-policy';
@@ -19,6 +19,9 @@ import { toAsciiDomain, toAsciiEmail } from '@/lib/idn';
import { usePolicyStore } from '@/stores/policy-store';
import { useAuthStore } from '@/stores/auth-store';
import { TRUSTED_SENDERS_BOOK_NAME, useContactStore } from '@/stores/contact-store';
import { policySubjectForClient } from '@/lib/admin/policy-subject';
import type { PolicySubject } from '@/lib/admin/policy-scopes';
import { cachedIdentitiesForAccount } from '@/hooks/use-pro-multi-account-identities';
export type ExternalMailPolicy = OrganisationPolicy['externalMail'];
@@ -128,22 +131,66 @@ export function externalSenders(
return out;
}
// ─── The active account ──────────────────────────────────────
// ─── Per account ─────────────────────────────────────────────
/** The externalMail settings of the active account's effective policy. */
export function currentExternalMailPolicy(): ExternalMailPolicy {
return usePolicyStore.getState().getOrganisation().externalMail ?? DEFAULT_ORGANISATION_POLICY.externalMail;
/** What decides who is external for one signed-in account. */
export interface ExternalMailContext {
/** That account's effective externalMail policy (its domain and group scopes applied). */
policy: ExternalMailPolicy;
/** That account's identities' addresses. */
identityEmails: string[];
}
/** Either core switch is on: the label or the recipient warning. */
export function coreExternalWarningsOn(policy: ExternalMailPolicy = currentExternalMailPolicy()): boolean {
return policy.senderLabel || policy.recipientWarning;
/** The active account: the effective policy and the identities in the stores. */
export function activeExternalMailContext(): ExternalMailContext {
return {
policy: usePolicyStore.getState().getOrganisation().externalMail ?? DEFAULT_ORGANISATION_POLICY.externalMail,
identityEmails: (useAuthStore.getState().identities ?? []).map((identity) => identity.email),
};
}
/** The internal domains for the active account. */
export function currentInternalDomains(policy: ExternalMailPolicy = currentExternalMailPolicy()): string[] {
const identities = useAuthStore.getState().identities ?? [];
return internalDomains(identities.map((identity) => identity.email), policy.domains);
/** The internal domains of a context. */
export function contextInternalDomains(context: ExternalMailContext): string[] {
return internalDomains(context.identityEmails, context.policy.domains);
}
type AccountClient = NonNullable<ReturnType<ReturnType<typeof useAuthStore.getState>['getClientForAccount']>>;
const subjects = new WeakMap<AccountClient, Promise<PolicySubject | null>>();
const identityLists = new WeakMap<AccountClient, Promise<string[]>>();
/** Whether `localAccountId` names another signed-in account than the active one. */
export function isOtherAccount(localAccountId: string | null | undefined): localAccountId is string {
const auth = useAuthStore.getState();
return !!localAccountId && localAccountId !== auth.activeAccountId && !!auth.getClientForAccount(localAccountId);
}
/**
* The context of a signed-in account by its login id (AccountEntry id): a
* message's own account in the unified inbox, or the account a message is
* sent from. Its policy is resolved for its own subject, so a domain or
* group scope of that account applies, and its own identities count. The
* active account (or none given) reads the stores directly.
*/
export async function externalMailContextFor(localAccountId?: string | null): Promise<ExternalMailContext> {
if (!isOtherAccount(localAccountId)) return activeExternalMailContext();
const client = useAuthStore.getState().getClientForAccount(localAccountId)!;
let subject = subjects.get(client);
if (!subject) {
subject = policySubjectForClient(client).catch(() => null);
subjects.set(client, subject);
}
let identities = identityLists.get(client);
if (!identities) {
const cached = cachedIdentitiesForAccount(localAccountId);
identities = cached
? Promise.resolve(cached.map((identity) => identity.email))
: client.getIdentities().then((list) => list.map((identity) => identity.email)).catch(() => []);
identityLists.set(client, identities);
}
const [resolvedSubject, identityEmails] = await Promise.all([subject, identities]);
const policy = usePolicyStore.getState().resolveFor(resolvedSubject).organisation?.externalMail
?? DEFAULT_ORGANISATION_POLICY.externalMail;
return { policy, identityEmails };
}
/**
@@ -183,3 +183,42 @@ describe('contacts that are not loaded yet', () => {
expect((await result).map((r) => r.address)).toEqual(['org-dave@org-two.test']);
});
});
describe('sending through another signed-in account', () => {
function signInDave(daveScope: Record<string, unknown> | null) {
usePolicyStore.setState({
rawPolicy: {
...DEFAULT_POLICY,
organisation: { ...DEFAULT_ORGANISATION_POLICY, externalMail: { ...DEFAULT_ORGANISATION_POLICY.externalMail, recipientWarning: true } },
scopes: daveScope ? [{ id: 'two', label: 'Two', match: { domains: ['org-two.test'] }, policy: { organisation: { externalMail: daveScope } } }] : [],
},
});
const daveClient = {
getUsername: () => 'org-dave@org-two.test',
getSharedAccounts: () => [],
getAccountInfo: () => null,
getPrincipalTypes: async () => ({}),
getIdentities: async () => [{ id: 'd1', name: 'Dave', email: 'org-dave@org-two.test' }],
};
useAuthStore.setState({
activeAccountId: 'acc-alice',
getClientForAccount: (id: string) => (id === 'acc-dave' ? daveClient : undefined),
} as never);
}
const asDave = (to: string[]): OutgoingContext => ({
...context(to), fromEmail: 'org-dave@org-two.test', identityEmail: 'org-dave@org-two.test', localAccountId: 'acc-dave',
});
it("uses that account's identities: its domain is internal, the active account's is not", async () => {
signInDave(null);
expect(await recipientsToConfirm(asDave(['colleague@org-two.test']))).toEqual([]);
expect((await recipientsToConfirm(asDave(['org-bob@example.org']))).map((r) => r.address)).toEqual(['org-bob@example.org']);
});
it("uses that account's policy: a scope that turns the warning off for it applies", async () => {
signInDave({ recipientWarning: false });
expect(await recipientsToConfirm(asDave(['someone@elsewhere.example']))).toEqual([]);
// The active account still asks.
expect((await recipientsToConfirm(context(['someone@elsewhere.example']))).map((r) => r.address)).toEqual(['someone@elsewhere.example']);
});
});
+7 -4
View File
@@ -9,8 +9,8 @@
import {
addressDomain,
contactAddresses,
currentExternalMailPolicy,
currentInternalDomains,
contextInternalDomains,
externalMailContextFor,
externalRecipients,
type ExternalRecipient,
} from '@/lib/external-mail';
@@ -51,12 +51,15 @@ export async function ensureContactsLoaded(waitMs = CONTACTS_WAIT_MS): Promise<v
/** The recipients the user is asked about; empty when the check doesn't apply. */
export async function recipientsToConfirm(context: OutgoingContext): Promise<ExternalRecipient[]> {
const policy = currentExternalMailPolicy();
// The sending account's policy and identities: an identity of another
// signed-in account sends under that account's domain or group scopes.
const account = await externalMailContextFor(context.localAccountId);
const { policy } = account;
if (!policy.recipientWarning) return [];
// The chosen identity's own address is internal. A From typed over it in
// the composer is not: anyone can type any domain there.
const identityDomain = addressDomain(context.identityEmail);
const internal = currentInternalDomains(policy);
const internal = contextInternalDomains(account);
if (identityDomain && !internal.includes(identityDomain)) internal.push(identityDomain);
let trusted: ((address: string) => boolean) | undefined;
if (policy.trustContacts) {
+6
View File
@@ -27,6 +27,12 @@ export interface OutgoingContext {
identityId?: string;
/** The chosen identity's own address (never a typed From override). */
identityEmail?: string;
/**
* The signed-in account (AccountEntry id) the message goes out through,
* when it isn't the active one: an identity of another account picked in
* the composer. Checks use that account's policy and identities.
*/
localAccountId?: string;
/** compose, reply, replyAll or forward; quick reply is a reply. */
mode: 'compose' | 'reply' | 'replyAll' | 'forward';
/** Plain-text-only message (no HTML part). */