Regenerate the session id after logging in or registering

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

@@ -6,6 +6,7 @@ Metrics = require('../../infrastructure/Metrics')
 logger = require("logger-sharelatex")
 querystring = require('querystring')
 Url = require("url")
+uid = require "uid"
 
 module.exports = AuthenticationController =
 	login: (req, res, next = (error) ->) ->
@@ -25,8 +26,9 @@ module.exports = AuthenticationController =
 				if user?
 					LoginRateLimiter.recordSuccessfulLogin email
 					AuthenticationController._recordSuccessfulLogin user._id
-					AuthenticationController._establishUserSession req, user, (error) ->
+					AuthenticationController.establishUserSession req, user, (error) ->
 						return next(error) if error?
+						req.session.justLoggedIn = true
 						logger.log email: email, user_id: user._id.toString(), "successful log in"
 						res.send redir: redir
 				else
@@ -118,7 +120,7 @@ module.exports = AuthenticationController =
 		Metrics.inc "user.login.failed"
 		callback()
 
-	_establishUserSession: (req, user, callback = (error) ->) ->
+	establishUserSession: (req, user, callback = (error) ->) ->
 		lightUser =
 			_id: user._id
 			first_name: user.first_name
@@ -126,6 +128,12 @@ module.exports = AuthenticationController =
 			isAdmin: user.isAdmin
 			email: user.email
 			referal_id: user.referal_id
+		# Regenerate the session to get a new sessionID (cookie value) to
+		# protect against session fixation attacks
+		oldSession = req.session
+		req.sessionStore.generate(req)
+		for key, value of oldSession
+			req.session[key] = value
+
 		req.session.user = lightUser
-		req.session.justLoggedIn = true
+		callback()
-		req.session.save callback

services/web/app/coffee/Features/User/UserController.coffee

@@ -89,17 +89,18 @@ module.exports =
 				next(err)
 			else
 				metrics.inc "user.register.success"
-				req.session.user = user
-				req.session.justRegistered = true
 				ReferalAllocator.allocate req.session.referal_id, user._id, req.session.referal_source, req.session.referal_medium
 				SubscriptionDomainAllocator.autoAllocate(user)
-				res.send
+				AuthenticationController.establishUserSession req, user, (error) ->
-					redir:redir
+					return callback(error) if error?
-					id:user._id.toString()
+					req.session.justRegistered = true
-					first_name: user.first_name
+					res.send
-					last_name: user.last_name
+						redir:redir
-					email: user.email
+						id:user._id.toString()
-					created: Date.now()
+						first_name: user.first_name
+						last_name: user.last_name
+						email: user.email
+						created: Date.now()
 
 
 	changePassword : (req, res, next = (error) ->)->

services/web/package.json

@@ -39,6 +39,7 @@
     "settings-sharelatex": "git+https://github.com/sharelatex/settings-sharelatex.git#v1.0.0",
     "socket.io": "0.9.16",
     "translations-sharelatex": "git+https://github.com/sharelatex/translations-sharelatex.git#master",
+    "uid": "0.0.2",
     "underscore": "1.6.0",
     "underscore.string": "^3.0.2",
     "v8-profiler": "^5.2.3",

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

@@ -40,7 +40,7 @@ describe "AuthenticationController", ->
 		beforeEach ->
 			@AuthenticationController._recordFailedLogin = sinon.stub()
 			@AuthenticationController._recordSuccessfulLogin = sinon.stub()
-			@AuthenticationController._establishUserSession = sinon.stub().callsArg(2)
+			@AuthenticationController.establishUserSession = sinon.stub().callsArg(2)
 			@req.body =
 				email: @email
 				password: @password
@@ -68,9 +68,12 @@ describe "AuthenticationController", ->
 					.should.equal true
 
 			it "should establish the user's session", ->
-				@AuthenticationController._establishUserSession
+				@AuthenticationController.establishUserSession
 					.calledWith(@req, @user)
 					.should.equal true
+					
+			it "should set res.session.justLoggedIn", ->
+				@req.session.justLoggedIn.should.equal true
 
 			it "should redirect the user to the specified location", ->
 				expect(@res.body).to.deep.equal redir: @redir
@@ -103,7 +106,7 @@ describe "AuthenticationController", ->
 					# 	type: 'error'
 
 			it "should not establish a session", ->
-				@AuthenticationController._establishUserSession.called.should.equal false
+				@AuthenticationController.establishUserSession.called.should.equal false
 
 			it "should record a failed login", ->
 				@AuthenticationController._recordFailedLogin.called.should.equal true
