From 640ab041673828e6b82f713f3d80d9f8ecd75156 Mon Sep 17 00:00:00 2001 From: pajgo Date: Sat, 27 Jul 2019 14:42:22 +0600 Subject: [PATCH 1/4] feat(ms-users): 429 error add more info --- src/actions/login.js | 4 ++-- src/utils/challenges/challenge.js | 8 ++++---- src/utils/checkIpLimits.js | 2 +- test/suites/challenge.js | 4 ++++ test/suites/login.js | 11 +++++++++++ test/suites/register.js | 2 +- 6 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/actions/login.js b/src/actions/login.js index 927d7247f..ad1a7d929 100644 --- a/src/actions/login.js +++ b/src/actions/login.js @@ -46,7 +46,7 @@ const globalLoginAttempts = async (ctx) => { // globally scoped go next if (globalAttempts > ctx.globalLockAfterAttempts) { const duration = moment().add(config.keepGlobalLoginAttempts, 'seconds').toNow(true); - const msg = `You are locked from making login attempts for the next ${duration}`; + const msg = `You are locked from making login attempts for the next ${duration} from ipaddress '${ctx.remoteip}'`; throw new Errors.HttpStatusError(429, msg); } }; @@ -70,7 +70,7 @@ const localLoginAttempts = async (ctx, data) => { // locally scoped login attempts are prioritized if (scopeAttempts > ctx.lockAfterAttempts) { const duration = moment().add(config.keepLoginAttempts, 'seconds').toNow(true); - const msg = `You are locked from making login attempts for the next ${duration}`; + const msg = `You are locked from making login attempts for the next ${duration} from ipaddress '${ctx.remoteip}'`; throw new Errors.HttpStatusError(429, msg); } }; diff --git a/src/utils/challenges/challenge.js b/src/utils/challenges/challenge.js index 954001e3e..3b94de821 100644 --- a/src/utils/challenges/challenge.js +++ b/src/utils/challenges/challenge.js @@ -1,4 +1,5 @@ const partial = require('lodash/partial'); +const moment = require('moment'); const { HttpStatusError } = require('common-errors'); const generateEmail = require('./email/generate.js'); const { @@ -7,9 +8,6 @@ const { } = require('../../constants.js'); const sendSms = require('./phone/send'); -// eslint-disable-next-line max-len -const isThrottled = new HttpStatusError(429, 'We\'ve already sent you an email, if it doesn\'t come - please try again in a little while or send us an email'); - // contains challenges const CHALLENGES = { @@ -36,7 +34,9 @@ async function generateChallenge(type, opts, ctx = {}, wait = false) { ctx.token = token; } catch (error) { if (error.message === '429') { - throw isThrottled; + const duration = moment().add(opts.ttl, 'seconds').toNow(true); + const msg = `We've already sent you an email, if it doesn't come - please try again in ${duration} or send us an email`; + throw new HttpStatusError(429, msg); } throw error; diff --git a/src/utils/checkIpLimits.js b/src/utils/checkIpLimits.js index 9dc293d98..87b18a5fd 100644 --- a/src/utils/checkIpLimits.js +++ b/src/utils/checkIpLimits.js @@ -27,7 +27,7 @@ module.exports = async function checkLimits(redis, registrationLimits, ipaddress const cardinality = props[3]; if (cardinality > times) { - const msg = 'You can\'t register more users from your ipaddress now'; + const msg = `You can't register more users from your ipaddress '${ipaddress}' now`; throw new HttpStatusError(429, msg); } diff --git a/test/suites/challenge.js b/test/suites/challenge.js index 1c23ce3dd..baddfc6a4 100644 --- a/test/suites/challenge.js +++ b/test/suites/challenge.js @@ -69,6 +69,7 @@ describe('#challenge', function challengeSuite() { }); it('must fail to send challenge email more than once in an hour per user', function test() { + const msgRe = /^We've already sent you an email, if it doesn't come - please try again in (.*) or send us an email$/; return Promise.bind(this) .then(requestChallenge) .then(requestChallenge) @@ -77,10 +78,12 @@ describe('#challenge', function challengeSuite() { .then((validation) => { expect(validation.name).to.be.eq('HttpStatusError'); expect(validation.statusCode).to.be.eq(429); + expect(validation.message).to.be.match(msgRe); }); }); it('must fail to send challeng email during race condition', function test() { + const msgRe = /^We've already sent you an email, if it doesn't come - please try again in (.*) or send us an email$/; return Promise .bind(this) .return([requestChallenge, requestChallenge, requestChallenge]) @@ -90,6 +93,7 @@ describe('#challenge', function challengeSuite() { .then((validation) => { expect(validation.name).to.be.eq('HttpStatusError'); expect(validation.statusCode).to.be.eq(429); + expect(validation.message).to.be.match(msgRe); }); }); }); diff --git a/test/suites/login.js b/test/suites/login.js index c6d348ace..a7a8974ca 100644 --- a/test/suites/login.js +++ b/test/suites/login.js @@ -2,6 +2,7 @@ const { inspectPromise } = require('@makeomatic/deploy'); const Promise = require('bluebird'); const assert = require('assert'); +const moment = require('moment'); const { expect } = require('chai'); const { omit, times } = require('lodash'); const sinon = require('sinon').usingPromise(Promise); @@ -117,6 +118,9 @@ describe('#login', function loginSuite() { it('must lock account for authentication after 5 invalid login attemps', () => { const userWithRemoteIP = { remoteip: '10.0.0.1', ...user }; const promises = []; + const config = this.users.config.jwt; + const duration = moment().add(config.keepLoginAttempts, 'seconds').toNow(true); + const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '${userWithRemoteIP.remoteip}'`; times(5, () => { promises.push(( @@ -139,6 +143,7 @@ describe('#login', function loginSuite() { .then((login) => { expect(login.name).to.be.eq('HttpStatusError'); expect(login.statusCode).to.be.eq(429); + expect(login.message).to.be.eq(eMsg); }) )); @@ -148,6 +153,9 @@ describe('#login', function loginSuite() { it('must lock ip for login completely after 15 attempts', async () => { const userWithRemoteIP = { remoteip: '10.0.0.1', ...user, username: 'doesnt_exist' }; const promises = []; + const config = this.users.config.jwt; + const duration = moment().add(config.keepGlobalLoginAttempts, 'seconds').toNow(true); + const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '${userWithRemoteIP.remoteip}'`; times(16, () => { promises.push(( @@ -164,6 +172,9 @@ describe('#login', function loginSuite() { expect(Http404.length).to.be.eq(15); expect(Http429.length).to.be.eq(1); + + const Http429Error = Http429[0]; + expect(Http429Error.message).to.be.eq(eMsg); }); it('should reject signing in with bogus or expired disposable password', () => { diff --git a/test/suites/register.js b/test/suites/register.js index ca7b69714..23cb091c2 100644 --- a/test/suites/register.js +++ b/test/suites/register.js @@ -217,7 +217,7 @@ describe('#register', function registerSuite() { .then((failed) => { assert.equal(failed.name, 'HttpStatusError'); assert.equal(failed.statusCode, 429); - assert.equal(failed.message, 'You can\'t register more users from your ipaddress now'); + assert.equal(failed.message, `You can't register more users from your ipaddress '${opts.ipaddress}' now`); }); }); }); From 1e71abfd19fb955cc139fd767b7ca6ddbbb7a05e Mon Sep 17 00:00:00 2001 From: pajgo Date: Wed, 28 Aug 2019 16:03:09 +0600 Subject: [PATCH 2/4] feat(ms-users): 429 error add more info * challenge tests logic change --- test/suites/challenge.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/test/suites/challenge.js b/test/suites/challenge.js index baddfc6a4..8cc2bb74d 100644 --- a/test/suites/challenge.js +++ b/test/suites/challenge.js @@ -1,6 +1,7 @@ const { inspectPromise } = require('@makeomatic/deploy'); const Promise = require('bluebird'); const { expect } = require('chai'); +const moment = require('moment'); describe('#challenge', function challengeSuite() { beforeEach(global.startService); @@ -69,7 +70,12 @@ describe('#challenge', function challengeSuite() { }); it('must fail to send challenge email more than once in an hour per user', function test() { - const msgRe = /^We've already sent you an email, if it doesn't come - please try again in (.*) or send us an email$/; + const { token } = this.users.config; + const { ttl } = token.email; + + const duration = moment().add(ttl, 'seconds').toNow(true); + const msg = `We've already sent you an email, if it doesn't come - please try again in ${duration} or send us an email`; + return Promise.bind(this) .then(requestChallenge) .then(requestChallenge) @@ -78,12 +84,17 @@ describe('#challenge', function challengeSuite() { .then((validation) => { expect(validation.name).to.be.eq('HttpStatusError'); expect(validation.statusCode).to.be.eq(429); - expect(validation.message).to.be.match(msgRe); + expect(validation.message).to.be.eq(msg); }); }); it('must fail to send challeng email during race condition', function test() { - const msgRe = /^We've already sent you an email, if it doesn't come - please try again in (.*) or send us an email$/; + const { token } = this.users.config; + const { ttl } = token.email; + + const duration = moment().add(ttl, 'seconds').toNow(true); + const msg = `We've already sent you an email, if it doesn't come - please try again in ${duration} or send us an email`; + return Promise .bind(this) .return([requestChallenge, requestChallenge, requestChallenge]) @@ -93,7 +104,7 @@ describe('#challenge', function challengeSuite() { .then((validation) => { expect(validation.name).to.be.eq('HttpStatusError'); expect(validation.statusCode).to.be.eq(429); - expect(validation.message).to.be.match(msgRe); + expect(validation.message).to.be.eq(msg); }); }); }); From d4e6384afd980b82ff3dcedd58372694bdc3235d Mon Sep 17 00:00:00 2001 From: pajgo Date: Thu, 29 Aug 2019 14:44:55 +0600 Subject: [PATCH 3/4] feat(ms-users): 429 error add more info * no ip vars in error messages --- test/suites/login.js | 4 ++-- test/suites/register.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/suites/login.js b/test/suites/login.js index a7a8974ca..53ff75d6a 100644 --- a/test/suites/login.js +++ b/test/suites/login.js @@ -120,7 +120,7 @@ describe('#login', function loginSuite() { const promises = []; const config = this.users.config.jwt; const duration = moment().add(config.keepLoginAttempts, 'seconds').toNow(true); - const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '${userWithRemoteIP.remoteip}'`; + const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '10.0.0.1'`; times(5, () => { promises.push(( @@ -155,7 +155,7 @@ describe('#login', function loginSuite() { const promises = []; const config = this.users.config.jwt; const duration = moment().add(config.keepGlobalLoginAttempts, 'seconds').toNow(true); - const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '${userWithRemoteIP.remoteip}'`; + const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '10.0.0.1'`; times(16, () => { promises.push(( diff --git a/test/suites/register.js b/test/suites/register.js index 23cb091c2..e149db13e 100644 --- a/test/suites/register.js +++ b/test/suites/register.js @@ -217,7 +217,7 @@ describe('#register', function registerSuite() { .then((failed) => { assert.equal(failed.name, 'HttpStatusError'); assert.equal(failed.statusCode, 429); - assert.equal(failed.message, `You can't register more users from your ipaddress '${opts.ipaddress}' now`); + assert.equal(failed.message, 'You can\'t register more users from your ipaddress \'192.168.1.1\' now'); }); }); }); From 489a79c229568034ff2ba7802a274310ce508324 Mon Sep 17 00:00:00 2001 From: pajgo Date: Fri, 13 Sep 2019 15:13:40 +0600 Subject: [PATCH 4/4] fix(ms-users): 429 error add more info * no duration var - used scalar --- test/suites/challenge.js | 13 ++----------- test/suites/login.js | 9 ++------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/test/suites/challenge.js b/test/suites/challenge.js index 8cc2bb74d..2736c14b1 100644 --- a/test/suites/challenge.js +++ b/test/suites/challenge.js @@ -1,7 +1,6 @@ const { inspectPromise } = require('@makeomatic/deploy'); const Promise = require('bluebird'); const { expect } = require('chai'); -const moment = require('moment'); describe('#challenge', function challengeSuite() { beforeEach(global.startService); @@ -70,11 +69,7 @@ describe('#challenge', function challengeSuite() { }); it('must fail to send challenge email more than once in an hour per user', function test() { - const { token } = this.users.config; - const { ttl } = token.email; - - const duration = moment().add(ttl, 'seconds').toNow(true); - const msg = `We've already sent you an email, if it doesn't come - please try again in ${duration} or send us an email`; + const msg = 'We\'ve already sent you an email, if it doesn\'t come - please try again in 4 hours or send us an email'; return Promise.bind(this) .then(requestChallenge) @@ -89,11 +84,7 @@ describe('#challenge', function challengeSuite() { }); it('must fail to send challeng email during race condition', function test() { - const { token } = this.users.config; - const { ttl } = token.email; - - const duration = moment().add(ttl, 'seconds').toNow(true); - const msg = `We've already sent you an email, if it doesn't come - please try again in ${duration} or send us an email`; + const msg = 'We\'ve already sent you an email, if it doesn\'t come - please try again in 4 hours or send us an email'; return Promise .bind(this) diff --git a/test/suites/login.js b/test/suites/login.js index 53ff75d6a..3502b96a5 100644 --- a/test/suites/login.js +++ b/test/suites/login.js @@ -2,7 +2,6 @@ const { inspectPromise } = require('@makeomatic/deploy'); const Promise = require('bluebird'); const assert = require('assert'); -const moment = require('moment'); const { expect } = require('chai'); const { omit, times } = require('lodash'); const sinon = require('sinon').usingPromise(Promise); @@ -118,9 +117,7 @@ describe('#login', function loginSuite() { it('must lock account for authentication after 5 invalid login attemps', () => { const userWithRemoteIP = { remoteip: '10.0.0.1', ...user }; const promises = []; - const config = this.users.config.jwt; - const duration = moment().add(config.keepLoginAttempts, 'seconds').toNow(true); - const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '10.0.0.1'`; + const eMsg = 'You are locked from making login attempts for the next 2 hours from ipaddress \'10.0.0.1\''; times(5, () => { promises.push(( @@ -153,9 +150,7 @@ describe('#login', function loginSuite() { it('must lock ip for login completely after 15 attempts', async () => { const userWithRemoteIP = { remoteip: '10.0.0.1', ...user, username: 'doesnt_exist' }; const promises = []; - const config = this.users.config.jwt; - const duration = moment().add(config.keepGlobalLoginAttempts, 'seconds').toNow(true); - const eMsg = `You are locked from making login attempts for the next ${duration} from ipaddress '10.0.0.1'`; + const eMsg = 'You are locked from making login attempts for the next 7 days from ipaddress \'10.0.0.1\''; times(16, () => { promises.push((