fix(server): clear ip record after whitelisting

lib/ip_record.js

@@ -30,6 +30,10 @@ module.exports = function (limits, now) {
     return rec
   }
 
+  IpRecord.prototype.clear = function () {
+    this.rl = this.bk = null
+  }
+
   IpRecord.prototype.getMinLifetimeMS = function () {
     return Math.max(
       limits.blockIntervalMs,

lib/server.js

@@ -149,10 +149,15 @@ module.exports = function createServer(config, log) {
           block: result.block,
           suspect: result.suspect
         })
+
         result.block = false
         result.suspect = false
+
+        return true
       }
     }
+
+    return false
   }
 
   function optionallyReportIp (result, ip, action) {
@@ -196,7 +201,7 @@ module.exports = function createServer(config, log) {
       fetchRecords(ip, email, phoneNumber)
         .spread(
           function (ipRecord, reputation, emailRecord, ipEmailRecord, smsRecord) {
-            if (ipRecord.isBlocked()) {
+            if (ipRecord.isBlocked() && ! allowWhitelisted({ block: true }, ip, email)) {
               // a blocked ip should just be ignored completely
               // it's malicious, it shouldn't penalize emails or allow
               // (most) escape hatches. just abort!
@@ -253,24 +258,26 @@ module.exports = function createServer(config, log) {
               blockReason = blockReasons.IP_BAD_REPUTATION
             }
 
+            const result = {
+              block: block,
+              blockReason: blockReason,
+              retryAfter: retryAfter,
+              unblock: canUnblock,
+              suspect: suspect
+            }
+
+            if (allowWhitelisted(result, ip, email)) {
+              // Prevent white-listed email addresses from blocking
+              // subsequent requests to /checkIpOnly
+              ipRecord.clear()
+            }
+
             return setRecords(ip, ipRecord, email, emailRecord, ipEmailRecord, phoneNumber, smsRecord)
-              .then(
-                function () {
-                  return {
-                    block: block,
-                    blockReason: blockReason,
-                    retryAfter: retryAfter,
-                    unblock: canUnblock,
-                    suspect: suspect
-                  }
-                }
-              )
+              .then(() => result)
           }
         )
         .then(
           function (result) {
-            allowWhitelisted(result, ip, email)
-
             log.info({
               op: 'request.check',
               email: email,

test/remote/check_tests.js

@@ -1,13 +1,22 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
-var test = require('tap').test
-var restify = require('restify')
-var TestServer = require('../test_server')
-var mcHelper = require('../memcache-helper')
+'use strict'
 
-var TEST_EMAIL = 'test@example.com'
-var TEST_IP = '192.0.2.1'
+const mcHelper = require('../memcache-helper')
+const Promise = require('bluebird')
+const restify = require('restify')
+const test = require('tap').test
+const TestServer = require('../test_server')
+
+const TEST_EMAIL = 'test@example.com'
+const TEST_IP = '192.0.2.1'
+const ALT_EMAILS = []
+for (let i = 0; i < 3; ++i) {
+  ALT_EMAILS[i] = `test.${i}@${i < 2 ? 'example.com' : 'restmail.net' }`
+}
+
+process.env.MAX_VERIFY_CODES = '1'
 
 var config = {
   listen: {
@@ -42,6 +51,7 @@ test(
 var client = restify.createJsonClient({
   url: 'http://127.0.0.1:' + config.listen.port
 })
+Promise.promisifyAll(client, { multiArgs: true })
 
 // NOTE: Leading semi-colon because ASI is funny.
 ; ['accountCreate', 'accountLogin', 'passwordChange'].forEach(function (action) {
@@ -88,6 +98,49 @@ test(
   }
 )
 
+test('allowed email addresses in /check do not block subsequent requests to /checkIpOnly', t => {
+  return client.postAsync('/check', {
+    email: ALT_EMAILS[0],
+    ip: TEST_IP,
+    action: 'recoveryEmailVerifyCode'
+  })
+  .spread((req, res, obj) => {
+    t.equal(res.statusCode, 200, '/check succeeded')
+    t.equal(obj.block, false, 'request was not blocked')
+
+    return client.postAsync('/check', {
+      email: ALT_EMAILS[1],
+      ip: TEST_IP,
+      action: 'recoveryEmailVerifyCode'
+    })
+  })
+  .spread((req, res, obj) => {
+    t.equal(res.statusCode, 200, '/check succeeded')
+    t.equal(obj.block, true, 'request was blocked')
+
+    return client.postAsync('/check', {
+      email: ALT_EMAILS[2],
+      ip: TEST_IP,
+      action: 'recoveryEmailVerifyCode'
+    })
+  })
+  .spread((req, res, obj) => {
+    t.equal(res.statusCode, 200, '/check succeeded')
+    t.equal(obj.block, false, 'request was not blocked')
+
+    return client.postAsync('/checkIpOnly', {
+      ip: TEST_IP,
+      action: 'consumeSigninCode'
+    })
+  })
+  .spread((req, res, obj) => {
+    t.equal(res.statusCode, 200, '/checkIpOnly succeeded')
+    t.equal(obj.block, false, 'request was not blocked')
+  })
+  .catch(err => t.fail(err))
+  .then(() => t.end())
+})
+
 test(
   'teardown',
   function (t) {