Merge pull request #759 from sharelatex/sk-login-with-v1-credentials

Login with v1 credentials

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

@@ -62,15 +62,22 @@ module.exports = AuthenticationController =
 			if err?
 				return next(err)
 			if user # `user` is either a user object or false
+				AuthenticationController.finishLogin(user, req, res, next)
+			else
+				res.json message: info
+		)(req, res, next)
+
+	finishLogin: (user, req, res, next) ->
 		redir = AuthenticationController._getRedirectFromSession(req) || "/project"
+		AuthenticationController._loginAsyncHandlers(req, user)
 		AuthenticationController.afterLoginSessionSetup req, user, (err) ->
 			if err?
 				return next(err)
 			AuthenticationController._clearRedirectFromSession(req)
+			if req.headers?['accept']?.match(/^application\/json.*$/)
 				res.json {redir: redir}
 			else
-				res.json message: info
-		)(req, res, next)
+				res.redirect(redir)
 
 	doPassportLogin: (req, username, password, done) ->
 		email = username.toLowerCase()
@@ -83,21 +90,23 @@ module.exports = AuthenticationController =
 				return done(error) if error?
 				if user?
 					# async actions
-					UserHandler.setupLoginData(user, ()->)
-					LoginRateLimiter.recordSuccessfulLogin(email)
-					AuthenticationController._recordSuccessfulLogin(user._id)
-					Analytics.recordEvent(user._id, "user-logged-in", {ip:req.ip})
-					Analytics.identifyUser(user._id, req.sessionID)
-					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
 					return done(null, user)
 				else
 					AuthenticationController._recordFailedLogin()
 					logger.log email: email, "failed log in"
 					return done(null, false, {text: req.i18n.translate("email_or_password_wrong_try_again"), type: 'error'})
 
+	_loginAsyncHandlers: (req, user) ->
+		UserHandler.setupLoginData(user, ()->)
+		LoginRateLimiter.recordSuccessfulLogin(user.email)
+		AuthenticationController._recordSuccessfulLogin(user._id)
+		Analytics.recordEvent(user._id, "user-logged-in", {ip:req.ip})
+		Analytics.identifyUser(user._id, req.sessionID)
+		logger.log email: user.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
+
 	setInSessionUser: (req, props) ->
 		for key, value of props
 			if req?.session?.passport?.user?

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

@@ -98,6 +98,7 @@ describe "AuthenticationController", ->
 			@req.session.destroy = sinon.stub().callsArgWith(0, null)
 			@req.session.save = sinon.stub().callsArgWith(0, null)
 			@req.sessionStore = {generate: sinon.stub()}
+			@AuthenticationController.finishLogin = sinon.stub()
 			@passport.authenticate.callsArgWith(1, null, @user, @info)
 			@err = new Error('woops')
 
@@ -123,27 +124,10 @@ describe "AuthenticationController", ->
 			afterEach ->
 				delete @req.session.postLoginRedirect
 
-			it 'should call req.login', () ->
+			it 'should call finishLogin', () ->
 				@AuthenticationController.passportLogin @req, @res, @next
-				@req.login.callCount.should.equal 1
-				@req.login.calledWith(@user).should.equal true
-
-			it 'should send a json response with redirect', () ->
-				@AuthenticationController.passportLogin @req, @res, @next
-				@res.json.callCount.should.equal 1
-				@res.json.calledWith({redir: 'some_redirect'}).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
+				@AuthenticationController.finishLogin.callCount.should.equal 1
+				@AuthenticationController.finishLogin.calledWith(@user).should.equal true
 
 		describe 'when authenticate does not produce a user', ->
 
@@ -151,9 +135,9 @@ describe "AuthenticationController", ->
 				@info = {text: 'a', type: 'b'}
 				@passport.authenticate.callsArgWith(1, null, false, @info)
 
-			it 'should not call req.login', () ->
+			it 'should not call finishLogin', () ->
 				@AuthenticationController.passportLogin @req, @res, @next
-				@req.login.callCount.should.equal 0
+				@AuthenticationController.finishLogin.callCount.should.equal 0
 
 			it 'should not send a json response with redirect', () ->
 				@AuthenticationController.passportLogin @req, @res, @next
@@ -255,7 +239,6 @@ describe "AuthenticationController", ->
 				@LoginRateLimiter.processLoginRequest.callsArgWith(1, null, true)
 				@AuthenticationManager.authenticate = sinon.stub().callsArgWith(2, null, @user)
 				@req.sessionID = Math.random()
-				@AnalyticsManager.identifyUser = sinon.stub()
 				@AuthenticationController.doPassportLogin(@req, @req.body.email, @req.body.password, @cb)
 
 			it "should attempt to authorise the user", ->
@@ -263,15 +246,24 @@ describe "AuthenticationController", ->
 					.calledWith(email: @email.toLowerCase(), @password)
 					.should.equal true
 
