diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index dfa19ed8a8..832f01043f 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -62,16 +62,23 @@ module.exports = AuthenticationController = if err? return next(err) if user # `user` is either a user object or false - redir = AuthenticationController._getRedirectFromSession(req) || "/project" - AuthenticationController.afterLoginSessionSetup req, user, (err) -> - if err? - return next(err) - AuthenticationController._clearRedirectFromSession(req) - res.json {redir: redir} + 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.redirect(redir) + doPassportLogin: (req, username, password, done) -> email = username.toLowerCase() LoginRateLimiter.processLoginRequest email, (err, isAllowed)-> @@ -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? diff --git a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee index f631dfe9a9..8482c28e04 100644 --- a/services/web/test/unit/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/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