mirror of
https://github.com/bulwarkmail/native.git
synced 2026-11-01 08:06:13 +00:00
fix: reply and forward from the address the message was delivered to
Replies went out from the default identity unless "Reply from received address" was on, because that setting gated both the match against the user's own identities and the catch-all From rewrite. Forwards took the rewrite too, and a reply to one's own sent message resolved the identity from the other party's address. Ported from the webmail (4fc7b8b5): - the own-identity match is unconditional and applies to forwards; only the catch-all From rewrite stays behind the setting, and never on a forward - a reply to our own message keeps the identity that sent it (#703) - `findReplyIdentityId` scans recipients in To, Cc, Bcc order instead of the identity list's, and prefers an untagged identity over a differently tagged sibling
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
+77
-10
@@ -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<string, string>();
|
||||
const byBase = new Map<string, { id: string; untagged: boolean }>();
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
Reference in New Issue
Block a user