fix(ip_record): Correctly total bad logins by unique email address.

lib/ip_record.js

@@ -39,17 +39,21 @@ module.exports = function (limits, now) {
   IpRecord.prototype.isOverBadLogins = function () {
     this.trimBadLogins(now())
     // IPs are limited based on the number of unique email
-    // addresses they access.  Take the highest-weighted
-    // bad-login event for each email address.
-    var total = 0
-    var seen = {}
+    // addresses they access.  Sum the highest-weighted
+    // bad-login event for each user account to determine
+    // the overall bad-logins score.
+    var weights = {}
     this.lf.forEach(function(info) {
-      var incr = limits.badLoginErrnoWeights[info.e] || 1
-      if (info.u in seen && seen[info.u] < incr) {
-        total -= seen[info.u]
-        seen[info.u] = incr
-      }
-      total += incr
+      var user = info.u
+      var errno = info.e
+      weights[user] = Math.max(
+        limits.badLoginErrnoWeights[errno] || 1,
+        weights[user] || 0
+      )
+    })
+    var total = 0
+    Object.keys(weights).forEach(function(user) {
+      total += weights[user]
     })
     return total > limits.maxBadLoginsPerIp
   }

test/local/ip_record_tests.js

@@ -169,19 +169,43 @@ test(
   'addBadLogins works per IP',
   function (t) {
     var ir = simpleIpRecord()
-    ir.addBadLogin({ errno: 999 })
+    ir.addBadLogin({ email: 'test1@example.com', errno: 999 })
     t.equal(ir.isOverBadLogins(), false, 'one record is not over')
-    ir.addBadLogin({ errno: 555 })
-    ir.addBadLogin({ errno: 444 })
+    ir.addBadLogin({ email: 'test2@example.com', errno: 555 })
+    ir.addBadLogin({ email: 'test3@example.com', errno: 444 })
     t.equal(ir.isOverBadLogins(), false, 'three records is not over')
-    ir.addBadLogin({ errno: 777 })
+    ir.addBadLogin({ email: 'test4@example.com', errno: 777 })
     t.equal(ir.isOverBadLogins(), true, 'four records is over')
 
     var ir2 = simpleIpRecord()
-    ir2.addBadLogin({ errno: 102 })
+    ir2.addBadLogin({ email: 'test1@example.com', errno: 102 })
     t.equal(ir2.isOverBadLogins(), false, 'one unknown record is not over')
-    ir2.addBadLogin({ errno: 102 })
+    ir2.addBadLogin({ email: 'test2@example.com', errno: 102 })
     t.equal(ir2.isOverBadLogins(), true, 'two unknown records is over')
     t.end()
   }
 )
+
+test(
+  'isOverBadLogins counts max per unique email addresses',
+  function (t) {
+    var ir = simpleIpRecord()
+    ir.addBadLogin({ email: 'test1@example.com' })
+    ir.addBadLogin({ email: 'test1@example.com' })
+    ir.addBadLogin({ email: 'test1@example.com' })
+    ir.addBadLogin({ email: 'test1@example.com' })
+    ir.addBadLogin({ email: 'test1@example.com' })
+    t.equal(ir.isOverBadLogins(), false, 'one email does not put it over')
+
+    ir.addBadLogin({ email: 'test2@example.com' })
+    t.equal(ir.isOverBadLogins(), false, 'two emails does not put it over')
+
+    ir.addBadLogin({ email: 'test3@example.com' })
+    t.equal(ir.isOverBadLogins(), false, 'three emails does not put it over')
+
+    ir.addBadLogin({ email: 'test1@example.com', errno: 102 })
+    t.equal(ir.isOverBadLogins(), true, 'extra score for first email puts it over')
+
+    t.end()
+  }
+)