fix(bounces): Cope with quoted email addresses in bounce notifications.

lib/bounces.js

@@ -2,6 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+var eaddrs = require('email-addresses')
 var P = require('./promise')
 
 module.exports = function (log, error) {
@@ -16,6 +17,23 @@ module.exports = function (log, error) {
       log.error({ op: 'databaseError', email: email, err: err })
     }
 
+    function findEmailRecord(email) {
+      return db.emailRecord(email)
+        .catch(function (err) {
+          // The mail agent may have mangled the email address
+          // that's being reported in the bounce notification.
+          // Try looking up by normalized form as well.
+          if (err.errno !== error.ERRNO.ACCOUNT_UNKNOWN) {
+            throw err
+          }
+          var normalizedEmail = eaddrs.parseOneAddress(email).address
+          if (normalizedEmail === email) {
+            throw err
+          }
+          return db.emailRecord(normalizedEmail)
+        })
+    }
+
     function deleteAccountIfUnverified(record) {
       if (!record.emailVerified) {
         return db.deleteAccount(record)
@@ -39,23 +57,18 @@ module.exports = function (log, error) {
       else if (message.complaint && message.complaint.complaintFeedbackType === 'abuse') {
         recipients = message.complaint.complainedRecipients
       }
-      var p = P.resolve()
-      recipients.forEach(function(recipient) {
+      return P.each(recipients, function (recipient) {
         var email = recipient.emailAddress
-        p = p.then(
-          function () {
-            log.info({ op: 'handleBounce', email: email, bounce: !!message.bounce })
-            log.increment('account.email_bounced')
-            return db.emailRecord(email)
-              .then(
-                deleteAccountIfUnverified,
-                gotError.bind(null, email)
-              )
-          }
-        )
-      })
-      return p.then(
+        log.info({ op: 'handleBounce', email: email, bounce: !!message.bounce })
+        log.increment('account.email_bounced')
+        return findEmailRecord(email)
+          .then(
+            deleteAccountIfUnverified,
+            gotError.bind(null, email)
+          )
+      }).then(
         function () {
+          // We always delete the message, even if handling some addrs failed.
           message.del()
         }
       )

package.json

@@ -30,6 +30,7 @@
     "binary-split": "0.1.2",
     "bluebird": "2.10.2",
     "convict": "1.0.1",
+    "email-addresses": "2.0.2",
     "envc": "2.4.0",
     "fxa-auth-mailer": "git+https://github.com/mozilla/fxa-auth-mailer.git#master",
     "fxa-jwtool": "0.7.1",

test/local/bounce_tests.js

@@ -51,7 +51,7 @@ test(
         return P.resolve({ })
       })
     }
-    return mockedBounces(mockLog, mockDB).handleBounce(mockMessage({
+    var mockMsg = mockMessage({
       bounce: {
         bounceType: 'Permanent',
         bouncedRecipients: [
@@ -59,7 +59,8 @@ test(
           { emailAddress: 'foobar@example.com'}
         ]
       }
-    })).then(function () {
+    })
+    return mockedBounces(mockLog, mockDB).handleBounce(mockMsg).then(function () {
       t.equal(mockDB.emailRecord.callCount, 2)
       t.equal(mockDB.deleteAccount.callCount, 2)
       t.equal(mockDB.emailRecord.args[0][0], 'test@example.com')
@@ -70,6 +71,7 @@ test(
       t.equal(mockLog.messages[5].level, 'info')
       t.equal(mockLog.messages[5].args[0].op, 'accountDeleted')
       t.equal(mockLog.messages[5].args[0].email, 'foobar@example.com')
+      t.equal(mockMsg.del.callCount, 1)
     })
   }
 )
@@ -162,14 +164,15 @@ test(
         return P.reject(new error({}))
       })
     }
-    return mockedBounces(mockLog, mockDB).handleBounce(mockMessage({
+    var mockMsg = mockMessage({
       bounce: {
         bounceType: 'Permanent',
         bouncedRecipients: [
           { emailAddress: 'test@example.com' },
         ]
       }
-    })).then(function () {
+    })
+    return mockedBounces(mockLog, mockDB).handleBounce(mockMsg).then(function () {
       t.equal(mockDB.emailRecord.callCount, 1)
       t.equal(mockDB.emailRecord.args[0][0], 'test@example.com')
       t.equal(mockLog.messages.length, 3)
@@ -178,6 +181,7 @@ test(
       t.equal(mockLog.messages[1].args[0], 'account.email_bounced')
       t.equal(mockLog.messages[2].args[0].op, 'databaseError')
       t.equal(mockLog.messages[2].args[0].email, 'test@example.com')
+      t.equal(mockMsg.del.callCount, 1)
     })
   }
 )
@@ -194,18 +198,19 @@ test(
           emailVerified: false
         })
       }),
-      deleteAccount: sinon.spy(function (email) {
+      deleteAccount: sinon.spy(function (record) {
         return P.reject(new error.unknownAccount('test@example.com'))
       })
     }
-    return mockedBounces(mockLog, mockDB).handleBounce(mockMessage({
+    var mockMsg = mockMessage({
       bounce: {
         bounceType: 'Permanent',
         bouncedRecipients: [
           { emailAddress: 'test@example.com' },
         ]
       }
-    })).then(function () {
+    })
+    return mockedBounces(mockLog, mockDB).handleBounce(mockMsg).then(function () {
       t.equal(mockDB.emailRecord.callCount, 1)
       t.equal(mockDB.emailRecord.args[0][0], 'test@example.com')
       t.equal(mockDB.deleteAccount.callCount, 1)
@@ -217,6 +222,46 @@ test(
       t.equal(mockLog.messages[2].args[0].op, 'databaseError')
       t.equal(mockLog.messages[2].args[0].email, 'test@example.com')
       t.equal(mockLog.messages[2].args[0].err.errno, error.ERRNO.ACCOUNT_UNKNOWN)
+      t.equal(mockMsg.del.callCount, 1)
+    })
+  }
+)
+
+test(
+  'quoted email addresses are normalized for lookup',
+  function (t) {
+    var mockLog = spyLog()
+    var mockDB = {
+      emailRecord: sinon.spy(function (email) {
+        // Lookup only succeeds when using original, unquoted email addr.
+        if (email !== 'test.@example.com') {
+          return P.reject(new error.unknownAccount(email))
+        }
+        return P.resolve({
+          uid: '123456',
+          email: email,
+          emailVerified: false
+        })
+      }),
+      deleteAccount: sinon.spy(function (record) {
+        return P.resolve({ })
+      })
+    }
+    return mockedBounces(mockLog, mockDB).handleBounce(mockMessage({
+      bounce: {
+        bounceType: 'Permanent',
+        bouncedRecipients: [
+          // Bounce message has email addr in quoted form, since some
+          // mail agents normalize it in this way.
+          { emailAddress: '"test."@example.com' },
+        ]
+      }
+    })).then(function () {
+      t.equal(mockDB.emailRecord.callCount, 2)
+      t.equal(mockDB.emailRecord.args[0][0], '"test."@example.com')
+      t.equal(mockDB.emailRecord.args[1][0], 'test.@example.com')
+      t.equal(mockDB.deleteAccount.callCount, 1)
+      t.equal(mockDB.deleteAccount.args[0][0].email, 'test.@example.com')
     })
   }
 )