Skip to content

fix: delete lock for user ip after password reset #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AVVS merged 4 commits into from
Dec 18, 2019
Merged

fix: delete lock for user ip after password reset #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac498f0
fix: delete lock for user ip after password reset
annahassel Dec 17, 2019
338e328
fix: remove unnecessary code
annahassel Dec 17, 2019
82efd00
fix: error
annahassel Dec 17, 2019
91166cd
chore: add test for previous error
annahassel Dec 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion schemas/verify.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@
"type": "boolean"
}
}
}
}
94 changes: 43 additions & 51 deletions src/actions/updatePassword.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const Promise = require('bluebird');
const Errors = require('common-errors');
const partialRight = require('lodash/partialRight');
const { HttpStatusError } = require('common-errors');

const scrypt = require('../utils/scrypt');
const redisKey = require('../utils/key');
const jwt = require('../utils/jwt');
Expand All @@ -15,44 +14,40 @@ const {
USERS_PASSWORD_FIELD,
USERS_ID_FIELD,
} = require('../constants');
const UserLoginRateLimiter = require('../utils/rate-limiters/user-login-rate-limiter');

// cache error
const Forbidden = new Errors.HttpStatusError(403, 'invalid token');
const Forbidden = new HttpStatusError(403, 'invalid token');

