From 029c96c7ccf720a7e393f160b259874553487583 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 10:05:48 +0100 Subject: [PATCH 01/18] Add sudo-mode 'confirm password' prompt --- .../SudoMode/SudoModeController.coffee | 58 +++++++++++++++++++ .../Features/SudoMode/SudoModeHandler.coffee | 27 +++++++++ services/web/app/coffee/router.coffee | 4 ++ .../app/views/sudo_mode/sudo_mode_prompt.pug | 39 +++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 services/web/app/coffee/Features/SudoMode/SudoModeController.coffee create mode 100644 services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee create mode 100644 services/web/app/views/sudo_mode/sudo_mode_prompt.pug diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee new file mode 100644 index 0000000000..f569d3476e --- /dev/null +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -0,0 +1,58 @@ +UserLocator = require "../User/UserLocator" +Settings = require "settings-sharelatex" +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' + + +module.exports = SudoModeController = + + sudoModePrompt: (req, res, next) -> + userId = AuthenticationController.getLoggedInUserId(req) + logger.log {userId}, "[SudoMode] rendering sudo mode password page" + SudoModeHandler.isSudoModeActive userId, (err, isActive) -> + if err? + logger.err {err, userId}, "[SudoMode] error checking if sudo mode is active" + return next(err) + if isActive + logger.log {userId}, "[SudoMode] sudo mode already active, redirecting" + return res.redirect('/project') + res.render 'sudo_mode/sudo_mode_prompt', title: 'confirm_your_password' + + submitPassword: (req, res, next) -> + userId = AuthenticationController.getLoggedInUserId(req) + redir = AuthenticationController._getRedirectFromSession(req) || "/project" + password = req.body.password + if !password + logger.log {userId}, "[SudoMode] no password supplied, failed authentication" + return next(new Error('no password supplied')) + logger.log {userId, redir}, "[SudoMode] checking user password" + UserGetter.getUser ObjectId(userId), {email: 1}, (err, userRecord) -> + if err? + logger.err {err, userId}, "[SudoMode] error getting user" + return next(err) + AuthenticationManager.authenticate email: userRecord.email, password, (err, user) -> + if err? + logger.err {err, userId}, "[SudoMode] error authenticating user" + return next(err) + if user? + logger.log {userId}, "[SudoMode] authenticated user, activating sudo mode" + SudoModeHandler.activateSudoMode userId, (err) -> + if err? + logger.err {err, userId}, "[SudoMode] error activating sudo mode" + return next(err) + return res.json { + redir: redir + } + else + logger.log {userId}, "[SudoMode] authentication failed for user" + return res.json { + message: { + text: req.i18n.translate("invalid_password"), + type: 'error' + } + } + diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee new file mode 100644 index 0000000000..398a39048c --- /dev/null +++ b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee @@ -0,0 +1,27 @@ +RedisWrapper = require('../../infrastructure/RedisWrapper') +rclient = RedisWrapper.client('sudomode') +logger = require('logger-sharelatex') + + +TIMEOUT_IN_SECONDS = 60 * 10 + + +module.exports = SudoModeHandler = + + _buildKey: (userId) -> + "SudoMode:{#{userId}}" + + activateSudoMode: (userId, callback=(err)->) -> + if !userId? + return callback(new Error('[SudoMode] user must not be supplied')) + duration = TIMEOUT_IN_SECONDS + logger.log {userId, duration}, "[SudoMode] activating sudo mode for user" + rclient.set SudoModeHandler._buildKey(userId), '1', 'EX', duration, callback + + isSudoModeActive: (userId, callback=(err, isActive)->) -> + if !userId? + return callback(new Error('[SudoMode] user must not be supplied')) + rclient.get SudoModeHandler._buildKey(userId), (err, result) -> + if err? + return callback(err) + callback(null, result == '1') diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 0556c23c5a..ff5cd4b64c 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -38,6 +38,7 @@ ContactRouter = require("./Features/Contacts/ContactRouter") ReferencesController = require('./Features/References/ReferencesController') AuthorizationMiddlewear = require('./Features/Authorization/AuthorizationMiddlewear') BetaProgramController = require('./Features/BetaProgram/BetaProgramController') +SudoModeController = require('./Features/SudoMode/SudoModeController') AnalyticsRouter = require('./Features/Analytics/AnalyticsRouter') AnnouncementsController = require("./Features/Announcements/AnnouncementsController") @@ -238,6 +239,9 @@ module.exports = class Router webRouter.get "/beta/participate", AuthenticationController.requireLogin(), BetaProgramController.optInPage webRouter.post "/beta/opt-in", AuthenticationController.requireLogin(), BetaProgramController.optIn webRouter.post "/beta/opt-out", AuthenticationController.requireLogin(), BetaProgramController.optOut + webRouter.get "/confirm-password", AuthenticationController.requireLogin(), SudoModeController.sudoModePrompt + webRouter.post "/confirm-password/submit", AuthenticationController.requireLogin(), SudoModeController.submitPassword + #Admin Stuff webRouter.get '/admin', AuthorizationMiddlewear.ensureUserIsSiteAdmin, AdminController.index diff --git a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug new file mode 100644 index 0000000000..3b089517c1 --- /dev/null +++ b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug @@ -0,0 +1,39 @@ +extends ../layout + +block content + .content.content-alt + .container + .row + .col-md-10.col-md-offset-1.col-lg-8.col-lg-offset-2 + .card + .page-header.text-centered + h1 Confirm your password to continue + div + .container-fluid + .row + .col-md-8.col-md-offset-2 + form(async-form="confirmPassword", name="confirmPassword", + action='/confirm-password/submit', method="POST", ng-cloak) + input(name='_csrf', type='hidden', value=csrfToken) + form-messages(for="confirmPassword") + .form-group + label + | #{translate('password')} + input.form-control( + type='password', + name='password', + required, + placeholder='********', + ng-model="password" + ) + span.small.text-primary( + ng-show="confirmPassword.password.$invalid && confirmPassword.password.$dirty" + ) + | #{translate("required")} + .actions + button.btn-primary.btn( + style="width: 100%", + type='submit', + ng-disabled="confirmPassword.inflight" + ) + span #{translate("confirm")} From 094784b6d5610882c7486e2954cf90dc22e3676e Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 10:25:32 +0100 Subject: [PATCH 02/18] protect settings page with sudo-mode middlewear --- .../SudoMode/SudoModeMiddlewear.coffee | 21 +++++++++++++++++++ services/web/app/coffee/router.coffee | 6 +++++- 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee new file mode 100644 index 0000000000..62516f0d34 --- /dev/null +++ b/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee @@ -0,0 +1,21 @@ +logger = require 'logger-sharelatex' +SudoModeHandler = require './SudoModeHandler' +AuthenticationController = require '../Authentication/AuthenticationController' + + +module.exports = SudoModeMiddlewear = + + protectPage: (req, res, next) -> + userId = AuthenticationController.getLoggedInUserId(req) + logger.log {userId}, "[SudoMode] protecting endpoint, checking if sudo mode is active" + SudoModeHandler.isSudoModeActive userId, (err, isActive) -> + if err? + logger.err {err, userId}, "[SudoMode] error checking if sudo mode is active" + return next(err) + if isActive + logger.log {userId}, "[SudoMode] sudo mode active, continuing" + return next() + else + logger.log {userId}, "[SudoMode] sudo mode not active, redirecting" + AuthenticationController._setRedirectInSession(req) + return res.redirect('/confirm-password') diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index ff5cd4b64c..5bbc416581 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -39,6 +39,7 @@ ReferencesController = require('./Features/References/ReferencesController') AuthorizationMiddlewear = require('./Features/Authorization/AuthorizationMiddlewear') BetaProgramController = require('./Features/BetaProgram/BetaProgramController') SudoModeController = require('./Features/SudoMode/SudoModeController') +SudoModeMiddlewear = require('./Features/SudoMode/SudoModeMiddlewear') AnalyticsRouter = require('./Features/Analytics/AnalyticsRouter') AnnouncementsController = require("./Features/Announcements/AnnouncementsController") @@ -86,7 +87,10 @@ module.exports = class Router webRouter.get '/user/activate', UserPagesController.activateAccountPage AuthenticationController.addEndpointToLoginWhitelist '/user/activate' - webRouter.get '/user/settings', AuthenticationController.requireLogin(), UserPagesController.settingsPage + webRouter.get '/user/settings', + AuthenticationController.requireLogin(), + SudoModeMiddlewear.protectPage, + UserPagesController.settingsPage webRouter.post '/user/settings', AuthenticationController.requireLogin(), UserController.updateUserSettings webRouter.post '/user/password/update', AuthenticationController.requireLogin(), UserController.changePassword From a3a21085260a3af6da85b3e03386c8ea3790b80a Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 11:35:47 +0100 Subject: [PATCH 03/18] Increase sudo-mode time to one hour --- .../web/app/coffee/Features/SudoMode/SudoModeHandler.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee index 398a39048c..67a10a81c9 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee @@ -3,7 +3,7 @@ rclient = RedisWrapper.client('sudomode') logger = require('logger-sharelatex') -TIMEOUT_IN_SECONDS = 60 * 10 +TIMEOUT_IN_SECONDS = 60 * 60 module.exports = SudoModeHandler = From 5a97521b0471f81a10f456af00ca9cd47457c076 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 11:36:05 +0100 Subject: [PATCH 04/18] Fix typo in log message --- .../web/app/coffee/Features/SudoMode/SudoModeHandler.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee index 67a10a81c9..5ae92b17a9 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee @@ -13,14 +13,14 @@ module.exports = SudoModeHandler = activateSudoMode: (userId, callback=(err)->) -> if !userId? - return callback(new Error('[SudoMode] user must not be supplied')) + return callback(new Error('[SudoMode] user must be supplied')) duration = TIMEOUT_IN_SECONDS logger.log {userId, duration}, "[SudoMode] activating sudo mode for user" rclient.set SudoModeHandler._buildKey(userId), '1', 'EX', duration, callback isSudoModeActive: (userId, callback=(err, isActive)->) -> if !userId? - return callback(new Error('[SudoMode] user must not be supplied')) + return callback(new Error('[SudoMode] user must be supplied')) rclient.get SudoModeHandler._buildKey(userId), (err, result) -> if err? return callback(err) From 16128288a91fdb46f1b601aefb49127ff63f1768 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 11:36:19 +0100 Subject: [PATCH 05/18] Add sudo-mode protection to sessions page --- services/web/app/coffee/router.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 5bbc416581..0a7a7bd69d 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -94,7 +94,10 @@ module.exports = class Router webRouter.post '/user/settings', AuthenticationController.requireLogin(), UserController.updateUserSettings webRouter.post '/user/password/update', AuthenticationController.requireLogin(), UserController.changePassword - webRouter.get '/user/sessions', AuthenticationController.requireLogin(), UserPagesController.sessionsPage + webRouter.get '/user/sessions', + AuthenticationController.requireLogin(), + SudoModeMiddlewear.protectPage, + UserPagesController.sessionsPage webRouter.post '/user/sessions/clear', AuthenticationController.requireLogin(), UserController.clearSessions webRouter.delete '/user/newsletter/unsubscribe', AuthenticationController.requireLogin(), UserController.unsubscribe From 08fd501ce2aa09e958d41be9fc78900df56841bc Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 11:36:35 +0100 Subject: [PATCH 06/18] Add a hint to the sudo-mode prompt --- services/web/app/views/sudo_mode/sudo_mode_prompt.pug | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug index 3b089517c1..9ad3003722 100644 --- a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug +++ b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug @@ -37,3 +37,9 @@ block content ng-disabled="confirmPassword.inflight" ) span #{translate("confirm")} + + .row + .col-md-8.col-md-offset-2 + p.text-centered + small + | Tip: You are entering "sudo mode", we won't ask for your password again for a while. From 993c261b10856192b01dfe653642755c05225471 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 11:37:32 +0100 Subject: [PATCH 07/18] start testing sudo-mode --- .../SudoMode/SudoModeHandlerTests.coffee | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee new file mode 100644 index 0000000000..994d793323 --- /dev/null +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee @@ -0,0 +1,110 @@ +SandboxedModule = require('sandboxed-module') +assert = require('assert') +require('chai').should() +expect = require('chai').expect +sinon = require('sinon') +modulePath = require('path').join __dirname, '../../../../app/js/Features/SudoMode/SudoModeHandler' + + +describe 'SudoModeHandler', -> + beforeEach -> + @userId = 'some_user_id' + @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()} + + describe '_buildKey', -> + + it 'should build a properly formed key', -> + expect(@SudoModeHandler._buildKey('123')).to.equal 'SudoMode:{123}' + + describe 'activateSudoMode', -> + beforeEach -> + @call = (cb) => + @SudoModeHandler.activateSudoMode @userId, cb + + describe 'when all goes well', -> + beforeEach -> + @rclient.set = sinon.stub().callsArgWith(4, null) + + it 'should not produce an error', (done) -> + @call (err) => + expect(err).to.equal null + done() + + it 'should set a value in redis', (done) -> + @call (err) => + expect(@rclient.set.callCount).to.equal 1 + expect(@rclient.set.calledWith( + 'SudoMode:{some_user_id}', '1', 'EX', 60*60 + )).to.equal true + done() + + describe 'when rclient.set produces an error', -> + beforeEach -> + @rclient.set = sinon.stub().callsArgWith(4, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + describe 'isSudoModeActive', -> + beforeEach -> + @call = (cb) => + @SudoModeHandler.isSudoModeActive @userId, cb + + describe 'when sudo-mode is active for that user', -> + beforeEach -> + @rclient.get = sinon.stub().callsArgWith(1, null, '1') + + it 'should not produce an error', (done) -> + @call (err, isActive) => + expect(err).to.equal null + done() + + it 'should get the value from redis', (done) -> + @call (err, isActive) => + expect(@rclient.get.callCount).to.equal 1 + expect(@rclient.get.calledWith('SudoMode:{some_user_id}')).to.equal true + done() + + it 'should produce a true result', (done) -> + @call (err, isActive) => + expect(isActive).to.equal true + done() + + describe 'when sudo-mode is not active for that user', -> + beforeEach -> + @rclient.get = sinon.stub().callsArgWith(1, null, null) + + it 'should not produce an error', (done) -> + @call (err, isActive) => + expect(err).to.equal null + done() + + it 'should get the value from redis', (done) -> + @call (err, isActive) => + expect(@rclient.get.callCount).to.equal 1 + expect(@rclient.get.calledWith('SudoMode:{some_user_id}')).to.equal true + done() + + it 'should produce a false result', (done) -> + @call (err, isActive) => + expect(isActive).to.equal false + done() + + describe 'when rclient.get produces an error', -> + beforeEach -> + @rclient.get = sinon.stub().callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, isActive) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + expect(isActive).to.be.oneOf [null, undefined] + done() From b09a41c557b3d3a2f0734375ed9381b6931540da Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 13:45:53 +0100 Subject: [PATCH 08/18] Fix acceptance tests --- .../acceptance/coffee/SessionTests.coffee | 53 ++++++++++++------- .../acceptance/coffee/helpers/User.coffee | 9 ++++ 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/services/web/test/acceptance/coffee/SessionTests.coffee b/services/web/test/acceptance/coffee/SessionTests.coffee index 56783b5b85..9343b0e544 100644 --- a/services/web/test/acceptance/coffee/SessionTests.coffee +++ b/services/web/test/acceptance/coffee/SessionTests.coffee @@ -34,9 +34,9 @@ describe "Sessions", -> expect(sessions[0].slice(0, 5)).to.equal 'sess:' next() - # should be able to access settings page + # should be able to access project list page , (next) => - @user1.getUserSettingsPage (err, statusCode) => + @user1.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 200 next() @@ -94,15 +94,15 @@ describe "Sessions", -> expect(sessions[1].slice(0, 5)).to.equal 'sess:' next() - # both should be able to access settings page + # both should be able to access project list page , (next) => - @user1.getUserSettingsPage (err, statusCode) => + @user1.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 200 next() , (next) => - @user2.getUserSettingsPage (err, statusCode) => + @user2.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 200 next() @@ -117,16 +117,16 @@ describe "Sessions", -> expect(sessions.length).to.equal 1 next() - # first session should not have access to settings page + # first session should not have access to project list page , (next) => - @user1.getUserSettingsPage (err, statusCode) => + @user1.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 302 next() # second session should still have access to settings , (next) => - @user2.getUserSettingsPage (err, statusCode) => + @user2.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 200 next() @@ -141,9 +141,9 @@ describe "Sessions", -> expect(sessions.length).to.equal 0 next() - # second session should not have access to settings page + # second session should not have access to project list page , (next) => - @user2.getUserSettingsPage (err, statusCode) => + @user2.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 302 next() @@ -216,22 +216,22 @@ describe "Sessions", -> expect(sessions.length).to.equal 1 next() - # users one and three should not be able to access settings page + # users one and three should not be able to access project list page , (next) => - @user1.getUserSettingsPage (err, statusCode) => + @user1.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 302 next() , (next) => - @user3.getUserSettingsPage (err, statusCode) => + @user3.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 302 next() - # user two should still be logged in, and able to access settings page + # user two should still be logged in, and able to access project list page , (next) => - @user2.getUserSettingsPage (err, statusCode) => + @user2.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 200 next() @@ -305,6 +305,19 @@ describe "Sessions", -> expect(sessions[1].slice(0, 5)).to.equal 'sess:' next() + # enter sudo-mode + , (next) => + @user2.getCsrfToken (err) => + expect(err).to.be.oneOf [null, undefined] + @user2.request.post { + uri: '/confirm-password/submit', + json: + password: @user2.password + }, (err, response, body) => + expect(err).to.be.oneOf [null, undefined] + expect(response.statusCode).to.equal 200 + next() + # check the sessions page , (next) => @user2.request.get { @@ -328,22 +341,22 @@ describe "Sessions", -> expect(sessions.length).to.equal 1 next() - # users one and three should not be able to access settings page + # users one and three should not be able to access project list page , (next) => - @user1.getUserSettingsPage (err, statusCode) => + @user1.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 302 next() , (next) => - @user3.getUserSettingsPage (err, statusCode) => + @user3.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 302 next() - # user two should still be logged in, and able to access settings page + # user two should still be logged in, and able to access project list page , (next) => - @user2.getUserSettingsPage (err, statusCode) => + @user2.getProjectListPage (err, statusCode) => expect(err).to.equal null expect(statusCode).to.equal 200 next() diff --git a/services/web/test/acceptance/coffee/helpers/User.coffee b/services/web/test/acceptance/coffee/helpers/User.coffee index eecde65322..da03cb9917 100644 --- a/services/web/test/acceptance/coffee/helpers/User.coffee +++ b/services/web/test/acceptance/coffee/helpers/User.coffee @@ -134,6 +134,15 @@ class User return callback(error) if error? callback(null, response.statusCode) + getProjectListPage: (callback=(error, statusCode)->) -> + @getCsrfToken (error) => + return callback(error) if error? + @request.get { + url: "/project" + }, (error, response, body) => + return callback(error) if error? + callback(null, response.statusCode) + module.exports = User From 4d662f23de2a9aedd328b17a6b641133649c6dd1 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 14:54:49 +0100 Subject: [PATCH 09/18] test SudoModeMiddlewear --- .../SudoMode/SudoModeMiddlewearTests.coffee | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee new file mode 100644 index 0000000000..0815428846 --- /dev/null +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee @@ -0,0 +1,102 @@ +SandboxedModule = require('sandboxed-module') +assert = require('assert') +require('chai').should() +expect = require('chai').expect +sinon = require('sinon') +modulePath = require('path').join __dirname, '../../../../app/js/Features/SudoMode/SudoModeMiddlewear' + + +describe 'SudoModeMiddlewear', -> + beforeEach -> + @userId = 'some_user_id' + @SudoModeHandler = + isSudoModeActive: sinon.stub() + @AuthenticationController = + getLoggedInUserId: sinon.stub().returns(@userId) + _setRedirectInSession: sinon.stub() + @SudoModeMiddlewear = SandboxedModule.require modulePath, requires: + './SudoModeHandler': @SudoModeHandler + '../Authentication/AuthenticationController': @AuthenticationController + 'logger-sharelatex': {log: sinon.stub(), err: sinon.stub()} + + describe 'protectPage', -> + beforeEach -> + @call = (cb) => + @req = {} + @res = {redirect: sinon.stub()} + @next = sinon.stub() + @SudoModeMiddlewear.protectPage @req, @res, @next + cb() + + describe 'when sudo mode is active', -> + beforeEach -> + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, null, true) + + it 'should get the current user id', (done) -> + @call () => + @AuthenticationController.getLoggedInUserId.callCount.should.equal 1 + done() + + it 'should check if sudo-mode is active', (done) -> + @call () => + @SudoModeHandler.isSudoModeActive.callCount.should.equal 1 + @SudoModeHandler.isSudoModeActive.calledWith(@userId).should.equal true + done() + + it 'should call next', (done) -> + @call () => + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.equal undefined + done() + + describe 'when sudo mode is not active', -> + beforeEach -> + @AuthenticationController._setRedirectInSession = sinon.stub() + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, null, false) + + it 'should get the current user id', (done) -> + @call () => + @AuthenticationController.getLoggedInUserId.callCount.should.equal 1 + done() + + it 'should check if sudo-mode is active', (done) -> + @call () => + @SudoModeHandler.isSudoModeActive.callCount.should.equal 1 + @SudoModeHandler.isSudoModeActive.calledWith(@userId).should.equal true + done() + + it 'should set redirect in session', (done) -> + @call () => + @AuthenticationController._setRedirectInSession.callCount.should.equal 1 + @AuthenticationController._setRedirectInSession.calledWith(@req).should.equal true + done() + + it 'should redirect to the password-prompt page', (done) -> + @call () => + @res.redirect.callCount.should.equal 1 + @res.redirect.calledWith('/confirm-password').should.equal true + done() + + describe 'when isSudoModeActive produces an error', -> + beforeEach -> + @AuthenticationController.getLoggedInUserId = sinon.stub().returns(@userId) + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, new Error('woops')) + + it 'should get the current user id', (done) -> + @call () => + @AuthenticationController.getLoggedInUserId.callCount.should.equal 1 + done() + + it 'should check if sudo-mode is active', (done) -> + @call () => + @SudoModeHandler.isSudoModeActive.callCount.should.equal 1 + @SudoModeHandler.isSudoModeActive.calledWith(@userId).should.equal true + done() + + it 'should call next with an error', (done) -> + @call () => + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error + done() From 27842996aa14618939acdd87f2abbdbe96bcdced Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 10 May 2017 15:15:57 +0100 Subject: [PATCH 10/18] start testing SudoModoController --- .../SudoMode/SudoModeController.coffee | 1 - .../SudoMode/SudoModeControllerTests.coffee | 20 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee index f569d3476e..1e5a9558df 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -1,4 +1,3 @@ -UserLocator = require "../User/UserLocator" Settings = require "settings-sharelatex" logger = require 'logger-sharelatex' SudoModeHandler = require './SudoModeHandler' diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee new file mode 100644 index 0000000000..09a9e3cf32 --- /dev/null +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee @@ -0,0 +1,20 @@ +SandboxedModule = require('sandboxed-module') +sinon = require 'sinon' +should = require("chai").should() +MockRequest = require "../helpers/MockRequest" +MockResponse = require "../helpers/MockResponse" +modulePath = '../../../../app/js/Features/SudoMode/SudoModeController' + +describe 'SudoModeController', -> + beforeEach -> + @user = + _id: 'abcd' + email: 'user@example.com' + @UserGetter = + getUser: sinon.stub().callsArgWith(2, null, @user) + @SudoModeHandler = + isSudoModeActive: sinon.stub() + activateSudoMode: sinon.stub() + + # describe '', -> + # beforeEach -> From 76285a15544cbba83db9474e3b1d228029526388 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 15 May 2017 10:07:22 +0100 Subject: [PATCH 11/18] Start testing SudoModeController --- .../SudoMode/SudoModeController.coffee | 1 - .../SudoMode/SudoModeControllerTests.coffee | 54 ++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee index 1e5a9558df..8938088cd1 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -1,4 +1,3 @@ -Settings = require "settings-sharelatex" logger = require 'logger-sharelatex' SudoModeHandler = require './SudoModeHandler' AuthenticationController = require '../Authentication/AuthenticationController' diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee index 09a9e3cf32..5f7683cab3 100644 --- a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee @@ -1,6 +1,7 @@ SandboxedModule = require('sandboxed-module') sinon = require 'sinon' should = require("chai").should() +expect = require('chai').expect MockRequest = require "../helpers/MockRequest" MockResponse = require "../helpers/MockResponse" modulePath = '../../../../app/js/Features/SudoMode/SudoModeController' @@ -15,6 +16,55 @@ describe 'SudoModeController', -> @SudoModeHandler = isSudoModeActive: sinon.stub() activateSudoMode: sinon.stub() + @AuthenticationController = + getLoggedInUserId: sinon.stub().returns(@user._id) + @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 - # describe '', -> - # beforeEach -> + describe 'sudoModePrompt', -> + beforeEach -> + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, null, false) + @req = {} + @res = {redirect: sinon.stub(), render: sinon.stub()} + @next = sinon.stub() + + it 'should get the logged in user id', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @AuthenticationController.getLoggedInUserId.callCount.should.equal 1 + @AuthenticationController.getLoggedInUserId.calledWith(@req).should.equal true + + it 'should check if sudo-mode is active', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @SudoModeHandler.isSudoModeActive.callCount.should.equal 1 + @SudoModeHandler.isSudoModeActive.calledWith(@user._id).should.equal true + + it 'should redirect when sudo-mode is active', -> + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, null, true) + @SudoModeController.sudoModePrompt(@req, @res, @next) + @res.redirect.callCount.should.equal 1 + @res.redirect.calledWith('/project').should.equal true + + it 'should render the sudo_mode_prompt page when sudo mode is not active', -> + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, null, false) + @SudoModeController.sudoModePrompt(@req, @res, @next) + @res.render.callCount.should.equal 1 + @res.render.calledWith('sudo_mode/sudo_mode_prompt').should.equal true + + describe 'when isSudoModeActive produces an error', -> + beforeEach -> + @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, new Error('woops')) + @next = sinon.stub() + + it 'should call next with an error', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error From 0f75d9f4d9aaea2666b551fb795ca00695ed837a Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 15 May 2017 10:56:43 +0100 Subject: [PATCH 12/18] Test SudoModeController --- .../SudoMode/SudoModeController.coffee | 4 + .../SudoMode/SudoModeControllerTests.coffee | 206 ++++++++++++++++++ 2 files changed, 210 insertions(+) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee index 8938088cd1..aca5315cbb 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -32,6 +32,10 @@ module.exports = SudoModeController = if err? logger.err {err, userId}, "[SudoMode] error getting user" return next(err) + if !userRecord? + 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) -> if err? logger.err {err, userId}, "[SudoMode] error authenticating user" diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee index 5f7683cab3..1870ba1d76 100644 --- a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee @@ -18,6 +18,7 @@ describe 'SudoModeController', -> activateSudoMode: sinon.stub() @AuthenticationController = getLoggedInUserId: sinon.stub().returns(@user._id) + _getRediretFromSession: sinon.stub() @AuthenticationManager = authenticate: sinon.stub() @UserGetter = @@ -68,3 +69,208 @@ describe 'SudoModeController', -> @SudoModeController.sudoModePrompt(@req, @res, @next) @next.callCount.should.equal 1 expect(@next.lastCall.args[0]).to.be.instanceof Error + + describe 'submitPassword', -> + beforeEach -> + @AuthenticationController._getRedirectFromSession = sinon.stub().returns '/somewhere' + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user) + @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user) + @SudoModeHandler.activateSudoMode = sinon.stub().callsArgWith(1, null) + @password = 'a_terrible_secret' + @req = {body: {password: @password}} + @res = {json: sinon.stub()} + @next = sinon.stub() + + describe 'when all goes well', -> + beforeEach -> + + it 'should get the logged in user id', -> + @SudoModeController.submitPassword(@req, @res, @next) + @AuthenticationController.getLoggedInUserId.callCount.should.equal 1 + @AuthenticationController.getLoggedInUserId.calledWith(@req).should.equal true + + it 'should get redirect from session', -> + @SudoModeController.submitPassword(@req, @res, @next) + @AuthenticationController._getRedirectFromSession.callCount.should.equal 1 + @AuthenticationController._getRedirectFromSession.calledWith(@req).should.equal true + + it 'should get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 1 + @UserGetter.getUser.calledWith('some_object_id', {email: 1}).should.equal true + + 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 + + it 'should activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 1 + @SudoModeHandler.activateSudoMode.calledWith(@user._id).should.equal true + + it 'should send back a json response', -> + @SudoModeController.submitPassword(@req, @res, @next) + @res.json.callCount.should.equal 1 + @res.json.calledWith({redir: '/somewhere'}).should.equal true + + it 'should not call next', -> + @SudoModeController.submitPassword(@req, @res, @next) + @next.callCount.should.equal 0 + + describe 'when no password is supplied', -> + beforeEach -> + @req.body.password = '' + @next = sinon.stub() + + it 'should return next with an error', -> + @SudoModeController.submitPassword(@req, @res, @next) + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error + + it 'should not get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 0 + + it 'should not try to authenticate the user with the password', -> + @SudoModeController.submitPassword(@req, @res, @next) + @AuthenticationManager.authenticate.callCount.should.equal 0 + + it 'should not activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 0 + + it 'should not send back a json response', -> + @SudoModeController.submitPassword(@req, @res, @next) + @res.json.callCount.should.equal 0 + + describe 'when getUser produces an error', -> + beforeEach -> + @UserGetter.getUser = sinon.stub().callsArgWith(2, new Error('woops')) + @next = sinon.stub() + + it 'should return next with an error', -> + @SudoModeController.submitPassword(@req, @res, @next) + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error + + it 'should get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 1 + @UserGetter.getUser.calledWith('some_object_id', {email: 1}).should.equal true + + it 'should not try to authenticate the user with the password', -> + @SudoModeController.submitPassword(@req, @res, @next) + @AuthenticationManager.authenticate.callCount.should.equal 0 + + it 'should not activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 0 + + it 'should not send back a json response', -> + @SudoModeController.submitPassword(@req, @res, @next) + @res.json.callCount.should.equal 0 + + describe 'when getUser does not find a user', -> + beforeEach -> + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, null) + @next = sinon.stub() + + it 'should return next with an error', -> + @SudoModeController.submitPassword(@req, @res, @next) + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error + + it 'should get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 1 + @UserGetter.getUser.calledWith('some_object_id', {email: 1}).should.equal true + + it 'should not try to authenticate the user with the password', -> + @SudoModeController.submitPassword(@req, @res, @next) + @AuthenticationManager.authenticate.callCount.should.equal 0 + + it 'should not activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 0 + + it 'should not send back a json response', -> + @SudoModeController.submitPassword(@req, @res, @next) + @res.json.callCount.should.equal 0 + + describe 'when authentication fails', -> + beforeEach -> + @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, null) + @res.json = sinon.stub() + @req.i18n = {translate: sinon.stub()} + + it 'should send back a failure message', -> + @SudoModeController.submitPassword(@req, @res, @next) + @res.json.callCount.should.equal 1 + expect(@res.json.lastCall.args[0]).to.have.keys ['message'] + expect(@res.json.lastCall.args[0].message).to.have.keys ['text', 'type'] + @req.i18n.translate.callCount.should.equal 1 + @req.i18n.translate.calledWith('invalid_password') + + it 'should get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 1 + @UserGetter.getUser.calledWith('some_object_id', {email: 1}).should.equal true + + 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 + + it 'should not activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 0 + + describe 'when authentication produces an error', -> + beforeEach -> + @AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, new Error('woops')) + @next = sinon.stub() + + it 'should return next with an error', -> + @SudoModeController.submitPassword(@req, @res, @next) + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error + + it 'should get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 1 + @UserGetter.getUser.calledWith('some_object_id', {email: 1}).should.equal true + + 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 + + it 'should not activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 0 + + describe 'when sudo mode activation produces an error', -> + beforeEach -> + @SudoModeHandler.activateSudoMode = sinon.stub().callsArgWith(1, new Error('woops')) + @next = sinon.stub() + + it 'should return next with an error', -> + @SudoModeController.submitPassword(@req, @res, @next) + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.be.instanceof Error + + it 'should get the user from storage', -> + @SudoModeController.submitPassword(@req, @res, @next) + @UserGetter.getUser.callCount.should.equal 1 + @UserGetter.getUser.calledWith('some_object_id', {email: 1}).should.equal true + + 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 + + it 'should have tried to activate sudo mode', -> + @SudoModeController.submitPassword(@req, @res, @next) + @SudoModeHandler.activateSudoMode.callCount.should.equal 1 + @SudoModeHandler.activateSudoMode.calledWith(@user._id).should.equal true From 17b064df1868d368c62725f9df3c7600cee54a47 Mon Sep 17 00:00:00 2001 From: James Allen Date: Mon, 15 May 2017 11:45:41 +0100 Subject: [PATCH 13/18] Adjust style of sudo mode prompt --- .../web/app/views/sudo_mode/sudo_mode_prompt.pug | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug index 9ad3003722..b88074002a 100644 --- a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug +++ b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug @@ -4,14 +4,14 @@ block content .content.content-alt .container .row - .col-md-10.col-md-offset-1.col-lg-8.col-lg-offset-2 + .col-md-8.col-md-offset-2.col-lg-6.col-lg-offset-3 .card .page-header.text-centered - h1 Confirm your password to continue + h2 Confirm password to continue div .container-fluid .row - .col-md-8.col-md-offset-2 + .col-md-12 form(async-form="confirmPassword", name="confirmPassword", action='/confirm-password/submit', method="POST", ng-cloak) input(name='_csrf', type='hidden', value=csrfToken) @@ -38,8 +38,8 @@ block content ) span #{translate("confirm")} - .row - .col-md-8.col-md-offset-2 + .row.row-spaced-small + .col-md-12 p.text-centered small - | Tip: You are entering "sudo mode", we won't ask for your password again for a while. + | We won't ask for your password again for a while. From c864288c4ec1daefa13890c9dd545dfb3b7ab91d Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 15 May 2017 11:53:52 +0100 Subject: [PATCH 14/18] On logout, clear sudo mode --- .../Features/SudoMode/SudoModeHandler.coffee | 6 ++ .../Features/User/UserController.coffee | 2 + .../SudoMode/SudoModeHandlerTests.coffee | 78 +++++++++++++++++++ .../coffee/User/UserControllerTests.coffee | 14 ++++ 4 files changed, 100 insertions(+) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee index 5ae92b17a9..004b63ccc2 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeHandler.coffee @@ -18,6 +18,12 @@ module.exports = SudoModeHandler = logger.log {userId, duration}, "[SudoMode] activating sudo mode for user" rclient.set SudoModeHandler._buildKey(userId), '1', 'EX', duration, callback + clearSudoMode: (userId, callback=(err)->) -> + if !userId? + return callback(new Error('[SudoMode] user must be supplied')) + logger.log {userId}, "[SudoMode] clearing sudo mode for user" + rclient.del SudoModeHandler._buildKey(userId), callback + isSudoModeActive: (userId, callback=(err, isActive)->) -> if !userId? return callback(new Error('[SudoMode] user must be supplied')) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 50b02fc918..65000985e1 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -11,6 +11,7 @@ AuthenticationManager = require("../Authentication/AuthenticationManager") AuthenticationController = require('../Authentication/AuthenticationController') UserSessionsManager = require("./UserSessionsManager") UserUpdater = require("./UserUpdater") +SudoModeHandler = require('../SudoMode/SudoModeHandler') settings = require "settings-sharelatex" module.exports = UserController = @@ -118,6 +119,7 @@ module.exports = UserController = if err logger.err err: err, 'error destorying session' UserSessionsManager.untrackSession(user, sessionId) + SudoModeHandler.clearSudoMode(user._id) res.redirect '/login' register : (req, res, next = (error) ->)-> diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee index 994d793323..e7f3ccd150 100644 --- a/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeHandlerTests.coffee @@ -30,6 +30,7 @@ describe 'SudoModeHandler', -> beforeEach -> @rclient.set = sinon.stub().callsArgWith(4, null) + it 'should not produce an error', (done) -> @call (err) => expect(err).to.equal null @@ -43,6 +44,22 @@ describe 'SudoModeHandler', -> )).to.equal true done() + describe 'when user id is not supplied', -> + beforeEach -> + @call = (cb) => + @SudoModeHandler.activateSudoMode null, cb + + it 'should produce an error', (done) -> + @call (err) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + it 'should not set value in redis', (done) -> + @call (err) => + expect(@rclient.set.callCount).to.equal 0 + done() + describe 'when rclient.set produces an error', -> beforeEach -> @rclient.set = sinon.stub().callsArgWith(4, new Error('woops')) @@ -53,6 +70,51 @@ describe 'SudoModeHandler', -> expect(err).to.be.instanceof Error done() + describe 'clearSudoMode', -> + beforeEach -> + @rclient.del = sinon.stub().callsArgWith(1, null) + @call = (cb) => + @SudoModeHandler.clearSudoMode @userId, cb + + it 'should not produce an error', (done) -> + @call (err) => + expect(err).to.equal null + done() + + it 'should delete key from redis', (done) -> + @call (err) => + expect(@rclient.del.callCount).to.equal 1 + expect(@rclient.del.calledWith( + 'SudoMode:{some_user_id}' + )).to.equal true + done() + + describe 'when rclient.del produces an error', -> + beforeEach -> + @rclient.del = sinon.stub().callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + describe 'when user id is not supplied', -> + beforeEach -> + @call = (cb) => + @SudoModeHandler.clearSudoMode null, cb + + it 'should produce an error', (done) -> + @call (err) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + it 'should not delete value in redis', (done) -> + @call (err) => + expect(@rclient.del.callCount).to.equal 0 + done() + describe 'isSudoModeActive', -> beforeEach -> @call = (cb) => @@ -108,3 +170,19 @@ describe 'SudoModeHandler', -> expect(err).to.be.instanceof Error expect(isActive).to.be.oneOf [null, undefined] done() + + describe 'when user id is not supplied', -> + beforeEach -> + @call = (cb) => + @SudoModeHandler.isSudoModeActive null, cb + + it 'should produce an error', (done) -> + @call (err) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + it 'should not get value in redis', (done) -> + @call (err) => + expect(@rclient.get.callCount).to.equal 0 + done() diff --git a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee index ecb33495c4..a71fe4bb91 100644 --- a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee @@ -60,6 +60,8 @@ describe "UserController", -> trackSession: sinon.stub() untrackSession: sinon.stub() revokeAllUserSessions: sinon.stub().callsArgWith(2, null) + @SudoModeHandler = + clearSudoMode: sinon.stub() @UserController = SandboxedModule.require modulePath, requires: "./UserLocator": @UserLocator "./UserDeleter": @UserDeleter @@ -73,6 +75,7 @@ describe "UserController", -> "../Subscription/SubscriptionDomainHandler":@SubscriptionDomainHandler "./UserHandler":@UserHandler "./UserSessionsManager": @UserSessionsManager + "../SudoMode/SudoModeHandler": @SudoModeHandler "settings-sharelatex": @settings "logger-sharelatex": log:-> @@ -302,6 +305,17 @@ describe "UserController", -> @UserController.logout @req, @res + it 'should clear sudo-mode', (done) -> + @req.session.destroy = sinon.stub().callsArgWith(0) + @SudoModeHandler.clearSudoMode = sinon.stub() + @res.redirect = (url)=> + url.should.equal "/login" + @SudoModeHandler.clearSudoMode.callCount.should.equal 1 + @SudoModeHandler.clearSudoMode.calledWith(@user._id).should.equal true + done() + + @UserController.logout @req, @res + describe "register", -> beforeEach -> From 1d7d0193fcb63167b2ee077ac779c3bce881d3dc Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 15 May 2017 13:37:54 +0100 Subject: [PATCH 15/18] Use translations for sudo-mode prompt --- services/web/app/views/sudo_mode/sudo_mode_prompt.pug | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug index b88074002a..55af6255ff 100644 --- a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug +++ b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug @@ -7,7 +7,7 @@ block content .col-md-8.col-md-offset-2.col-lg-6.col-lg-offset-3 .card .page-header.text-centered - h2 Confirm password to continue + h2 #{translate('confirm_password_to_continue')} div .container-fluid .row @@ -42,4 +42,4 @@ block content .col-md-12 p.text-centered small - | We won't ask for your password again for a while. + | #{translate('confirm_password_footer')} From 707a81cc2a6d1a9214f2065ed1b8719643f66826 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 15 May 2017 15:46:11 +0100 Subject: [PATCH 16/18] Correct title of confirm-password page --- .../web/app/coffee/Features/SudoMode/SudoModeController.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee index aca5315cbb..1e3e9592b6 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -18,7 +18,7 @@ module.exports = SudoModeController = if isActive logger.log {userId}, "[SudoMode] sudo mode already active, redirecting" return res.redirect('/project') - res.render 'sudo_mode/sudo_mode_prompt', title: 'confirm_your_password' + res.render 'sudo_mode/sudo_mode_prompt', title: 'confirm_password_to_continue' submitPassword: (req, res, next) -> userId = AuthenticationController.getLoggedInUserId(req) From 60d3e4a97b27960a2322e05c016126aeeac197cd Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 15 May 2017 15:46:24 +0100 Subject: [PATCH 17/18] If external auth system is in use, skip sudo-mode checks --- .../SudoMode/SudoModeController.coffee | 3 +++ .../SudoMode/SudoModeMiddlewear.coffee | 3 +++ .../infrastructure/ExpressLocals.coffee | 10 ++++---- .../SudoMode/SudoModeControllerTests.coffee | 23 ++++++++++++++++++- .../SudoMode/SudoModeMiddlewearTests.coffee | 23 ++++++++++++++++++- 5 files changed, 55 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee index 1e3e9592b6..da31522a6c 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeController.coffee @@ -9,6 +9,9 @@ UserGetter = require '../User/UserGetter' module.exports = SudoModeController = sudoModePrompt: (req, res, next) -> + if req.externalAuthenticationSystemUsed() + logger.log {userId}, "[SudoMode] using external auth, redirecting" + return res.redirect('/project') userId = AuthenticationController.getLoggedInUserId(req) logger.log {userId}, "[SudoMode] rendering sudo mode password page" SudoModeHandler.isSudoModeActive userId, (err, isActive) -> diff --git a/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee b/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee index 62516f0d34..479d67eee0 100644 --- a/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee +++ b/services/web/app/coffee/Features/SudoMode/SudoModeMiddlewear.coffee @@ -6,6 +6,9 @@ AuthenticationController = require '../Authentication/AuthenticationController' module.exports = SudoModeMiddlewear = protectPage: (req, res, next) -> + if req.externalAuthenticationSystemUsed() + logger.log {userId}, "[SudoMode] using external auth, skipping sudo-mode check" + return next() userId = AuthenticationController.getLoggedInUserId(req) logger.log {userId}, "[SudoMode] protecting endpoint, checking if sudo mode is active" SudoModeHandler.isSudoModeActive userId, (err, isActive) -> diff --git a/services/web/app/coffee/infrastructure/ExpressLocals.coffee b/services/web/app/coffee/infrastructure/ExpressLocals.coffee index ba753cc86e..100fd28bd6 100644 --- a/services/web/app/coffee/infrastructure/ExpressLocals.coffee +++ b/services/web/app/coffee/infrastructure/ExpressLocals.coffee @@ -84,6 +84,11 @@ module.exports = (app, webRouter, apiRouter)-> webRouter.use addSetContentDisposition apiRouter.use addSetContentDisposition + webRouter.use (req, res, next)-> + req.externalAuthenticationSystemUsed = res.locals.externalAuthenticationSystemUsed = -> + Settings.ldap? or Settings.saml? + next() + webRouter.use (req, res, next)-> cdnBlocked = req.query.nocdn == 'true' or req.session.cdnBlocked @@ -222,11 +227,6 @@ module.exports = (app, webRouter, apiRouter)-> res.locals.formatPrice = SubscriptionFormatters.formatPrice next() - webRouter.use (req, res, next)-> - res.locals.externalAuthenticationSystemUsed = -> - Settings.ldap? or Settings.saml? - next() - webRouter.use (req, res, next)-> currentUser = AuthenticationController.getSessionUser(req) if currentUser? diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee index 1870ba1d76..c1de2e278c 100644 --- a/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeControllerTests.coffee @@ -34,7 +34,7 @@ describe 'SudoModeController', -> describe 'sudoModePrompt', -> beforeEach -> @SudoModeHandler.isSudoModeActive = sinon.stub().callsArgWith(1, null, false) - @req = {} + @req = {externalAuthenticationSystemUsed: sinon.stub().returns(false)} @res = {redirect: sinon.stub(), render: sinon.stub()} @next = sinon.stub() @@ -70,6 +70,27 @@ describe 'SudoModeController', -> @next.callCount.should.equal 1 expect(@next.lastCall.args[0]).to.be.instanceof Error + it 'should not render page', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @res.render.callCount.should.equal 0 + + describe 'when external auth system is used', -> + beforeEach -> + @req.externalAuthenticationSystemUsed = sinon.stub().returns(true) + + it 'should redirect', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @res.redirect.callCount.should.equal 1 + @res.redirect.calledWith('/project').should.equal true + + it 'should not check if sudo mode is active', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @SudoModeHandler.isSudoModeActive.callCount.should.equal 0 + + it 'should not render page', -> + @SudoModeController.sudoModePrompt(@req, @res, @next) + @res.render.callCount.should.equal 0 + describe 'submitPassword', -> beforeEach -> @AuthenticationController._getRedirectFromSession = sinon.stub().returns '/somewhere' diff --git a/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee b/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee index 0815428846..00e8d0ae57 100644 --- a/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee +++ b/services/web/test/UnitTests/coffee/SudoMode/SudoModeMiddlewearTests.coffee @@ -21,8 +21,9 @@ describe 'SudoModeMiddlewear', -> describe 'protectPage', -> beforeEach -> + @externalAuth = false @call = (cb) => - @req = {} + @req = {externalAuthenticationSystemUsed: sinon.stub().returns(@externalAuth)} @res = {redirect: sinon.stub()} @next = sinon.stub() @SudoModeMiddlewear.protectPage @req, @res, @next @@ -100,3 +101,23 @@ describe 'SudoModeMiddlewear', -> @next.callCount.should.equal 1 expect(@next.lastCall.args[0]).to.be.instanceof Error done() + + describe 'when external auth is being used', -> + beforeEach -> + @externalAuth = true + + it 'should immediately return next with no args', (done) -> + @call () => + @next.callCount.should.equal 1 + expect(@next.lastCall.args[0]).to.not.exist + done() + + it 'should not get the current user id', (done) -> + @call () => + @AuthenticationController.getLoggedInUserId.callCount.should.equal 0 + done() + + it 'should not check if sudo-mode is active', (done) -> + @call () => + @SudoModeHandler.isSudoModeActive.callCount.should.equal 0 + done() From 25e0a1935078cc2f74344db5862881467cde1f5f Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Tue, 16 May 2017 11:51:06 +0100 Subject: [PATCH 18/18] Make confirm-password routes more restful --- services/web/app/coffee/router.coffee | 2 +- services/web/app/views/sudo_mode/sudo_mode_prompt.pug | 2 +- services/web/test/acceptance/coffee/SessionTests.coffee | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 1b237e409e..549cc50e81 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -248,7 +248,7 @@ module.exports = class Router webRouter.post "/beta/opt-in", AuthenticationController.requireLogin(), BetaProgramController.optIn webRouter.post "/beta/opt-out", AuthenticationController.requireLogin(), BetaProgramController.optOut webRouter.get "/confirm-password", AuthenticationController.requireLogin(), SudoModeController.sudoModePrompt - webRouter.post "/confirm-password/submit", AuthenticationController.requireLogin(), SudoModeController.submitPassword + webRouter.post "/confirm-password", AuthenticationController.requireLogin(), SudoModeController.submitPassword #Admin Stuff diff --git a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug index 55af6255ff..27ccc41eb6 100644 --- a/services/web/app/views/sudo_mode/sudo_mode_prompt.pug +++ b/services/web/app/views/sudo_mode/sudo_mode_prompt.pug @@ -13,7 +13,7 @@ block content .row .col-md-12 form(async-form="confirmPassword", name="confirmPassword", - action='/confirm-password/submit', method="POST", ng-cloak) + action='/confirm-password', method="POST", ng-cloak) input(name='_csrf', type='hidden', value=csrfToken) form-messages(for="confirmPassword") .form-group diff --git a/services/web/test/acceptance/coffee/SessionTests.coffee b/services/web/test/acceptance/coffee/SessionTests.coffee index 9343b0e544..07a431a229 100644 --- a/services/web/test/acceptance/coffee/SessionTests.coffee +++ b/services/web/test/acceptance/coffee/SessionTests.coffee @@ -310,7 +310,7 @@ describe "Sessions", -> @user2.getCsrfToken (err) => expect(err).to.be.oneOf [null, undefined] @user2.request.post { - uri: '/confirm-password/submit', + uri: '/confirm-password', json: password: @user2.password }, (err, response, body) =>