diff --git a/src/lib/__tests__/reply-identity.test.ts b/src/lib/__tests__/reply-identity.test.ts index 981890fa..fae0528d 100644 --- a/src/lib/__tests__/reply-identity.test.ts +++ b/src/lib/__tests__/reply-identity.test.ts @@ -1,5 +1,7 @@ import { describe, it, expect } from 'vitest'; -import { findReplyIdentityId, findDraftIdentityId, resolveReplyFrom, findComposeIdentityId } from '../reply-identity'; +import { + findReplyIdentityId, findDraftIdentityId, resolveReplyFrom, findComposeIdentityId, resolveReplyIdentity, +} from '../reply-identity'; import type { Identity } from '../../api/types'; const identities: Identity[] = [ @@ -21,6 +23,49 @@ describe('findReplyIdentityId', () => { expect(findReplyIdentityId(identities, { to: [{ email: 'x@other.com' }] })).toBeNull(); expect(findReplyIdentityId([], { to: [{ email: 'me@example.com' }] })).toBeNull(); }); + + // The shared-mailbox shape: the team address in To, the member's own + // address in Cc. Scanning identities rather than recipients would answer + // with whichever identity comes first. + const shared: Identity[] = [ + { id: 'owner', name: 'Owner', email: 'owner@example.com', mayDelete: false }, + { id: 'team', name: 'Team', email: 'team@example.com', mayDelete: false }, + ]; + + it('prefers a To recipient over a Cc one, whatever order the identities are in', () => { + expect(findReplyIdentityId(shared, { + to: [{ email: 'team@example.com' }], + cc: [{ email: 'owner@example.com' }], + })).toBe('team'); + }); + + it('ranks a Bcc identity below the address in To', () => { + const withArchive: Identity[] = [ + { id: 'archive', name: 'Archive', email: 'archive@example.com', mayDelete: false }, + { id: 'team', name: 'Team', email: 'team@example.com', mayDelete: false }, + ]; + expect(findReplyIdentityId(withArchive, { + to: [{ email: 'team@example.com' }], + bcc: [{ email: 'archive@example.com' }], + })).toBe('team'); + }); + + it('takes an exact match anywhere over a sub-address match in To', () => { + expect(findReplyIdentityId(identities, { + to: [{ email: 'me+news@example.com' }], + cc: [{ email: 'info@example.com' }], + })).toBe('info'); + }); + + // A `+tag` identifies who the address was given to, so a delivery to an + // unknown tag must not answer with a sibling's tag and disclose it. + it('prefers the untagged identity over a differently-tagged sibling', () => { + const tagged: Identity[] = [ + { id: 'eu', name: 'Sales EU', email: 'sales+eu@example.com', mayDelete: false }, + { id: 'sales', name: 'Sales', email: 'sales@example.com', mayDelete: false }, + ]; + expect(findReplyIdentityId(tagged, { to: [{ email: 'sales+us@example.com' }] })).toBe('sales'); + }); }); describe('findComposeIdentityId', () => { @@ -54,3 +99,34 @@ describe('resolveReplyFrom', () => { expect(resolveReplyFrom(identities, { to: [{ email: 'x@other.com' }] })).toBeNull(); }); }); + +describe('resolveReplyIdentity', () => { + const ownEmails = ['me@example.com', 'info@example.com']; + + it('sends from the own identity the message was delivered to, without the setting', () => { + expect(resolveReplyIdentity(identities, { + from: { email: 'alice@other.com' }, + to: [{ email: 'info@example.com' }], + }, { ownEmails, catchAll: false })).toEqual({ identityId: 'info' }); + }); + + it('never rewrites From unless the catch-all is asked for', () => { + const original = { from: { email: 'alice@other.com' }, to: [{ email: 'sales@example.com', name: 'Sales' }] }; + expect(resolveReplyIdentity(identities, original, { ownEmails, catchAll: false })).toBeNull(); + expect(resolveReplyIdentity(identities, original, { ownEmails, catchAll: true })).toEqual({ + identityId: 'main', overrideEmail: 'sales@example.com', overrideName: 'Sales', + }); + }); + + it('replies to our own message from the identity that sent it', () => { + expect(resolveReplyIdentity(identities, { + from: { email: 'me@example.com', name: 'Alias' }, + to: [{ email: 'bob@example.com' }], + }, { ownEmails, catchAll: true })).toEqual({ identityId: 'alias' }); + }); + + it('returns null without identities or a match', () => { + expect(resolveReplyIdentity([], { to: [{ email: 'info@example.com' }] }, { ownEmails, catchAll: true })).toBeNull(); + expect(resolveReplyIdentity(identities, { to: [{ email: 'x@other.com' }] }, { ownEmails, catchAll: true })).toBeNull(); + }); +}); diff --git a/src/lib/reply-identity.ts b/src/lib/reply-identity.ts index 20c68f0b..0e25dbea 100644 --- a/src/lib/reply-identity.ts +++ b/src/lib/reply-identity.ts @@ -2,6 +2,7 @@ // webmail's lib/reply-identity.ts. import type { Identity } from '../api/types'; +import { isSelfSent } from './reply-recipients'; interface ReplyRecipient { email?: string | null; @@ -39,9 +40,14 @@ function domainOf(email: string): string { } /** - * The identity that received the original message: exact address match over - * to/cc/bcc first, then with `+tag` sub-addressing stripped. Null when none - * matches (caller keeps the default identity). + * Pick the identity a message was delivered to, among the user's own. Never + * rewrites `From:` - it only chooses which configured address sends. Null + * when none matches (caller keeps the default identity). + * + * Recipients are scanned in To, then Cc, then Bcc order, exact matches before + * `+tag`-stripped ones. Scanning the identities instead would let the identity + * list's order decide, so `To: team@, Cc: you@` would reply as you rather + * than as the team - the exact case this is meant to fix. */ export function findReplyIdentityId( identities: Identity[], @@ -63,16 +69,38 @@ export function findReplyIdentityId( return null; } - const exactMatches = new Set(receivedAddresses.map(normalizeEmailAddress)); - const exactIdentity = identities.find((identity) => exactMatches.has(normalizeEmailAddress(identity.email))); - if (exactIdentity) { - return exactIdentity.id; + const byExact = new Map(); + const byBase = new Map(); + for (const identity of identities) { + const exact = normalizeEmailAddress(identity.email); + const base = normalizeBaseEmailAddress(identity.email); + if (!byExact.has(exact)) { + byExact.set(exact, identity.id); + } + // For a `+tag` delivery with no exact identity, the untagged identity is the + // answer; a differently-tagged sibling would disclose an unrelated tag. + const untagged = exact === base; + const existing = byBase.get(base); + if (!existing || (untagged && !existing.untagged)) { + byBase.set(base, { id: identity.id, untagged }); + } } - const baseMatches = new Set(receivedAddresses.map(normalizeBaseEmailAddress)); - const baseIdentity = identities.find((identity) => baseMatches.has(normalizeBaseEmailAddress(identity.email))); + for (const address of receivedAddresses) { + const exactId = byExact.get(normalizeEmailAddress(address)); + if (exactId) { + return exactId; + } + } - return baseIdentity?.id ?? null; + for (const address of receivedAddresses) { + const baseMatch = byBase.get(normalizeBaseEmailAddress(address)); + if (baseMatch) { + return baseMatch.id; + } + } + + return null; } /** @@ -227,3 +255,42 @@ export function resolveReplyFrom( return null; } + +/** + * Identity (and optional From override) for a reply or forward: + * 1. Replying to our own message in a thread (#703): the identity that sent + * it. The recipients are the other party's, and on a catch-all domain + * they would even become a From override. + * 2. The own identity the original was delivered to (`findReplyIdentityId`). + * Unconditional: it only ever picks a configured address. + * 3. With `catchAll`, the same-domain catch-all From override of + * `resolveReplyFrom`. Callers pass it only for an opted-in reply, never + * a forward: a reply continues a thread whose participants know the + * addressing, a forward shows the rewritten From to someone new. + * Null when nothing matches (the caller keeps its default identity). + */ +export function resolveReplyIdentity( + identities: Identity[], + original: ReplyRecipients & { from?: ReplyRecipient | null }, + opts: { ownEmails: string[]; catchAll: boolean }, +): ReplyFromResolution | null { + if (identities.length === 0) { + return null; + } + + const senderEmail = original.from?.email?.trim(); + if (senderEmail && isSelfSent({ from: [{ email: senderEmail }] }, opts.ownEmails)) { + const senderId = findDraftIdentityId(identities, original.from); + if (senderId) { + return { identityId: senderId }; + } + } + + const recipients = { to: original.to, cc: original.cc, bcc: original.bcc }; + const ownId = findReplyIdentityId(identities, recipients); + if (ownId) { + return { identityId: ownId }; + } + + return opts.catchAll ? resolveReplyFrom(identities, recipients) : null; +} diff --git a/src/screens/ComposeScreen.tsx b/src/screens/ComposeScreen.tsx index 13d0ec65..20b26d7f 100644 --- a/src/screens/ComposeScreen.tsx +++ b/src/screens/ComposeScreen.tsx @@ -54,7 +54,7 @@ import { buildQuoteHeader, formatQuoteDate, type QuoteHeaderLabels } from '../li import { isValidEmail, splitPastedRecipients, expandRecipients, parseRecipient, type Recipient as ParsedRecipient, } from '../lib/recipients'; -import { findDraftIdentityId, resolveReplyFrom } from '../lib/reply-identity'; +import { findDraftIdentityId, resolveReplyIdentity } from '../lib/reply-identity'; import { shouldBlockEditorRemoteImages } from '../lib/editor-html'; import { hasSignature, buildEmbeddedSignatureHtml, containsEmbeddedSignature, spliceSignature, @@ -835,10 +835,11 @@ export default function ComposeScreen({ route, navigation }: Props) { // Choose the identity once we know both the loaded identities and the // compose context: a re-opened draft keeps the identity it was written - // with; with auto-select on, a reply picks the identity that received the - // original (or a catch-all alias on an owned domain as a From override); - // otherwise the preferred ("Use as default") identity, else the one - // matching the active account. + // with; a reply or forward sends from the own identity the original was + // delivered to (a reply to our own message, from the one that sent it); + // with auto-select on, a reply to a catch-all alias on an owned domain + // takes that address as a From override; otherwise the preferred ("Use as + // default") identity, else the one matching the active account. React.useEffect(() => { if (selectedIdentityId || identities.length === 0) return; const activeEmail = useAccountStore.getState().getActiveAccount()?.email || jmapClient.username; @@ -854,8 +855,12 @@ export default function ComposeScreen({ route, navigation }: Props) { setSelectedIdentityId(matched ?? defaultIdentity.id); return; } - if (autoSelectReplyIdentity && replyTo) { - const resolved = resolveReplyFrom(identities, { to: replyTo.to, cc: replyTo.cc, bcc: replyTo.bcc }); + if (replyTo) { + // The catch-all From rewrite is opt-in and never used on a forward. + const resolved = resolveReplyIdentity(identities, replyTo, { + ownEmails, + catchAll: autoSelectReplyIdentity && mode !== 'forward', + }); if (resolved) { setSelectedIdentityId(resolved.identityId); if (resolved.overrideEmail) { @@ -865,6 +870,7 @@ export default function ComposeScreen({ route, navigation }: Props) { } } setSelectedIdentityId(defaultIdentity.id); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [identities, autoSelectReplyIdentity, replyTo, draft, selectedIdentityId, preferredIdentityIds]); const primaryIdentity = React.useMemo(() => {