/**
* Verify that username and password match
* @param {Object} service
* @param {String} username
* @param {String} password
*/
function usernamePasswordReset(username, password) {
return Promise
.bind(this, username)
.then(getInternalData)
.tap(isActive)
.tap(isBanned)
.tap(hasPassword)
.tap((data) => scrypt.verify(data.password, password))
.then((data) => data[USERS_ID_FIELD]);
async function usernamePasswordReset(service, username, password) {
const internalData = await getInternalData.call(service, username);

await isActive(internalData);
await isBanned(internalData);
await hasPassword(internalData);

await scrypt.verify(internalData.password, password);

return internalData[USERS_ID_FIELD];
}

/**
* Sets new password for a given username
* @param {String} username
* @param {Object} service
* @param {String} userId
* @param {String} password
*/
function setPassword(_username, password) {
const { redis } = this;

return Promise
.bind(this, _username)
.then(getUserId)
.then((userId) => Promise.props({
userId,
hash: scrypt.hash(password),
}))
.then(({ userId, hash }) => redis
.hset(redisKey(userId, USERS_DATA), USERS_PASSWORD_FIELD, hash)
.return(userId));
async function setPassword(service, userId, password) {
const { redis } = service;
const hash = await scrypt.hash(password);

return redis.hset(redisKey(userId, USERS_DATA), USERS_PASSWORD_FIELD, hash);
}

/**
Expand All @@ -71,48 +66,45 @@ function setPassword(_username, password) {
* @apiParam (Payload) {Boolean} [invalidateTokens=false] - if set to `true` will invalidate issued tokens
* @apiParam (Payload) {String} [remoteip] - will be used for rate limiting if supplied
*/
function updatePassword(request) {
const { redis } = this;
const { newPassword: password, remoteip } = request.params;
async function updatePassword(request) {
const { config, redis, tokenManager } = this;
const { newPassword: password, remoteip = false } = request.params;
const invalidateTokens = !!request.params.invalidateTokens;
const loginRateLimiter = new UserLoginRateLimiter(redis, config.rateLimiters.userLogin);

let userId;

// 2 cases - token reset and current password reset
let promise;
if (request.params.resetToken) {
// Refactor Me If You Can
promise = Promise
.resolve(this.tokenManager.verify(request.params.resetToken, {
try {
const tokenData = await tokenManager.verify(request.params.resetToken, {
erase: true,
control: { action: USERS_ACTION_RESET },
}))
.catchThrow(Forbidden)
.get('id')
.bind(this);
});

// get real user id
userId = await getUserId.call(this, tokenData.id);
} catch (e) {
throw Forbidden;
}
} else {
promise = Promise
.bind(this, [request.params.username, request.params.currentPassword])
.spread(usernamePasswordReset);
userId = await usernamePasswordReset(this, request.params.username, request.params.currentPassword);
}

// update password
promise = promise.then(partialRight(setPassword, password));
await setPassword(this, userId, password);

if (invalidateTokens) {
promise = promise.tap(jwt.reset);
await jwt.reset(userId);
}

if (remoteip) {
promise = promise.tap(function resetLock(username) {
return redis.del(redisKey(username, 'ip', remoteip));
});
if (remoteip !== false && loginRateLimiter.isEnabled()) {
await loginRateLimiter.cleanupForUserIp(userId, remoteip);
}

return promise.return({ success: true });
return { success: true };
}

/**
* Public API
*/
module.exports = updatePassword;
module.exports.updatePassword = setPassword;
module.exports.transports = [require('@microfleet/core').ActionTransport.amqp];
4 changes: 4 additions & 0 deletions src/utils/sliding-window-limiter/redis/lua/slidingWindowReserve.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ if windowInterval == false then
end

blockInterval = false
else
if blockInterval == 0 then
return redis.error_reply('`blockInterval` must be greater than 0 if `windowInterval` is greater than 0')
end
end

if reserveToken == 1 then
Expand Down
4 changes: 2 additions & 2 deletions test/suites/admins.js → test/suites/accounts/admins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/* global startService */

const assert = require('assert');
const simpleDispatcher = require('../helpers/simple-dispatcher');
const simpleDispatcher = require('../../helpers/simple-dispatcher');

describe('#admins', function verifySuite() {
const constants = require('../../src/constants');
const constants = require('../../../src/constants');
const ctx = {};

let service;
Expand Down
45 changes: 37 additions & 8 deletions test/suites/actions/update-password.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const { inspectPromise } = require('@makeomatic/deploy');
const { deepStrictEqual, strictEqual } = require('assert');
const { expect } = require('chai');
const { inspectPromise } = require('@makeomatic/deploy');

const redisKey = require('../../../src/utils/key.js');
const simpleDispatcher = require('../../helpers/simple-dispatcher');

Expand Down Expand Up @@ -108,13 +110,40 @@ describe('#updatePassword', function updatePasswordSuite() {
});
});

it('must update password passed with a valid challenge token', function test() {
return simpleDispatcher(this.users.router)('users.updatePassword', { resetToken: this.token, newPassword: 'vvv' })
.reflect()
.then(inspectPromise())
.then((updatePassword) => {
expect(updatePassword).to.be.deep.eq({ success: true });
});
it('must update password passed with a valid challenge token', async function test() {
const { amqp, redis } = this.users;

const result = await amqp.publishAndWait(
'users.updatePassword',
{ resetToken: this.token, newPassword: 'vvv' }
);
const hashedPassword = await redis.hget(`${this.userId}!data`, 'password');

deepStrictEqual(result, { success: true });
strictEqual(hashedPassword.startsWith('scrypt'), true);
strictEqual(hashedPassword.length > 50, true);
});

it('must delete lock for ip after success update', async function test() {
const { amqp, redis } = this.users;

await redis.zadd(`${this.userId}!ip!10.0.0.1`, 1576335000001, 'token1');
await redis.zadd(`${this.userId}!ip!10.0.0.1`, 1576335000002, 'token2');
await redis.zadd('gl!ip!ctr!10.0.0.1', 1576335000001, 'token1');
await redis.zadd('gl!ip!ctr!10.0.0.1', 1576335000002, 'token2');

strictEqual(await redis.zrange(`${this.userId}!ip!10.0.0.1`, 0, -1).get('length'), 2);
strictEqual(await redis.zrange('gl!ip!ctr!10.0.0.1', 0, -1).get('length'), 2);

const result = await amqp.publishAndWait(
'users.updatePassword',
{ resetToken: this.token, newPassword: 'vvv', remoteip: '10.0.0.1' }
);

deepStrictEqual(result, { success: true });

strictEqual(await redis.zrange(`${this.userId}!ip!10.0.0.1`, 0, -1).get('length'), 0);
strictEqual(await redis.zrange('gl!ip!ctr!10.0.0.1', 0, -1).get('length'), 0);
});
});
});
Expand Down
60 changes: 33 additions & 27 deletions test/suites/utils/sliding-window-limiter/redis/lua/sliding-window-reserve.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,57 +10,63 @@ describe('redis.slidingWindowReserve script', function suite() {
it('should be able to throw error if params are invalid', async () => {
const { redis } = this.ctx.users;

await rejects(async () => redis.slidingWindowReserve(), /^ReplyError: ERR wrong number of arguments for 'evalsha' command$/);
await rejects(redis.slidingWindowReserve(), /^ReplyError: ERR wrong number of arguments for 'evalsha' command$/);

// invalid number of keys
await rejects(async () => redis.slidingWindowReserve(''), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(async () => redis.slidingWindowReserve(null), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(async () => redis.slidingWindowReserve('perchik'), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(async () => redis.slidingWindowReserve(true), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(redis.slidingWindowReserve(''), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(redis.slidingWindowReserve(null), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(redis.slidingWindowReserve('perchik'), /^ReplyError: ERR value is not an integer or out of range$/);
await rejects(redis.slidingWindowReserve(true), /^ReplyError: ERR value is not an integer or out of range$/);

// invalid redis key argument
await rejects(async () => redis.slidingWindowReserve(1), /^ReplyError: invalid `tokenDbKey` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, ''), /^ReplyError: invalid `tokenDbKey` argument$/);
await rejects(redis.slidingWindowReserve(1), /^ReplyError: invalid `tokenDbKey` argument$/);
await rejects(redis.slidingWindowReserve(1, ''), /^ReplyError: invalid `tokenDbKey` argument$/);

// invalid current time argument
await rejects(async () => redis.slidingWindowReserve(1, 'perchik'), /^ReplyError: invalid `currentTime` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 0), /^ReplyError: invalid `currentTime` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', -1), /^ReplyError: invalid `currentTime` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 'fat'), /^ReplyError: invalid `currentTime` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik'), /^ReplyError: invalid `currentTime` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 0), /^ReplyError: invalid `currentTime` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', -1), /^ReplyError: invalid `currentTime` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 'fat'), /^ReplyError: invalid `currentTime` argument$/);

// invalid window interval argument
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000), /^ReplyError: invalid `windowInterval` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, -1), /^ReplyError: invalid `windowInterval` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 'fat'), /^ReplyError: invalid `windowInterval` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000), /^ReplyError: invalid `windowInterval` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, -1), /^ReplyError: invalid `windowInterval` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 'fat'), /^ReplyError: invalid `windowInterval` argument$/);

// invalid window limit argument
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 0), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, -1), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 'fat'), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 0), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, -1), /^ReplyError: invalid `windowLimit` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 'fat'), /^ReplyError: invalid `windowLimit` argument$/);

