From f9ed3671487df7be176b0d4ce447b9442e245126 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 19 Sep 2018 11:53:00 +0100 Subject: [PATCH] Move the auth mechanism for sudo-mode into SudoModeHandler --- .../SudoMode/SudoModeController.coffee | 3 +- .../Features/SudoMode/SudoModeHandler.coffee | 4 +++ .../SudoMode/SudoModeControllerTests.coffee | 32 +++++++++---------- .../SudoMode/SudoModeHandlerTests.coffee | 17 ++++++++++ 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee index 98bfcab033..b26b88e4f2 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -1,7 +1,6 @@ logger = require 'logger-sharelatex' SudoModeHandler = require './SudoModeHandler' AuthenticationController = require '../Authentication/AuthenticationController' -AuthenticationManager = require '../Authentication/AuthenticationManager' ObjectId = require('../../infrastructure/Mongoose').mongo.ObjectId UserGetter = require '../User/UserGetter' Settings = require 'settings-sharelatex' @@ -40,7 +39,7 @@ module.exports = SudoModeController = err = new Error('user not found') logger.err {err, userId}, "[SudoMode] user not found" return next(err) - AuthenticationManager.authenticate email: userRecord.email, password, (err, user) -> + SudoModeHandler.authenticate userRecord.email, password, (err, user) -> if err? logger.err {err, userId}, "[SudoMode] error authenticating user" return next(err) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee index 004b63ccc2..e145f3cc5b 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee @@ -1,6 +1,7 @@ RedisWrapper = require('../../infrastructure/RedisWrapper') rclient = RedisWrapper.client('sudomode') logger = require('logger-sharelatex') +AuthenticationManager = require '../Authentication/AuthenticationManager' TIMEOUT_IN_SECONDS = 60 * 60 @@ -11,6 +12,9 @@ module.exports = SudoModeHandler = _buildKey: (userId) -> "SudoMode:{#{userId}}" + authenticate: (email, password, callback=(err, user)->) -> + AuthenticationManager.authenticate {email}, password, callback + activateSudoMode: (userId, callback=(err)->) -> if !userId? return callback(new Error('[SudoMode] user must be supplied')) diff --git a/services/web/test/unit/coffee/SudoMode/SudoModeControllerTests.coffee b/services/web/test/unit/coffee/SudoMode/SudoModeControllerTests.coffee index c1de2e278c..6b6c747034 100644 --- a/services/web/test/unit/coffee/SudoMode/SudoModeControllerTests.coffee +++ b/services/web/test/unit/coffee/SudoMode/SudoModeControllerTests.coffee @@ -14,20 +14,18 @@ describe 'SudoModeController', -> @UserGetter = getUser: sinon.stub().callsArgWith(2, null, @user) @SudoModeHandler = + authenticate: sinon.stub() isSudoModeActive: sinon.stub() activateSudoMode: sinon.stub() @AuthenticationController = getLoggedInUserId: sinon.stub().returns(@user._id) _getRediretFromSession: sinon.stub() - @AuthenticationManager = - authenticate: sinon.stub() @UserGetter = getUser: sinon.stub() @SudoModeController = SandboxedModule.require modulePath, requires: 'logger-sharelatex': {log: sinon.stub(), err: sinon.stub()} './SudoModeHandler': @SudoModeHandler '../Authentication/AuthenticationController': @AuthenticationController - '../Authentication/AuthenticationManager': @AuthenticationManager '../../infrastructure/Mongoose': {mongo: {ObjectId: () -> 'some_object_id'}} '../User/UserGetter': @UserGetter @@ -95,7 +93,7 @@ describe 'SudoModeController', -> beforeEach -> @AuthenticationController._getRedirectFromSession = sinon.stub().returns '/somewhere' @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user) - @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user) + @SudoModeHandler.authenticate = sinon.stub().callsArgWith(2, null, @user) @SudoModeHandler.activateSudoMode = sinon.stub().callsArgWith(1, null) @password = 'a_terrible_secret' @req = {body: {password: @password}} @@ -122,8 +120,8 @@ describe 'SudoModeController', -> it 'should try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 1 - @AuthenticationManager.authenticate.calledWith({email: @user.email}, @password).should.equal true + @SudoModeHandler.authenticate.callCount.should.equal 1 + @SudoModeHandler.authenticate.calledWith(@user.email, @password).should.equal true it 'should activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) @@ -155,7 +153,7 @@ describe 'SudoModeController', -> it 'should not try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 0 + @SudoModeHandler.authenticate.callCount.should.equal 0 it 'should not activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) @@ -182,7 +180,7 @@ describe 'SudoModeController', -> it 'should not try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 0 + @SudoModeHandler.authenticate.callCount.should.equal 0 it 'should not activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) @@ -209,7 +207,7 @@ describe 'SudoModeController', -> it 'should not try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 0 + @SudoModeHandler.authenticate.callCount.should.equal 0 it 'should not activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) @@ -221,7 +219,7 @@ describe 'SudoModeController', -> describe 'when authentication fails', -> beforeEach -> - @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, null) + @SudoModeHandler.authenticate = sinon.stub().callsArgWith(2, null, null) @res.json = sinon.stub() @req.i18n = {translate: sinon.stub()} @@ -240,8 +238,8 @@ describe 'SudoModeController', -> it 'should try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 1 - @AuthenticationManager.authenticate.calledWith({email: @user.email}, @password).should.equal true + @SudoModeHandler.authenticate.callCount.should.equal 1 + @SudoModeHandler.authenticate.calledWith(@user.email, @password).should.equal true it 'should not activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) @@ -249,7 +247,7 @@ describe 'SudoModeController', -> describe 'when authentication produces an error', -> beforeEach -> - @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, new Error('woops')) + @SudoModeHandler.authenticate = sinon.stub().callsArgWith(2, new Error('woops')) @next = sinon.stub() it 'should return next with an error', -> @@ -264,8 +262,8 @@ describe 'SudoModeController', -> it 'should try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 1 - @AuthenticationManager.authenticate.calledWith({email: @user.email}, @password).should.equal true + @SudoModeHandler.authenticate.callCount.should.equal 1 + @SudoModeHandler.authenticate.calledWith(@user.email, @password).should.equal true it 'should not activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) @@ -288,8 +286,8 @@ describe 'SudoModeController', -> it 'should try to authenticate the user with the password', -> @SudoModeController.submitPassword(@req, @res, @next) - @AuthenticationManager.authenticate.callCount.should.equal 1 - @AuthenticationManager.authenticate.calledWith({email: @user.email}, @password).should.equal true + @SudoModeHandler.authenticate.callCount.should.equal 1 + @SudoModeHandler.authenticate.calledWith(@user.email, @password).should.equal true it 'should have tried to activate sudo mode', -> @SudoModeController.submitPassword(@req, @res, @next) diff --git a/services/web/test/unit/coffee/SudoMode/SudoModeHandlerTests.coffee b/services/web/test/unit/coffee/SudoMode/SudoModeHandlerTests.coffee index e7f3ccd150..17684f4a68 100644 --- a/services/web/test/unit/coffee/SudoMode/SudoModeHandlerTests.coffee +++ b/services/web/test/unit/coffee/SudoMode/SudoModeHandlerTests.coffee @@ -9,12 +9,17 @@ modulePath = require('path').join __dirname, '../../../../app/js/Features/SudoMo describe 'SudoModeHandler', -> beforeEach -> @userId = 'some_user_id' + @email = 'someuser@example.com' + @user = + _id: @userId + email: @email @rclient = {get: sinon.stub(), set: sinon.stub(), del: sinon.stub()} @RedisWrapper = client: () => @rclient @SudoModeHandler = SandboxedModule.require modulePath, requires: '../../infrastructure/RedisWrapper': @RedisWrapper 'logger-sharelatex': @logger = {log: sinon.stub(), err: sinon.stub()} + '../Authentication/AuthenticationManager': @AuthenticationManager = {} describe '_buildKey', -> @@ -115,6 +120,18 @@ describe 'SudoModeHandler', -> expect(@rclient.del.callCount).to.equal 0 done() + describe 'authenticate', -> + beforeEach -> + @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user) + + it 'should call AuthenticationManager.authenticate', (done) -> + @SudoModeHandler.authenticate @email, 'password', (err, user) => + expect(err).to.not.exist + expect(user).to.exist + expect(user).to.deep.equal @user + expect(@AuthenticationManager.authenticate.callCount).to.equal 1 + done() + describe 'isSudoModeActive', -> beforeEach -> @call = (cb) =>