diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 32d6fa79cb..7271e72864 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -106,6 +106,14 @@ module.exports = UserController = setNewPasswordUrl: setNewPasswordUrl } + clearSessions: (req, res, next = (error) ->) -> + metrics.inc "user.clear-sessions" + user = AuthenticationController.getSessionUser(req) + logger.log {user_id: user._id}, "clearing sessions for user" + UserSessionsManager.revokeAllUserSessions user, [req.sessionID], (err) -> + return next(err) if err? + res.sendStatus 201 + changePassword : (req, res, next = (error) ->)-> metrics.inc "user.password-change" oldPass = req.body.currentPassword diff --git a/services/web/app/coffee/Features/User/UserPagesController.coffee b/services/web/app/coffee/Features/User/UserPagesController.coffee index e862b3c0af..76d88803a7 100644 --- a/services/web/app/coffee/Features/User/UserPagesController.coffee +++ b/services/web/app/coffee/Features/User/UserPagesController.coffee @@ -66,9 +66,12 @@ module.exports = accountSettingsTabActive: true sessionsPage: (req, res, next) -> - user_id = AuthenticationController.getLoggedInUserId(req) - logger.log user: user_id, "loading settings page" - UserSessionsManager.getAllUserSessions user_id, (err, sessions) -> + user = AuthenticationController.getSessionUser(req) + logger.log user_id: user._id, "loading sessions page" + UserSessionsManager.getAllUserSessions user, [req.sessionID], (err, sessions) -> + if err? + logger.err {user_id: user._id}, "error getting all user sessions" + return next(err) res.render 'user/sessions', title: "sessions" sessions: sessions diff --git a/services/web/app/coffee/Features/User/UserSessionsManager.coffee b/services/web/app/coffee/Features/User/UserSessionsManager.coffee index e55b7a6a5a..8e44df3d21 100644 --- a/services/web/app/coffee/Features/User/UserSessionsManager.coffee +++ b/services/web/app/coffee/Features/User/UserSessionsManager.coffee @@ -56,29 +56,33 @@ module.exports = UserSessionsManager = UserSessionsManager._checkSessions(user, () ->) callback() - getAllUserSessions: (user_id, callback=(err, sessionKeys)->) -> - sessionSetKey = UserSessionsManager._sessionSetKey({_id: user_id}) + getAllUserSessions: (user, exclude, callback=(err, sessionKeys)->) -> + exclude = _.map(exclude, UserSessionsManager._sessionKey) + sessionSetKey = UserSessionsManager._sessionSetKey(user) rclient.smembers sessionSetKey, (err, sessionKeys) -> if err? - logger.err {user_id}, "error getting all session keys for user from redis" + logger.err user_id: user._id, "error getting all session keys for user from redis" return callback(err) + sessionKeys = _.filter sessionKeys, (k) -> !(_.contains(exclude, k)) + if sessionKeys.length == 0 + logger.log {user_id: user._id}, "no other sessions found, returning" + return callback(null, []) rclient.mget sessionKeys, (err, sessions) -> if err? - logger.err {user_id}, "error getting all sessions for user from redis" + logger.err {user_id: user._id}, "error getting all sessions for user from redis" return callback(err) - hashedSessionKeys = sessionKeys.map (key) -> - crypto.createHash('md5').update(key).digest('hex') - expiries = sessions.map (s) -> - if s == null - return null - s = JSON.parse(s) - s?.user?.session_created or s?.passport?.user?.session_created - pairs = _.zip(hashedSessionKeys, expiries) result = [] - for pair in pairs - result.push {id: pair[0], expires: pair[1]} - console.log ">> result:", result + for session in sessions + if session is null + continue + session = JSON.parse(session) + session_user = session?.user or session?.passport?.user + result.push { + ip_address: session_user.ip_address, + session_created: session_user.session_created + } + return callback(null, result) revokeAllUserSessions: (user, retain, callback=(err)->) -> diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 3a07280723..56dd8d821b 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -88,7 +88,8 @@ 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(), UserPagesController.sessionsPage + webRouter.post '/user/sessions/clear', AuthenticationController.requireLogin(), UserController.clearSessions webRouter.delete '/user/newsletter/unsubscribe', AuthenticationController.requireLogin(), UserController.unsubscribe webRouter.delete '/user', AuthenticationController.requireLogin(), UserController.deleteUser diff --git a/services/web/app/views/user/sessions.jade b/services/web/app/views/user/sessions.jade index a1ab8fac0e..948ed2d94f 100644 --- a/services/web/app/views/user/sessions.jade +++ b/services/web/app/views/user/sessions.jade @@ -1,14 +1,49 @@ extends ../layout + +block scripts + script(type='text/javascript'). + window.otherSessions = !{JSON.stringify(sessions)} + + block content .content.content-alt .container .row .col-md-10.col-md-offset-1.col-lg-8.col-lg-offset-2 - .card + .card.clear-user-sessions(ng-controller="ClearSessionsController", ng-cloak) .page-header h1 #{translate("your_sessions")} - .sessions-list - each session in sessions - div.session - | #{session.id} - #{session.expires} \ No newline at end of file + + div + p.small + | #{translate("clear_sessions_description")} + + div + div(ng-if="state.otherSessions.length == 0") + p.text-center + | #{translate("no_other_sessions")} + + div(ng-if="state.success == true") + p.text-success.text-center + | #{translate('clear_sessions_success')} + + div(ng-if="state.otherSessions.length != 0") + table.table.table-striped + thead + tr + th #{translate("ip_address")} + th #{translate("session_created_at")} + tr(ng-repeat="session in state.otherSessions") + td {{session.ip_address}} + td {{session.session_created | formatDate}} + + p.actions + .text-center + button.btn.btn-lg.btn-primary( + ng-click="clearSessions()" + ) #{translate('clear_sessions')} + + div(ng-if="state.error == true") + p.text-danger.error + | #{translate('generic_something_went_wrong')} diff --git a/services/web/app/views/user/settings.jade b/services/web/app/views/user/settings.jade index 44028f1c54..2e17eb1888 100644 --- a/services/web/app/views/user/settings.jade +++ b/services/web/app/views/user/settings.jade @@ -114,6 +114,14 @@ block content a(id="beta-program-participate-link" href="/beta/participate") #{translate("manage_beta_program_membership")} hr + + h3 + | #{translate("sessions")} + + div + a(id="sessions-link", href="/user/sessions") #{translate("manage_sessions")} + + hr if !externalAuthenticationSystemUsed() diff --git a/services/web/public/coffee/main.coffee b/services/web/public/coffee/main.coffee index c723031016..60cc38ae6a 100644 --- a/services/web/public/coffee/main.coffee +++ b/services/web/public/coffee/main.coffee @@ -2,6 +2,7 @@ define [ "main/project-list/index" "main/user-details" "main/account-settings" + "main/clear-sessions" "main/account-upgrade" "main/plans" "main/group-members" @@ -32,6 +33,3 @@ define [ "__MAIN_CLIENTSIDE_INCLUDES__" ], () -> angular.bootstrap(document.body, ["SharelatexApp"]) - - - diff --git a/services/web/public/coffee/main/clear-sessions.coffee b/services/web/public/coffee/main/clear-sessions.coffee new file mode 100644 index 0000000000..5524ff8d89 --- /dev/null +++ b/services/web/public/coffee/main/clear-sessions.coffee @@ -0,0 +1,20 @@ +define [ + "base" +], (App) -> + App.controller "ClearSessionsController", ["$scope", "$http", ($scope, $http) -> + + $scope.state = + otherSessions: window.otherSessions + error: false + success: false + + $scope.clearSessions = () -> + console.log ">> clearing all sessions" + $http({method: 'POST', url: "/user/sessions/clear", headers: {'X-CSRF-Token': window.csrfToken}}) + .success () -> + $scope.state.otherSessions = [] + $scope.state.error = false + $scope.state.success = true + .error () -> + $scope.state.error = true + ] diff --git a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee index 38ca579acd..a9c98e02ec 100644 --- a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee @@ -81,6 +81,7 @@ describe "UserController", -> @res = send: sinon.stub() + sendStatus: sinon.stub() json: sinon.stub() @next = sinon.stub() describe "deleteUser", -> @@ -224,6 +225,29 @@ describe "UserController", -> }) .should.equal true + describe 'clearSessions', -> + + it 'should call revokeAllUserSessions', (done) -> + @UserController.clearSessions @req, @res + @UserSessionsManager.revokeAllUserSessions.callCount.should.equal 1 + done() + + it 'send a 201 response', (done) -> + @res.sendStatus = (status) => + status.should.equal 201 + done() + @UserController.clearSessions @req, @res + + describe 'when revokeAllUserSessions produces an error', -> + + it 'should call next with an error', (done) -> + @UserSessionsManager.revokeAllUserSessions.callsArgWith(2, new Error('woops')) + next = (err) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + @UserController.clearSessions @req, @res, next + describe "changePassword", -> it "should check the old password is the current one at the moment", (done)-> diff --git a/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee index 0df1d65fe6..bb9fa22a15 100644 --- a/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee @@ -20,6 +20,8 @@ describe "UserPagesController", -> findById: sinon.stub().callsArgWith(1, null, @user) @UserGetter = getUser: sinon.stub().callsArgWith(2, null, @user) + @UserSessionsManager = + getAllUserSessions: sinon.stub() @dropboxStatus = {} @DropboxHandler = getUserRegistrationStatus : sinon.stub().callsArgWith(1, null, @dropboxStatus) @@ -27,11 +29,15 @@ describe "UserPagesController", -> notFound: sinon.stub() @AuthenticationController = getLoggedInUserId: sinon.stub().returns(@user._id) + getSessionUser: sinon.stub().returns(@user) @UserPagesController = SandboxedModule.require modulePath, requires: "settings-sharelatex":@settings - "logger-sharelatex": log:-> + "logger-sharelatex": + log:-> + err:-> "./UserLocator": @UserLocator "./UserGetter": @UserGetter + "./UserSessionsManager": @UserSessionsManager "../Errors/ErrorController": @ErrorController '../Dropbox/DropboxHandler': @DropboxHandler '../Authentication/AuthenticationController': @AuthenticationController @@ -100,6 +106,34 @@ describe "UserPagesController", -> done() @UserPagesController.loginPage @req, @res + describe 'sessionsPage', -> + + beforeEach -> + @UserSessionsManager.getAllUserSessions.callsArgWith(2, null, []) + + it 'should render user/sessions', (done) -> + @res.render = (page)-> + page.should.equal "user/sessions" + done() + @UserPagesController.sessionsPage @req, @res + + it 'should have called getAllUserSessions', (done) -> + @res.render = (page) => + @UserSessionsManager.getAllUserSessions.callCount.should.equal 1 + done() + @UserPagesController.sessionsPage @req, @res + + describe 'when getAllUserSessions produces an error', -> + + beforeEach -> + @UserSessionsManager.getAllUserSessions.callsArgWith(2, new Error('woops')) + + it 'should call next with an error', (done) -> + @next = (err) => + assert(err != null) + assert(err instanceof Error) + done() + @UserPagesController.sessionsPage @req, @res, @next describe "settingsPage", -> diff --git a/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee b/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee index 9a09aa88f0..f9bf846e54 100644 --- a/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee @@ -21,6 +21,7 @@ describe 'UserSessionsManager', -> sadd: sinon.stub() srem: sinon.stub() smembers: sinon.stub() + mget: sinon.stub() expire: sinon.stub() @rclient.multi.returns(@rclient) @rclient.get.returns(@rclient) @@ -404,6 +405,97 @@ describe 'UserSessionsManager', -> @rclient.expire.callCount.should.equal 0 done() + describe 'getAllUserSessions', -> + + beforeEach -> + @sessionKeys = ['sess:one', 'sess:two', 'sess:three'] + @sessions = [ + '{"user": {"ip_address": "a", "session_created": "b"}}', + '{"passport": {"user": {"ip_address": "c", "session_created": "d"}}}' + ] + @exclude = ['two'] + @rclient.smembers.callsArgWith(1, null, @sessionKeys) + @rclient.mget.callsArgWith(1, null, @sessions) + @call = (callback) => + @UserSessionsManager.getAllUserSessions @user, @exclude, callback + + it 'should not produce an error', (done) -> + @call (err, sessions) => + expect(err).to.equal null + done() + + it 'should get sessions', (done) -> + @call (err, sessions) => + expect(sessions).to.deep.equal [ + { ip_address: 'a', session_created: 'b' }, + { ip_address: 'c', session_created: 'd' } + ] + done() + + it 'should have called rclient.smembers', (done) -> + @call (err, sessions) => + @rclient.smembers.callCount.should.equal 1 + done() + + it 'should have called rclient.mget', (done) -> + @call (err, sessions) => + @rclient.mget.callCount.should.equal 1 + done() + + describe 'when there are no other sessions', -> + + beforeEach -> + @sessionKeys = ['sess:two'] + @rclient.smembers.callsArgWith(1, null, @sessionKeys) + + it 'should not produce an error', (done) -> + @call (err, sessions) => + expect(err).to.equal null + done() + + it 'should produce an empty list of sessions', (done) -> + @call (err, sessions) => + expect(sessions).to.deep.equal [] + done() + + it 'should have called rclient.smembers', (done) -> + @call (err, sessions) => + @rclient.smembers.callCount.should.equal 1 + done() + + it 'should not have called rclient.mget', (done) -> + @call (err, sessions) => + @rclient.mget.callCount.should.equal 0 + done() + + describe 'when smembers produces an error', -> + + beforeEach -> + @rclient.smembers.callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sessions) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + it 'should not have called rclient.mget', (done) -> + @call (err, sessions) => + @rclient.mget.callCount.should.equal 0 + done() + + describe 'when mget produces an error', -> + + beforeEach -> + @rclient.mget.callsArgWith(1, new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, sessions) => + expect(err).to.not.equal null + expect(err).to.be.instanceof Error + done() + + describe '_checkSessions', -> beforeEach ->