// invalid block interval argument
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1), /^ReplyError: invalid `blockInterval` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, -1), /^ReplyError: invalid `blockInterval` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 'fat'), /^ReplyError: invalid `blockInterval` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1), /^ReplyError: invalid `blockInterval` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, -1), /^ReplyError: invalid `blockInterval` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 'fat'), /^ReplyError: invalid `blockInterval` argument$/);

// invalid reserve token argument
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0), /^ReplyError: invalid `reserveToken` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, -1), /^ReplyError: invalid `reserveToken` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0), /^ReplyError: invalid `reserveToken` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, -1), /^ReplyError: invalid `reserveToken` argument$/);
await rejects(
async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, 'fat'),
redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, 'fat'),
/^ReplyError: invalid `reserveToken` argument$/
);

// invalid token argument
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, 1), /^ReplyError: invalid `token` argument$/);
await rejects(async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, 1, ''), /^ReplyError: invalid `token` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, 1), /^ReplyError: invalid `token` argument$/);
await rejects(redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 0, 1, ''), /^ReplyError: invalid `token` argument$/);

// if window interval equals 0 then block interval has no sense
await rejects(
async () => redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 10, 0),
redis.slidingWindowReserve(1, 'perchik', 1576335000000, 0, 1, 10, 0),
/^ReplyError: `blockInterval` has no sense if `windowInterval` is gt 0$/
);

// if window interval gt 0 then block interval must be gt 0
await rejects(
redis.slidingWindowReserve(1, 'perchik', 1576335000000, 1000, 1, 0, 0),
/^ReplyError: `blockInterval` must be greater than 0 if `windowInterval` is greater than 0$/
);
});

describe('should be able to works well if window interval equals 0', () => {
Expand Down
Loading