@@ -272,48 +275,42 @@ describe "AuthenticationController", ->
 					.calledWith(@req, {allow_auth_token: true})
 					.should.equal true
 
+	describe "_redirectToLoginOrRegisterPage", ->
+		beforeEach ->
+			@middleware = @AuthenticationController.requireLogin(@options = { load_from_db: false })
+			@req.session = {}
+			@AuthenticationController._redirectToRegisterPage = sinon.stub()
+			@AuthenticationController._redirectToLoginPage = sinon.stub()
+			@req.query = {}
 
-
+		describe "they have come directly to the url", ->
-		describe "_redirectToLoginOrRegisterPage", ->
+			beforeEach -> 
-
-			beforeEach ->
-				@middleware = @AuthenticationController.requireLogin(@options = { load_from_db: false })
-				@req.session = {}
-				@AuthenticationController._redirectToRegisterPage = sinon.stub()
-				@AuthenticationController._redirectToLoginPage = sinon.stub()
 				@req.query = {}
+				@middleware(@req, @res, @next)
 
-			describe "they have come directly to the url", ->
+			it "should redirect to the login page", ->
-				beforeEach -> 
+				@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal false
-					@req.query = {}
+				@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal true
-					@middleware(@req, @res, @next)
 
-				it "should redirect to the login page", ->
+		describe "they have come via a templates link", ->
-					@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal false
-					@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal true
 
-			describe "they have come via a templates link", ->
+			beforeEach -> 
-
+				@req.query.zipUrl = "something"
-				beforeEach -> 
+				@middleware(@req, @res, @next)
-					@req.query.zipUrl = "something"
-					@middleware(@req, @res, @next)
-
-				it "should redirect to the register page", ->
-					@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal true
-					@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
-
-			describe "they have been invited to a project", ->
-				
-				beforeEach -> 
-					@req.query.project_name = "something"
-					@middleware(@req, @res, @next)
-
-				it "should redirect to the register page", ->
-					@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal true
-					@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
 
+			it "should redirect to the register page", ->
+				@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal true
+				@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
 
+		describe "they have been invited to a project", ->
+			
+			beforeEach -> 
+				@req.query.project_name = "something"
+				@middleware(@req, @res, @next)
 
+			it "should redirect to the register page", ->
+				@AuthenticationController._redirectToRegisterPage.calledWith(@req, @res).should.equal true
+				@AuthenticationController._redirectToLoginPage.calledWith(@req, @res).should.equal false
 
 	describe "_redirectToRegisterPage", ->
 		beforeEach ->
@@ -371,11 +368,13 @@ describe "AuthenticationController", ->
 		it "should call the callback", ->
 			@callback.called.should.equal true
 
-	describe "_establishUserSession", ->
+	describe "establishUserSession", ->
 		beforeEach ->
 			@req.session =
 				save: sinon.stub().callsArg(0)
-			@AuthenticationController._establishUserSession @req, @user, @callback
+			@req.sessionStore =
+				generate: sinon.stub()
+			@AuthenticationController.establishUserSession @req, @user, @callback
 
 		it "should set the session user to a basic version of the user", ->
 			@req.session.user._id.should.equal @user._id
@@ -384,6 +383,9 @@ describe "AuthenticationController", ->
 			@req.session.user.last_name.should.equal @user.last_name
 			@req.session.user.referal_id.should.equal @user.referal_id
 			@req.session.user.isAdmin.should.equal @user.isAdmin
+			
+		it "should regenerate the session to protect against session fixation", ->
+			@req.sessionStore.generate.calledWith(@req).should.equal true
 
 		it "should return the callback", ->
 			@callback.called.should.equal true

services/web/test/UnitTests/coffee/User/UserControllerTests.coffee

@@ -29,7 +29,8 @@ describe "UserController", ->
 			unsubscribe: sinon.stub().callsArgWith(1)
 		@UserRegistrationHandler =
 			registerNewUser: sinon.stub()
-		@AuthenticationController = {}
+		@AuthenticationController =
+			establishUserSession: sinon.stub().callsArg(2)
 		@AuthenticationManager =
 			authenticate: sinon.stub()
 			setUserPassword: sinon.stub()
@@ -181,7 +182,9 @@ describe "UserController", ->
 		it "should put the user on the session and mark them as justRegistered", (done)->
 			@UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user)
 			@res.send = =>
-				assert.deepEqual @user, @req.session.user
+				@AuthenticationController.establishUserSession
+					.calledWith(@req, @user)
+					.should.equal true
 				assert.equal @req.session.justRegistered, true
 				done()
 			@UserController.register @req, @res