Regenerate session on login, protect against session-fixation attack.

services/web/app/coffee/Features/Authentication/AuthenticationController.coffee

@@ -39,7 +39,19 @@ module.exports = AuthenticationController =
 				return next(err)
 			if user # `user` is either a user object or false
 				req.login user, (err) ->
-					res.json {redir: req._redir}
+					# Regenerate the session to get a new sessionID (cookie value) to
+					# protect against session fixation attacks
+					oldSession = req.session
+					req.session.destroy()
+					req.sessionStore.generate(req)
+					for key, value of oldSession
+						req.session[key] = value
+					req.session.save (err) ->
+						if err?
+							logger.err {user_id: user._id}, "error saving regenerated session after login"
+							return next(err)
+						UserSessionsManager.trackSession(user, req.sessionID, () ->)
+						res.json {redir: req._redir}
 			else
 				res.json message: info
 		)(req, res, next)
@@ -60,9 +72,8 @@ module.exports = AuthenticationController =
 					LoginRateLimiter.recordSuccessfulLogin(email)
 					AuthenticationController._recordSuccessfulLogin(user._id)
 					Analytics.recordEvent(user._id, "user-logged-in")
-					UserSessionsManager.trackSession(user, req.sessionID, () ->)
-					req.session.justLoggedIn = true
 					logger.log email: email, user_id: user._id.toString(), "successful log in"
+					req.session.justLoggedIn = true
 					# capture the request ip for use when creating the session
 					user._login_req_ip = req.ip
 					req._redir = redir

services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee

@@ -20,7 +20,7 @@ describe "AuthenticationController", ->
 			"../Security/LoginRateLimiter": @LoginRateLimiter = { processLoginRequest:sinon.stub(), recordSuccessfulLogin:sinon.stub() }
 			"../User/UserHandler": @UserHandler = {setupLoginData:sinon.stub()}
 			"../Analytics/AnalyticsManager": @AnalyticsManager = { recordEvent: sinon.stub() }
-			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
+			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub(), err: sinon.stub() }
 			"settings-sharelatex": {}
 			"passport": @passport =
 				authenticate: sinon.stub().returns(sinon.stub())
@@ -50,6 +50,10 @@ describe "AuthenticationController", ->
 			@info = null
 			@req.login = sinon.stub().callsArgWith(1, null)
 			@res.json = sinon.stub()
+			@req.session = @session = {test: 'test'}
+			@req.session.destroy = sinon.stub()
+			@req.session.save = sinon.stub().callsArgWith(0, null)
+			@req.sessionStore = {generate: sinon.stub()}
 			@passport.authenticate.callsArgWith(1, null, @user, @info)
 
 		it 'should call passport.authenticate', () ->
@@ -85,6 +89,18 @@ describe "AuthenticationController", ->
 				@res.json.callCount.should.equal 1
 				@res.json.calledWith({redir: @req._redir}).should.equal true
 
+			describe 'when session.save produces an error', () ->
+				beforeEach ->
+					@req.session.save = sinon.stub().callsArgWith(0, new Error('woops'))
+
+				it 'should return next with an error', () ->
+					@AuthenticationController.passportLogin @req, @res, @next
+					@next.calledWith(@err).should.equal true
+
+				it 'should not return json', () ->
+					@AuthenticationController.passportLogin @req, @res, @next
+					@res.json.callCount.should.equal 0
+
 		describe 'when authenticate does not produce a user', ->
 
 			beforeEach ->