diff --git a/app/controllers/api/v1/lookup.js b/app/controllers/api/v1/lookup.js index 83bdfcfa4..1d10eba6d 100644 --- a/app/controllers/api/v1/lookup.js +++ b/app/controllers/api/v1/lookup.js @@ -14,7 +14,7 @@ async function lookup(ctx) { // domain found must be that host or a parent of it) domainName: isSANB(ctx.query.domain) ? ctx.query.domain : undefined, username: isSANB(ctx.query.username) - ? ctx.query.username.toLowerCase() + ? ctx.query.username.normalize('NFC').toLowerCase() : false, ignoreBilling: ctx.query.ignore_billing, client: ctx.client, diff --git a/app/models/aliases.js b/app/models/aliases.js index 65c5eda53..90cdf333b 100644 --- a/app/models/aliases.js +++ b/app/models/aliases.js @@ -755,8 +755,9 @@ Aliases.pre('validate', function (next) { }); } - // trim and convert to lowercase - this.name = this.name.trim().toLowerCase(); + // trim, normalize Unicode (NFC) and convert to lowercase + // (e.g. so "blåtest" matches regardless of composed/decomposed "å") + this.name = this.name.trim().normalize('NFC').toLowerCase(); // require alias name if ( diff --git a/app/models/emails.js b/app/models/emails.js index 7d3670528..297aad863 100644 --- a/app/models/emails.js +++ b/app/models/emails.js @@ -1553,7 +1553,7 @@ Emails.statics.queue = async function ( // rewrite from header to be without "+" symbol // so that users can send with "+" address filtering // - const name = parseUsername(addresses[0]); // converts to ASCII + const name = punycode.toASCII(parseUsername(addresses[0])); // converts to ASCII const domain = parseHostFromDomainOrAddress(addresses[0]); // converts to ASCII from = `${name}@${domain}`; // ASCII formatted From address header } diff --git a/helpers/get-forwarding-addresses.js b/helpers/get-forwarding-addresses.js index ba439c3cf..3ce79bf46 100644 --- a/helpers/get-forwarding-addresses.js +++ b/helpers/get-forwarding-addresses.js @@ -29,6 +29,7 @@ const i18n = require('#helpers/i18n'); const getForwardingConfiguration = require('#helpers/get-forwarding-configuration'); const getKeyInfo = require('#helpers/get-key-info'); const getMaxForwardedAddresses = require('#helpers/get-max-forwarded-addresses'); +const getUsernameVariants = require('#helpers/get-username-variants'); const isEmail = require('#helpers/is-email'); const isExpiredOrNewlyCreated = require('#helpers/is-expired-or-newly-created'); const isRetryableError = require('#helpers/is-retryable-error'); @@ -469,6 +470,12 @@ async function getForwardingAddresses( // (e.g. user@example.com => hello) const username = isEmail(address) ? parseUsername(address) : false; + // Unicode and punycode forms of username to match alias names against + // (e.g. "fællestest" and legacy "xn--fllestest-g3a") + const usernameVariants = username ? getUsernameVariants(username) : []; + const isUsername = (name) => + usernameVariants.includes(name.normalize('NFC').toLowerCase()); + // // store if the domain was bad and not on paid plan (required for bad domains) // @@ -987,7 +994,11 @@ async function getForwardingAddresses( .catch((cacheErr) => logger.fatal(cacheErr)); } - if (username && regex && regex.test(username.toLowerCase())) { + const matchedUsername = regex + ? usernameVariants.find((variant) => regex.test(variant)) + : undefined; + + if (matchedUsername) { // // opt-in subdomain-aware substitution: // replace the literal tokens %SUBDOMAIN% and %HOST% in the target @@ -1011,7 +1022,7 @@ async function getForwardingAddresses( const hasDollarInterpolation = REGEX_INTERPOLATED_DOLLAR.test(target); const substitutedAlias = hasDollarInterpolation - ? username.toLowerCase().replace(regex, target) + ? matchedUsername.replace(regex, target) : target; // RFC 5321 limits a mailbox address to 254 octets. Enforce the bound @@ -1085,25 +1096,19 @@ async function getForwardingAddresses( // check if we have a match (and if it is ignored) if (_.isString(addr[0]) && addr[0].indexOf('!') === 0) { // !!! -> 550 - if ( - addr[0].indexOf('!!!') === 0 && - username === addr[0].toLowerCase().slice(3) - ) { + if (addr[0].indexOf('!!!') === 0 && isUsername(addr[0].slice(3))) { hardRejected = true; break; } // !! -> 421 - if ( - addr[0].indexOf('!!') === 0 && - username === addr[0].toLowerCase().slice(2) - ) { + if (addr[0].indexOf('!!') === 0 && isUsername(addr[0].slice(2))) { softRejected = true; break; } // ! -> 250 - if (username === addr[0].toLowerCase().slice(1)) { + if (isUsername(addr[0].slice(1))) { ignored = true; break; } @@ -1124,7 +1129,7 @@ async function getForwardingAddresses( `${lowerCaseAddress} domain of ${domain} has an invalid "${config.recordPrefix}" TXT record due to an invalid email address of "${element}"` ); - if (_.isString(addr[0]) && username === addr[0].toLowerCase()) { + if (_.isString(addr[0]) && isUsername(addr[0])) { if (isURL(addr[1], config.isURLOptions)) forwardingAddresses.push(addr[1]); else forwardingAddresses.push(addr[1].toLowerCase()); diff --git a/helpers/get-forwarding-configuration.js b/helpers/get-forwarding-configuration.js index deceeefec..5887663d6 100644 --- a/helpers/get-forwarding-configuration.js +++ b/helpers/get-forwarding-configuration.js @@ -19,6 +19,7 @@ const Domains = require('#models/domains'); const Users = require('#models/users'); const config = require('#config'); const email = require('#helpers/email'); +const getUsernameVariants = require('#helpers/get-username-variants'); const i18n = require('#helpers/i18n'); const isEmail = require('#helpers/is-email'); const logger = require('#helpers/logger'); @@ -68,6 +69,11 @@ async function getForwardingConfiguration({ if (domainName !== undefined && !isVerifiedHost(domain.name, domainName)) return {}; + // Unicode and punycode forms of username to match alias names against + // (e.g. "fællestest" and legacy "xn--fllestest-g3a") + const usernameVariants = username ? getUsernameVariants(username) : []; + if (username) username = usernameVariants[0]; + let hasMultiplePGP = false; let hasMultipleSMIME = false; @@ -128,26 +134,21 @@ async function getForwardingConfiguration({ // // NOTE: we are doing it this way for backwards compatibility for now - // (in the future we should probably just to `toUnicode` some users already converted so this supports legacy) - // (e.g. some users have aliases starting with "xn--" right now which means they already realized this bug) + // (e.g. some users have aliases starting with "xn--" right now + // since the local part was previously converted to punycode) // // if there were no aliases found but a `username` was passed - // then we can attempt to convert ASCII to Unicode and perform a lookup - if ( - aliases.length === 0 && - username && - punycode.toUnicode(username) !== username - ) { + // then we can attempt a lookup with its other Unicode/punycode forms + if (aliases.length === 0 && usernameVariants.length > 1) { aliases = await Aliases.find({ domain: domain._id, - name: punycode.toUnicode(username) + name: { $in: usernameVariants.slice(1) } }) .select( 'id user has_imap has_pgp public_key has_smime smime_certificate recipients name is_enabled error_code_if_disabled has_recipient_verification verified_recipients vacation_responder' ) .lean() .exec(); - if (aliases.length > 0) username = punycode.toUnicode(username); } if (aliases.length === 0) return {}; @@ -444,8 +445,7 @@ async function getForwardingConfiguration({ for (const recipient of alias.recipients) { if (!isEmail(recipient)) continue; const [rcptName, rcptDomain] = recipient.split('@'); - if (rcptName !== username && rcptName !== punycode.toUnicode(username)) - continue; + if (!usernameVariants.includes(rcptName)) continue; if (rcptDomain !== domain.name) continue; const id = aliasIdsWithIMAPByName[rcptName]; if (!id) continue; @@ -613,10 +613,7 @@ async function getForwardingConfiguration({ } if (regex) { - if ( - regex.test(username) || - regex.test(punycode.toUnicode(username)) - ) { + if (usernameVariants.some((variant) => regex.test(variant))) { pushToBody(alias); } @@ -625,8 +622,7 @@ async function getForwardingConfiguration({ } } - if (username !== alias.name && punycode.toUnicode(username) !== alias.name) - continue; + if (!usernameVariants.includes(alias.name)) continue; pushToBody(alias); } diff --git a/helpers/get-username-variants.js b/helpers/get-username-variants.js new file mode 100644 index 000000000..087766826 --- /dev/null +++ b/helpers/get-username-variants.js @@ -0,0 +1,33 @@ +/** + * Copyright (c) Forward Email LLC + * SPDX-License-Identifier: BUSL-1.1 + */ + +const punycode = require('node:punycode'); + +// +// returns every form a username can be matched against in alias names +// and regular expressions (first entry is always the NFC Unicode form) +// +// e.g. "fællestest" -> [ "fællestest", "xn--fllestest-g3a" ] +// "xn--fllestest-g3a" -> [ "xn--fllestest-g3a", "fællestest" ] +// +// NOTE: aliases were previously matched against the punycode form of the +// local part, so some users created aliases starting with "xn--" +// which we still need to support for backwards compatibility +// +function getUsernameVariants(username) { + const variants = [username.normalize('NFC').toLowerCase()]; + for (const fn of [punycode.toASCII, punycode.toUnicode]) { + try { + const variant = fn(variants[0]).normalize('NFC').toLowerCase(); + if (!variants.includes(variant)) variants.push(variant); + } catch { + // ignore punycode conversion errors + } + } + + return variants; +} + +module.exports = getUsernameVariants; diff --git a/helpers/parse-username.js b/helpers/parse-username.js index eabc3edf6..04b6d454b 100644 --- a/helpers/parse-username.js +++ b/helpers/parse-username.js @@ -3,19 +3,22 @@ * SPDX-License-Identifier: BUSL-1.1 */ -const punycode = require('node:punycode'); - const parseAddresses = require('#helpers/parse-addresses'); +// +// NOTE: the local part is returned as Unicode (NFC normalized and lowercase) +// punycode is only defined for domain names, so a local part such as +// "fællestest" is not rewritten to "xn--fllestest-g3a" anymore +// (see `#helpers/get-username-variants` for matching legacy aliases) +// function parseUsername(address, ignorePlus = false) { address = parseAddresses(address)[0]; - let username = + const username = !ignorePlus && address.includes('+') ? address.split('+')[0] : address.split('@')[0]; - username = punycode.toASCII(username).toLowerCase(); - return username; + return username.normalize('NFC').toLowerCase(); } module.exports = parseUsername; diff --git a/test/unicode-alias-forwarding.js b/test/unicode-alias-forwarding.js new file mode 100644 index 000000000..fd55f0a29 --- /dev/null +++ b/test/unicode-alias-forwarding.js @@ -0,0 +1,198 @@ +/** + * Copyright (c) Forward Email LLC + * SPDX-License-Identifier: BUSL-1.1 + */ + +// +// Tests for aliases with a Unicode (non-ASCII) local part, e.g. +// "fællestest@example.com". +// +// The local part used to be converted to punycode ("xn--fllestest-g3a") +// before alias names and regular expressions were matched against it, so +// e.g. `/^.*llestest$/` never matched and the catch-all was used instead. +// +// The tests stub the DNS resolver (`this.resolver`) and Redis client +// (`this.client`) and override `#helpers/get-forwarding-configuration` in the +// require cache (the same approach as `test/wildcard-regex-forwarding.js`). +// + +const test = require('ava'); +const Redis = require('ioredis-mock'); + +// creates the mongoose connections the models require +// (no database is queried by these tests) +// eslint-disable-next-line import/no-unassigned-import +require('./utils'); + +// +// install the paid-plan mock BEFORE requiring the helper under test +// +const GFC_PATH = require.resolve('#helpers/get-forwarding-configuration'); + +let paidPlanHandler = async () => ({}); + +require.cache[GFC_PATH] = { + id: GFC_PATH, + filename: GFC_PATH, + loaded: true, + exports: (...args) => paidPlanHandler(...args) +}; + +const getForwardingAddresses = require('#helpers/get-forwarding-addresses'); +const getUsernameVariants = require('#helpers/get-username-variants'); +const parseUsername = require('#helpers/parse-username'); + +const ROOT = 'example.com'; + +// "fællestest" with "æ" (which has no decomposed form) +const FAELLES = 'fællestest'; +// "blåtest" composed (NFC) and decomposed (NFD, "a" + U+030A) +const BLA_NFC = 'blåtest'.normalize('NFC'); +const BLA_NFD = 'blåtest'.normalize('NFD'); + +function makeContext(txtRecords) { + return { + client: new Redis(), + resolver: { + async resolveTxt(host) { + if (host === ROOT) return txtRecords.map((record) => [record]); + const err = new Error(`queryTxt ENODATA ${host}`); + err.code = 'ENODATA'; + throw err; + }, + async resolveMx(host) { + if (host === ROOT) + return [ + { exchange: 'mx1.forwardemail.net', priority: 10 }, + { exchange: 'mx2.forwardemail.net', priority: 10 } + ]; + return []; + } + } + }; +} + +function run(ctx, address) { + return getForwardingAddresses.call(ctx, address, [], true, { + originalFromAddressRootDomain: 'example.net' + }); +} + +test.beforeEach(() => { + paidPlanHandler = async () => ({}); +}); + +// +// helpers +// +test('parseUsername keeps Unicode local part (NFC, lowercase)', (t) => { + t.is(parseUsername(`${FAELLES}@${ROOT}`), FAELLES); + t.is(parseUsername(`FÆLLESTEST@${ROOT}`), FAELLES); + t.is(parseUsername(`${BLA_NFD}@${ROOT}`), BLA_NFC); + t.is(parseUsername(`${FAELLES}+tag@${ROOT}`), FAELLES); + t.is(parseUsername(`${FAELLES}+tag@${ROOT}`, true), `${FAELLES}+tag`); + t.is(parseUsername(`Hello@${ROOT}`), 'hello'); +}); + +test('getUsernameVariants returns Unicode and punycode forms', (t) => { + t.deepEqual(getUsernameVariants(FAELLES), [FAELLES, 'xn--fllestest-g3a']); + t.deepEqual(getUsernameVariants('xn--fllestest-g3a'), [ + 'xn--fllestest-g3a', + FAELLES + ]); + t.deepEqual(getUsernameVariants(BLA_NFD), [BLA_NFC, 'xn--bltest-jua']); + t.deepEqual(getUsernameVariants('hello'), ['hello']); +}); + +// +// free plan (TXT records) +// +test('regex matches Unicode local part instead of catch-all', async (t) => { + const ctx = makeContext([ + 'forward-email=/^.*llestest$/:regex@example.net,catchall@example.net' + ]); + const { addresses } = await run(ctx, `${FAELLES}@${ROOT}`); + t.deepEqual(addresses, ['regex@example.net']); +}); + +test('regex matches decomposed (NFD) recipient', async (t) => { + const ctx = makeContext([ + `forward-email=/^${BLA_NFC}$/:regex@example.net,catchall@example.net` + ]); + const { addresses } = await run(ctx, `${BLA_NFD}@${ROOT}`); + t.deepEqual(addresses, ['regex@example.net']); +}); + +test('regex $1 substitution keeps Unicode', async (t) => { + const ctx = makeContext(['forward-email=/^(.*)$/:user+$1@example.net']); + const { addresses } = await run(ctx, `${FAELLES}@${ROOT}`); + t.deepEqual(addresses, [`user+${FAELLES}@example.net`]); +}); + +test('exact Unicode alias name matches', async (t) => { + const ctx = makeContext([ + `forward-email=${FAELLES}:exact@example.net,catchall@example.net` + ]); + const { addresses } = await run(ctx, `${FAELLES}@${ROOT}`); + t.deepEqual(addresses, ['exact@example.net']); +}); + +test('exact alias name matches regardless of NFC/NFD', async (t) => { + const ctx = makeContext([ + `forward-email=${BLA_NFD}:exact@example.net,catchall@example.net` + ]); + const { addresses } = await run(ctx, `${BLA_NFC}@${ROOT}`); + t.deepEqual(addresses, ['exact@example.net']); +}); + +test('legacy punycode alias name still matches', async (t) => { + const ctx = makeContext([ + 'forward-email=xn--fllestest-g3a:legacy@example.net,catchall@example.net' + ]); + const { addresses } = await run(ctx, `${FAELLES}@${ROOT}`); + t.deepEqual(addresses, ['legacy@example.net']); +}); + +test('ignored Unicode alias name is ignored', async (t) => { + const ctx = makeContext([`forward-email=!${FAELLES},catchall@example.net`]); + const result = await run(ctx, `${FAELLES}@${ROOT}`); + t.true(result.ignored); +}); + +test('regression: ASCII regex and catch-all unchanged', async (t) => { + const ctx = makeContext([ + 'forward-email=/^(support|info)$/:user+$1@example.net,catchall@example.net' + ]); + const support = await run(ctx, `support@${ROOT}`); + t.deepEqual(support.addresses, ['user+support@example.net']); + const other = await run(ctx, `other@${ROOT}`); + t.deepEqual(other.addresses, ['catchall@example.net']); +}); + +// +// paid plan (mapping from get-forwarding-configuration) +// +test('paid plan: regex mapping matches Unicode local part', async (t) => { + const ctx = makeContext(['forward-email-site-verification=abc123']); + let lookedUpUsername; + paidPlanHandler = async ({ username }) => { + lookedUpUsername = username; + return { + mapping: ['/^.*llestest$/:regex@example.net', 'catchall@example.net'] + }; + }; + + const { addresses } = await run(ctx, `${FAELLES}@${ROOT}`); + t.is(lookedUpUsername, FAELLES); + t.deepEqual(addresses, ['regex@example.net']); +}); + +test('paid plan: exact Unicode alias mapping matches', async (t) => { + const ctx = makeContext(['forward-email-site-verification=abc123']); + paidPlanHandler = async () => ({ + mapping: [`${FAELLES}:exact@example.net`, 'catchall@example.net'] + }); + + const { addresses } = await run(ctx, `${FAELLES}@${ROOT}`); + t.deepEqual(addresses, ['exact@example.net']); +});