Conversation
store/query/auth.js
Outdated
|
|
||
| const time = 250 * user.failedAttempts; | ||
| const lockedTill = moment().add(time, 'ms'); | ||
| await this.findOneAndUpdate({ email }, { $set: { failedAttempts: user.failedAttempts + 1, locked: lockedTill } }); |
There was a problem hiding this comment.
What's wrong with user.save() in this instance? Why not set the user.locked field directly?
store/query/auth.js
Outdated
| const users = await this.find({ email, password: hash }); | ||
| if (users.length === 0) throw new Error('Wrong e-mail or password'); | ||
| if (user.locked && moment().isBefore(user.locked)) { | ||
| throw { Error: 'LockError', retry: user.locked }; |
There was a problem hiding this comment.
Change the key to name instead of Error. A true error object has { name: 'ErrorType', message: 'details' }. Sticking to this convention will allow us to replace this object literal with an error instance.
api/controllers/login.js
Outdated
| } catch (e) { | ||
| if (e.Error === 'LockError') { | ||
| res.set('Retry-After', e.retry); | ||
| return res.failure('To many attempts.', 429); |
There was a problem hiding this comment.
The message should be changed to:
'Login have been locked for a short cooldown period due to failed login attempts.'
api/controllers/login.js
Outdated
| return res.success(token); | ||
| } catch (e) { | ||
| if (e.Error === 'LockError') { | ||
| res.set('Retry-After', e.retry); |
There was a problem hiding this comment.
Please format the retry date as an ISO 8601 string, if that is not already the case.
kparkov
left a comment
kparkov
left a comment
There was a problem hiding this comment.
No unit tests were written to validate this change.
|
Notice: 4 test are failing in this commit because they are outdated (has been fixed in another issue), and has nothing to do with the newest implemented test. |
2 participants
No description provided.