diff --git a/lib/ip_record.js b/lib/ip_record.js index bfa1f70..119313b 100644 --- a/lib/ip_record.js +++ b/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, diff --git a/lib/server.js b/lib/server.js index d8074c6..59547bf 100755 --- a/lib/server.js +++ b/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, diff --git a/test/remote/check_tests.js b/test/remote/check_tests.js index 4cb3c6d..e4044c3 100644 --- a/test/remote/check_tests.js +++ b/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) {