Skip to content

Commit 6f989a8

Browse files
pajgoannahassel
authored andcommitted
feat(ms-users): 429 error add more info (#412)
1 parent afdbe1c commit 6f989a8

6 files changed

Lines changed: 20 additions & 8 deletions

File tree

src/actions/login.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const globalLoginAttempts = async (ctx) => {
4646
// globally scoped go next
4747
if (globalAttempts > ctx.globalLockAfterAttempts) {
4848
const duration = moment().add(config.keepGlobalLoginAttempts, 'seconds').toNow(true);
49-
const msg = `You are locked from making login attempts for the next ${duration}`;
49+
const msg = `You are locked from making login attempts for the next ${duration} from ipaddress '${ctx.remoteip}'`;
5050
throw new Errors.HttpStatusError(429, msg);
5151
}
5252
};
@@ -70,7 +70,7 @@ const localLoginAttempts = async (ctx, data) => {
7070
// locally scoped login attempts are prioritized
7171
if (scopeAttempts > ctx.lockAfterAttempts) {
7272
const duration = moment().add(config.keepLoginAttempts, 'seconds').toNow(true);
73-
const msg = `You are locked from making login attempts for the next ${duration}`;
73+
const msg = `You are locked from making login attempts for the next ${duration} from ipaddress '${ctx.remoteip}'`;
7474
throw new Errors.HttpStatusError(429, msg);
7575
}
7676
};

src/utils/challenges/challenge.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const partial = require('lodash/partial');
2+
const moment = require('moment');
23
const { HttpStatusError } = require('common-errors');
34
const generateEmail = require('./email/generate.js');
45
const {
@@ -7,9 +8,6 @@ const {
78
} = require('../../constants.js');
89
const sendSms = require('./phone/send');
910

10-
// eslint-disable-next-line max-len
11-
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');
12-
1311
// contains challenges
1412
const CHALLENGES = {
1513

@@ -36,7 +34,9 @@ async function generateChallenge(type, opts, ctx = {}, wait = false) {
3634
ctx.token = token;
3735
} catch (error) {
3836
if (error.message === '429') {
39-
throw isThrottled;
37+
const duration = moment().add(opts.ttl, 'seconds').toNow(true);
38+
const msg = `We've already sent you an email, if it doesn't come - please try again in ${duration} or send us an email`;
39+
throw new HttpStatusError(429, msg);
4040
}
4141

4242
throw error;

src/utils/checkIpLimits.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ module.exports = async function checkLimits(redis, registrationLimits, ipaddress
2727

2828
const cardinality = props[3];
2929
if (cardinality > times) {
30-
const msg = 'You can\'t register more users from your ipaddress now';
30+
const msg = `You can't register more users from your ipaddress '${ipaddress}' now`;
3131
throw new HttpStatusError(429, msg);
3232
}
3333

test/suites/challenge.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ describe('#challenge', function challengeSuite() {
6969
});
7070

7171
it('must fail to send challenge email more than once in an hour per user', function test() {
72+
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';
73+
7274
return Promise.bind(this)
7375
.then(requestChallenge)
7476
.then(requestChallenge)
@@ -77,10 +79,13 @@ describe('#challenge', function challengeSuite() {
7779
.then((validation) => {
7880
expect(validation.name).to.be.eq('HttpStatusError');
7981
expect(validation.statusCode).to.be.eq(429);
82+
expect(validation.message).to.be.eq(msg);
8083
});
8184
});
8285

8386
it('must fail to send challeng email during race condition', function test() {
87+
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';
88+
8489
return Promise
8590
.bind(this)
8691
.return([requestChallenge, requestChallenge, requestChallenge])
@@ -90,6 +95,7 @@ describe('#challenge', function challengeSuite() {
9095
.then((validation) => {
9196
expect(validation.name).to.be.eq('HttpStatusError');
9297
expect(validation.statusCode).to.be.eq(429);
98+
expect(validation.message).to.be.eq(msg);
9399
});
94100
});
95101
});

test/suites/login.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ describe('#login', function loginSuite() {
117117
it('must lock account for authentication after 5 invalid login attemps', () => {
118118
const userWithRemoteIP = { remoteip: '10.0.0.1', ...user };
119119
const promises = [];
120+
const eMsg = 'You are locked from making login attempts for the next 2 hours from ipaddress \'10.0.0.1\'';
120121

121122
times(5, () => {
122123
promises.push((
@@ -139,6 +140,7 @@ describe('#login', function loginSuite() {
139140
.then((login) => {
140141
expect(login.name).to.be.eq('HttpStatusError');
141142
expect(login.statusCode).to.be.eq(429);
143+
expect(login.message).to.be.eq(eMsg);
142144
})
143145
));
144146

@@ -148,6 +150,7 @@ describe('#login', function loginSuite() {
148150
it('must lock ip for login completely after 15 attempts', async () => {
149151
const userWithRemoteIP = { remoteip: '10.0.0.1', ...user, username: 'doesnt_exist' };
150152
const promises = [];
153+
const eMsg = 'You are locked from making login attempts for the next 7 days from ipaddress \'10.0.0.1\'';
151154

152155
times(16, () => {
153156
promises.push((
@@ -164,6 +167,9 @@ describe('#login', function loginSuite() {
164167

165168
expect(Http404.length).to.be.eq(15);
166169
expect(Http429.length).to.be.eq(1);
170+
171+
const Http429Error = Http429[0];
172+
expect(Http429Error.message).to.be.eq(eMsg);
167173
});
168174

169175
it('should reject signing in with bogus or expired disposable password', () => {

test/suites/register.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ describe('#register', function registerSuite() {
217217
.then((failed) => {
218218
assert.equal(failed.name, 'HttpStatusError');
219219
assert.equal(failed.statusCode, 429);
220-
assert.equal(failed.message, 'You can\'t register more users from your ipaddress now');
220+
assert.equal(failed.message, 'You can\'t register more users from your ipaddress \'192.168.1.1\' now');
221221
});
222222
});
223223
});

0 commit comments

Comments
 (0)