From 15fd090e7a882c11ef929e9c5e42bac5f485ba5e Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 30 Oct 2020 13:33:13 +0100 Subject: [PATCH] Merge pull request #3325 from overleaf/jpa-session-cleanup [UserEmailsController] clear sessions after changing the primary email GitOrigin-RevId: 319b483a3c2851c37c0a340ba9c43a86225a9246 --- .../src/Features/User/UserEmailsController.js | 14 ++++ .../test/acceptance/src/UserEmailsTests.js | 75 +++++++++++++++++++ .../src/User/UserEmailsControllerTests.js | 47 +++++++++++- 3 files changed, 134 insertions(+), 2 deletions(-) diff --git a/services/web/app/src/Features/User/UserEmailsController.js b/services/web/app/src/Features/User/UserEmailsController.js index 0efeb9e358..a711a09276 100644 --- a/services/web/app/src/Features/User/UserEmailsController.js +++ b/services/web/app/src/Features/User/UserEmailsController.js @@ -1,6 +1,8 @@ +const logger = require('logger-sharelatex') const AuthenticationController = require('../Authentication/AuthenticationController') const UserGetter = require('./UserGetter') const UserUpdater = require('./UserUpdater') +const UserSessionsManager = require('./UserSessionsManager') const EmailHandler = require('../Email/EmailHandler') const EmailHelper = require('../Helpers/EmailHelper') const UserEmailsConfirmationHandler = require('./UserEmailsConfirmationHandler') @@ -134,6 +136,18 @@ const UserEmailsController = { return UserEmailsController._handleEmailError(err, req, res, next) } AuthenticationController.setInSessionUser(req, { email: email }) + const user = AuthenticationController.getSessionUser(req) + UserSessionsManager.revokeAllUserSessions( + user, + [req.sessionID], + err => { + if (err) + logger.warn( + { err }, + 'failed revoking secondary sessions after changing default email' + ) + } + ) res.sendStatus(200) } ) diff --git a/services/web/test/acceptance/src/UserEmailsTests.js b/services/web/test/acceptance/src/UserEmailsTests.js index 982a9eb02c..6e121fc47b 100644 --- a/services/web/test/acceptance/src/UserEmailsTests.js +++ b/services/web/test/acceptance/src/UserEmailsTests.js @@ -820,6 +820,81 @@ describe('UserEmails', function() { }) }) }) + + describe('session cleanup', function() { + beforeEach(function setupSecondSession(done) { + this.userSession2 = new User() + this.userSession2.email = this.user.email + this.userSession2.emails = this.user.emails + this.userSession2.password = this.user.password + // login before adding the new email address + // User.login() performs a mongo update and resets the .emails field. + this.userSession2.login(done) + }) + + beforeEach(function checkSecondSessionLiveness(done) { + this.userSession2.request( + { method: 'GET', url: '/project', followRedirect: false }, + (error, response) => { + expect(error).to.not.exist + expect(response.statusCode).to.equal(200) + done() + } + ) + }) + + beforeEach(function addSecondaryEmail(done) { + this.user.request( + { + method: 'POST', + url: '/user/emails', + json: { email: 'new-confirmed-default@example.com' } + }, + (error, response) => { + expect(error).to.not.exist + expect(response.statusCode).to.equal(204) + done() + } + ) + }) + + beforeEach(function confirmSecondaryEmail(done) { + db.users.updateOne( + { 'emails.email': 'new-confirmed-default@example.com' }, + { $set: { 'emails.$.confirmedAt': new Date() } }, + done + ) + }) + + beforeEach(function setDefault(done) { + this.user.request( + { + method: 'POST', + url: '/user/emails/default', + json: { email: 'new-confirmed-default@example.com' } + }, + (error, response) => { + expect(error).to.not.exist + expect(response.statusCode).to.equal(200) + done() + } + ) + }) + + it('should logout the other sessions', function(done) { + this.userSession2.request( + { method: 'GET', url: '/project', followRedirect: false }, + (error, response) => { + expect(error).to.not.exist + expect(response.statusCode).to.equal(302) + expect(response.headers) + .to.have.property('location') + .to.match(new RegExp('^/login')) + done() + } + ) + }) + }) }) describe('when not logged in', function() { diff --git a/services/web/test/unit/src/User/UserEmailsControllerTests.js b/services/web/test/unit/src/User/UserEmailsControllerTests.js index 2facb49d4e..f90da6703e 100644 --- a/services/web/test/unit/src/User/UserEmailsControllerTests.js +++ b/services/web/test/unit/src/User/UserEmailsControllerTests.js @@ -12,6 +12,7 @@ const Errors = require('../../../../app/src/Features/Errors/Errors') describe('UserEmailsController', function() { beforeEach(function() { this.req = new MockRequest() + this.req.sessionID = Math.random().toString() this.res = new MockResponse() this.next = sinon.stub() this.user = { _id: 'mock-user-id', email: 'example@overleaf.com' } @@ -23,12 +24,16 @@ describe('UserEmailsController', function() { } } this.AuthenticationController = { + getSessionUser: sinon.stub().returns(this.user), getLoggedInUserId: sinon.stub().returns(this.user._id), setInSessionUser: sinon.stub() } this.Features = { hasFeature: sinon.stub() } + this.UserSessionsManager = { + revokeAllUserSessions: sinon.stub().yields() + } this.UserUpdater = { addEmailAddress: sinon.stub(), removeEmailAddress: sinon.stub(), @@ -58,6 +63,7 @@ describe('UserEmailsController', function() { '../Authentication/AuthenticationController': this .AuthenticationController, '../../infrastructure/Features': this.Features, + './UserSessionsManager': this.UserSessionsManager, './UserGetter': this.UserGetter, './UserUpdater': this.UserUpdater, '../Email/EmailHandler': (this.EmailHandler = { @@ -74,10 +80,11 @@ describe('UserEmailsController', function() { '../Institutions/InstitutionsAPI': this.InstitutionsAPI, '../Errors/HttpErrorHandler': this.HttpErrorHandler, '../Errors/Errors': Errors, - 'logger-sharelatex': { + 'logger-sharelatex': (this.logger = { log() {}, + warn: sinon.stub(), err() {} - } + }) } }) }) @@ -317,6 +324,42 @@ describe('UserEmailsController', function() { } }) }) + + it('should reset the users other sessions', function(done) { + this.UserUpdater.setDefaultEmailAddress.yields() + + this.res.callback = () => { + expect( + this.UserSessionsManager.revokeAllUserSessions + ).to.have.been.calledWith(this.user, [this.req.sessionID]) + done() + } + + this.UserEmailsController.setDefault(this.req, this.res, done) + }) + + it('handles error from revoking sessions and returns 200', function(done) { + this.UserUpdater.setDefaultEmailAddress.yields() + const redisError = new Error('redis error') + this.UserSessionsManager.revokeAllUserSessions = sinon + .stub() + .yields(redisError) + + this.res.callback = () => { + expect(this.res.statusCode).to.equal(200) + + // give revoke process time to run + setTimeout(() => { + expect(this.logger.warn).to.have.been.calledWith( + sinon.match({ err: redisError }), + 'failed revoking secondary sessions after changing default email' + ) + done() + }) + } + + this.UserEmailsController.setDefault(this.req, this.res, done) + }) }) describe('endorse', function() {