Merge pull request #19865 from mozilla/double-match-on-bounce-check

chore(bounces): Use a normalized and aliased email to search bounces

Author: nshirley

packages/fxa-auth-server/config/index.ts

@@ -614,12 +614,18 @@ const convictConf = convict({
       },
       emailAliasNormalization: {
         default: JSON.stringify([
-          { domain: 'mozilla.com', regex: '\\+.*', replace: '%' },
+          { domain: 'mozilla.com', regex: '\\+.*', replace: '' },
         ]),
-        doc: 'List of email domain configurations for alias normalization. Each entry should have domain, regex, and replace properties. Example: [{domain: "mozilla.com", regex: "\\+[^@]+" }].',
+        doc: 'List of email domain configurations for alias normalization. Each entry should have domain, regex, and replace properties. The replace value is used for the root email (strip alias), and can be overridden with a wildcard pattern at runtime. Example: [{domain: "mozilla.com", regex: "\\+.*", replace: "" }].',
         env: 'BOUNCES_EMAIL_ALIAS_NORMALIZATION',
         format: String,
       },
+      aliasCheckEnabled: {
+        doc: 'Flag to enable checking email bounces on normalized email aliases',
+        format: Boolean,
+        default: false,
+        env: 'BOUNCES_ALIAS_CHECK_ENABLED',
+      },
     },
     connectionTimeout: {
       doc: 'Milliseconds to wait for the connection to establish (default is 2 minutes)',

packages/fxa-auth-server/lib/bounces.js

@@ -11,6 +11,7 @@ module.exports = (config, db) => {
   const configBounces = (config.smtp && config.smtp.bounces) || {};
   const ignoreTemplates = configBounces.ignoreTemplates || [];
   const BOUNCES_ENABLED = !!configBounces.enabled;
+  const BOUNCES_ALIAS_CHECK_ENABLED = !!configBounces.aliasCheckEnabled;
 
   const BOUNCE_TYPE_HARD = 1;
   const BOUNCE_TYPE_SOFT = 2;
@@ -38,17 +39,58 @@ module.exports = (config, db) => {
       return;
     }
 
-    // This strips out 'alias' stuff from an email and replace
-    // them with wildcards, allowing us to turn up bounce records
-    // on email aliases.
-    const normalizedEmail = emailNormalization.normalizeEmailAliases(
-      email,
-      '%'
-    );
-    const bounces = await db.emailBounces(normalizedEmail);
+    let bounces;
+
+    if (BOUNCES_ALIAS_CHECK_ENABLED) {
+      bounces = await checkBouncesWithAliases(email);
+    } else {
+      bounces = await db.emailBounces(email);
+    }
+
     return applyRules(bounces);
   }
 
+  async function checkBouncesWithAliases(email) {
+    // Given an email alias like test+123@domain.com:
+    // We look for bounces to the 'root' email -> `test@domain.com`
+    // And look for bounces to the alias with a wildcard -> `test+%@domain.com`
+    //
+    // This prevents us from picking up false positives when we replace the alias
+    // with a wildcard, and doesn't miss the root email bounces either. We have to
+    // use both because just using the wildcard would miss bounces sent to the root
+    // and just using the root with a wildcard would pickup false positives.
+    //
+    // So, test+123@domain.com would match:
+    //   - test@domain.com            Covered by normalized email
+    //   - test+123@domain.com        Covered by wildcard email
+    //   - test+asdf@domain.com       Covered by wildcard email
+    // but not
+    //   - testing@domain.com         Not picked up by wildcard since we include the '+'
+    const normalizedEmail = emailNormalization.normalizeEmailAliases(email, '');
+    const wildcardEmail = emailNormalization.normalizeEmailAliases(email, '+%');
+
+    const [normalizedBounces, wildcardBounces] = await Promise.all([
+      db.emailBounces(normalizedEmail),
+      db.emailBounces(wildcardEmail),
+    ]);
+
+    // Merge and deduplicate by email+createdAt
+    // there shouldn't be any overlap, but just in case
+    const seen = new Set();
+    const merged = [...normalizedBounces, ...wildcardBounces].filter(
+      (bounce) => {
+        const key = `${bounce.email}:${bounce.createdAt}`;
+        if (seen.has(key)) {
+          return false;
+        }
+        seen.add(key);
+        return true;
+      }
+    );
+
+    return merged.sort((a, b) => b.createdAt - a.createdAt);
+  }
+
   // Relies on the order of the bounces array to be sorted by date,
   // descending. So, each bounce in the array must be older than the
   // previous.

packages/fxa-shared/email/email-normalization.ts

@@ -33,7 +33,14 @@ export class EmailNormalization {
     }
   }
 
-  normalizeEmailAliases(email: string): string {
+  /**
+   * Normalizes email aliases by applying configured regex replacements.
+   * Optionally, a replacement string can be overridden.
+   * @param email The email address to normalize.
+   * @param replaceOverride Optional string to replace matched aliases.
+   * @returns The normalized email address.
+   */
+  normalizeEmailAliases(email: string, replaceOverride?: string): string {
     email = email?.trim()?.toLocaleLowerCase() || '';
     const parts = email.split('@');
     if (parts?.length !== 2) {
@@ -48,7 +55,7 @@ export class EmailNormalization {
     this.emailTransforms
       .filter((x) => x.domain === domain)
       .forEach((x) => {
-        email = email.replace(x.regex, x.replace);
+        email = email.replace(x.regex, replaceOverride || x.replace);
       });
 
     return `${email}@${domain}`;