From ae71f2deca11139fe739d666d52d3c6d48c885a1 Mon Sep 17 00:00:00 2001 From: Fabien O'Carroll Date: Thu, 18 Oct 2018 15:58:29 +0700 Subject: [PATCH] Added spam prevention for v2 sessions (#10030) no-issue - Added spam prevention to POST /session - This blocks repeated requests the the /session endpoint preventing brute force password attacks - Updated session controller to reset brute middleware - This updates the session controller to reset the brute force protection on a successful login. This is required so that a user is not locked out forever :o!! --- core/server/api/v2/session.js | 9 +++++-- core/server/web/api/v2/admin/routes.js | 6 ++++- core/test/unit/api/v2/session_spec.js | 37 ++++++++++++++++++++++++-- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/core/server/api/v2/session.js b/core/server/api/v2/session.js index e8e21ad8ff..63b0405d7f 100644 --- a/core/server/api/v2/session.js +++ b/core/server/api/v2/session.js @@ -25,8 +25,13 @@ const session = { password: object.password }).then((user) => { return Promise.resolve((req, res, next) => { - req.user = user; - auth.session.createSession(req, res, next); + req.brute.reset(function (err) { + if (err) { + return next(err); + } + req.user = user; + auth.session.createSession(req, res, next); + }); }); }).catch((err) => { throw new common.errors.UnauthorizedError({ diff --git a/core/server/web/api/v2/admin/routes.js b/core/server/web/api/v2/admin/routes.js index 15005bdd88..f4b1b849bb 100644 --- a/core/server/web/api/v2/admin/routes.js +++ b/core/server/web/api/v2/admin/routes.js @@ -148,7 +148,11 @@ module.exports = function apiRoutes() { // ## Sessions router.get('/session', mw.authAdminAPI, api.http(apiv2.session.read)); // We don't need auth when creating a new session (logging in) - router.post('/session', api.http(apiv2.session.add)); + router.post('/session', + shared.middlewares.brute.globalBlock, + shared.middlewares.brute.userLogin, + api.http(apiv2.session.add) + ); router.del('/session', mw.authAdminAPI, api.http(apiv2.session.delete)); // ## Authentication diff --git a/core/test/unit/api/v2/session_spec.js b/core/test/unit/api/v2/session_spec.js index 9840b8c08f..79dcbd8715 100644 --- a/core/test/unit/api/v2/session_spec.js +++ b/core/test/unit/api/v2/session_spec.js @@ -49,8 +49,12 @@ describe('Session controller', function () { }); }); - it('it returns a function that sets req.user and calls createSession if the check works', function () { - const fakeReq = {}; + it('it returns a function that calls req.brute.reset, sets req.user and calls createSession if the check works', function () { + const fakeReq = { + brute: { + reset: sandbox.stub().callsArg(0) + } + }; const fakeRes = {}; const fakeNext = () => {}; const fakeUser = models.User.forge({}); @@ -65,6 +69,8 @@ describe('Session controller', function () { }, {}).then((fn) => { fn(fakeReq, fakeRes, fakeNext); }).then(function () { + should.equal(fakeReq.brute.reset.callCount, 1); + const createSessionStubCall = createSessionStub.getCall(0); should.equal(fakeReq.user, fakeUser); should.equal(createSessionStubCall.args[0], fakeReq); @@ -72,6 +78,33 @@ describe('Session controller', function () { should.equal(createSessionStubCall.args[2], fakeNext); }); }); + + it('it returns a function that calls req.brute.reset and calls next if reset errors', function () { + const resetError = new Error(); + const fakeReq = { + brute: { + reset: sandbox.stub().callsArgWith(0, resetError) + } + }; + const fakeRes = {}; + const fakeNext = sandbox.stub(); + const fakeUser = models.User.forge({}); + sandbox.stub(models.User, 'check') + .resolves(fakeUser); + + const createSessionStub = sandbox.stub(sessionServiceMiddleware, 'createSession'); + + return sessionController.add({ + username: 'freddy@vodafone.com', + password: 'qu33nRul35' + }, {}).then((fn) => { + fn(fakeReq, fakeRes, fakeNext); + }).then(function () { + should.equal(fakeReq.brute.reset.callCount, 1); + should.equal(fakeNext.callCount, 1); + should.equal(fakeNext.args[0][0], resetError); + }); + }); }); describe('#delete', function () {