+			it "should establish the user's session", ->
+				@cb.calledWith(null, @user).should.equal true
+
+		describe '_loginAsyncHandlers', ->
+			beforeEach ->
+				@UserHandler.setupLoginData = sinon.stub()
+				@LoginRateLimiter.recordSuccessfulLogin = sinon.stub()
+				@AuthenticationController._recordSuccessfulLogin = sinon.stub()
+				@AnalyticsManager.recordEvent = sinon.stub()
+				@AnalyticsManager.identifyUser = sinon.stub()
+				@AuthenticationController._loginAsyncHandlers(@req, @user)
+
 			it "should call identifyUser", ->
 				@AnalyticsManager.identifyUser.calledWith(@user._id, @req.sessionID).should.equal true
 
 			it "should setup the user data in the background", ->
 				@UserHandler.setupLoginData.calledWith(@user).should.equal true
 
-			it "should establish the user's session", ->
-				@cb.calledWith(null, @user).should.equal true
-
 			it "should set res.session.justLoggedIn", ->
 				@req.session.justLoggedIn.should.equal true
 
@@ -281,11 +273,11 @@ describe "AuthenticationController", ->
 					.should.equal true
 
 			it "should tell the rate limiter that there was a success for that email", ->
-				@LoginRateLimiter.recordSuccessfulLogin.calledWith(@email.toLowerCase()).should.equal true
+				@LoginRateLimiter.recordSuccessfulLogin.calledWith(@user.email).should.equal true
 
 			it "should log the successful login", ->
 				@logger.log
-					.calledWith(email: @email.toLowerCase(), user_id: @user._id.toString(), "successful log in")
+					.calledWith(email: @user.email, user_id: @user._id.toString(), "successful log in")
 					.should.equal true
 
 			it "should track the login event", ->
@@ -587,3 +579,56 @@ describe "AuthenticationController", ->
 			@AuthenticationController._clearRedirectFromSession(@req)
 			expect(@req.session.postLoginRedirect).to.equal undefined
 
+
+	describe 'finishLogin', ->
+		# - get redirect
+		# - async handlers
+		# - afterLoginSessionSetup
+		# - clear redirect
+		# - issue redir, two ways
+		beforeEach ->
+			@AuthenticationController._getRedirectFromSession = sinon.stub().returns '/some/page'
+			@AuthenticationController._loginAsyncHandlers = sinon.stub()
+			@AuthenticationController.afterLoginSessionSetup = sinon.stub().callsArgWith(2, null)
+			@AuthenticationController._clearRedirectFromSession = sinon.stub()
+			@req.headers = {accept: 'application/json, whatever'}
+			@res.json = sinon.stub()
+			@res.redirect = sinon.stub()
+
+		it 'should extract the redirect from the session', () ->
+			@AuthenticationController.finishLogin(@user, @req, @res, @next)
+			expect(@AuthenticationController._getRedirectFromSession.callCount).to.equal 1
+			expect(@AuthenticationController._getRedirectFromSession.calledWith(@req)).to.equal true
+
+		it 'should call the async handlers', () ->
+			@AuthenticationController.finishLogin(@user, @req, @res, @next)
+			expect(@AuthenticationController._loginAsyncHandlers.callCount).to.equal 1
+			expect(@AuthenticationController._loginAsyncHandlers.calledWith(@req, @user)).to.equal true
+
+		it 'should call afterLoginSessionSetup', () ->
+			@AuthenticationController.finishLogin(@user, @req, @res, @next)
+			expect(@AuthenticationController.afterLoginSessionSetup.callCount).to.equal 1
+			expect(@AuthenticationController.afterLoginSessionSetup.calledWith(@req, @user)).to.equal true
+
+		it 'should clear redirect from session', () ->
+			@AuthenticationController.finishLogin(@user, @req, @res, @next)
+			expect(@AuthenticationController._clearRedirectFromSession.callCount).to.equal 1
+			expect(@AuthenticationController._clearRedirectFromSession.calledWith(@req)).to.equal true
+
+		it 'should issue a json response with a redirect', () ->
+			@AuthenticationController.finishLogin(@user, @req, @res, @next)
+			expect(@res.json.callCount).to.equal 1
+			expect(@res.redirect.callCount).to.equal 0
+			expect(@res.json.calledWith({ redir: '/some/page' })).to.equal true
+
+		describe 'with a non-json request', ->
+			beforeEach ->
+				@req.headers = {}
+				@res.json = sinon.stub()
+				@res.redirect = sinon.stub()
+
+			it 'should issue a plain redirect', () ->
+				@AuthenticationController.finishLogin(@user, @req, @res, @next)
+				expect(@res.json.callCount).to.equal 0
+				expect(@res.redirect.callCount).to.equal 1
+				expect(@res.redirect.calledWith('/some/page')).to.equal true