From 2119dcbb589f2946f6ed107426eca9a927c936ff Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 15 Sep 2016 14:36:11 +0100 Subject: [PATCH] Finalise login workflow, works with login form again. --- .../AuthenticationController.coffee | 78 +++++-------------- services/web/app/coffee/router.coffee | 7 +- .../AuthenticationControllerTests.coffee | 60 +++++++++++++- 3 files changed, 80 insertions(+), 65 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 8d12acfb64..4342da9d77 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -11,6 +11,7 @@ basicAuth = require('basic-auth-connect') UserHandler = require("../User/UserHandler") UserSessionsManager = require("../User/UserSessionsManager") Analytics = require "../Analytics/AnalyticsManager" +passport = require 'passport' module.exports = AuthenticationController = @@ -32,6 +33,22 @@ module.exports = AuthenticationController = deserializeUser: (user, cb) -> cb(null, user) + passportLogin: (req, res, next) -> + # This function is middleware which wraps the passport.authenticate middleware, + # so we can send back our custom `{message: {text: "", type: ""}}` responses on failure, + # and send a `{redir: ""}` response on success + passport.authenticate('local', (err, user, info) -> + # `user` is either a user object or false + if err? + return next(err) + if user + req.login user, (err) -> + res.json {redir: req._redir} + else + res.json message: info + )(req, res, next) + + doPassportLogin: (req, username, password, done) -> email = username.toLowerCase() redir = Url.parse(req?.body?.redir or "/project").path @@ -39,7 +56,7 @@ module.exports = AuthenticationController = return done(err) if err? if !isAllowed logger.log email:email, "too many login requests" - return done(null, null, {message: req.i18n.translate("to_many_login_requests_2_mins"), type: 'error'}) + return done(null, null, {text: req.i18n.translate("to_many_login_requests_2_mins"), type: 'error'}) AuthenticationManager.authenticate email: email, password, (error, user) -> return done(error) if error? if user? @@ -58,7 +75,7 @@ module.exports = AuthenticationController = else AuthenticationController._recordFailedLogin() logger.log email: email, "failed log in" - return done(null, false, {message: req.i18n.translate("email_or_password_wrong_try_again"), type: 'error'}) + return done(null, false, {text: req.i18n.translate("email_or_password_wrong_try_again"), type: 'error'}) isUserLoggedIn: (req) -> user_id = AuthenticationController.getLoggedInUserId(req) @@ -152,60 +169,3 @@ module.exports = AuthenticationController = _recordFailedLogin: (callback = (error) ->) -> Metrics.inc "user.login.failed" callback() - - # establishUserSession: (req, user, callback = (error) ->) -> - # dienow - # lightUser = - # _id: user._id - # first_name: user.first_name - # last_name: user.last_name - # isAdmin: user.isAdmin - # email: user.email - # referal_id: user.referal_id - # session_created: (new Date()).toISOString() - # ip_address: req.ip - # # 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.user = lightUser - - # UserSessionsManager.trackSession(user, req.sessionID, () ->) - # callback() - - - # doLogin: (options, req, res, next) -> - # dienow - # email = options.email?.toLowerCase() - # password = options.password - # redir = Url.parse(options.redir or "/project").path - # LoginRateLimiter.processLoginRequest email, (err, isAllowed)-> - # if !isAllowed - # logger.log email:email, "too many login requests" - # res.statusCode = 429 - # return res.send - # message: - # text: req.i18n.translate("to_many_login_requests_2_mins"), - # type: 'error' - # AuthenticationManager.authenticate email: email, password, (error, user) -> - # return next(error) if error? - # if user? - # UserHandler.setupLoginData user, -> - # LoginRateLimiter.recordSuccessfulLogin email - # AuthenticationController._recordSuccessfulLogin user._id - # 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" - # Analytics.recordEvent user._id, "user-logged-in" - # res.json redir: redir - # else - # AuthenticationController._recordFailedLogin() - # logger.log email: email, "failed log in" - # res.json message: - # text: req.i18n.translate("email_or_password_wrong_try_again"), - # type: 'error' diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index 86a97b0a40..5b8f0b49f9 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -43,7 +43,6 @@ AnalyticsRouter = require('./Features/Analytics/AnalyticsRouter') logger = require("logger-sharelatex") _ = require("underscore") -passport = require('passport') module.exports = class Router constructor: (webRouter, apiRouter)-> @@ -54,10 +53,8 @@ module.exports = class Router webRouter.get '/login', UserPagesController.loginPage AuthenticationController.addEndpointToLoginWhitelist '/login' - # webRouter.post '/login', AuthenticationController.login - webRouter.post '/login', passport.authenticate('local'), (req, res) -> - console.log ">> login done", req._redir - res.json {redir: req._redir} + webRouter.post '/login', AuthenticationController.passportLogin + webRouter.get '/logout', UserController.logout webRouter.get '/restricted', AuthorizationMiddlewear.restricted diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee index 3e0812d3ff..07d9b60567 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee @@ -22,6 +22,8 @@ describe "AuthenticationController", -> "../Analytics/AnalyticsManager": @AnalyticsManager = { recordEvent: sinon.stub() } "logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() } "settings-sharelatex": {} + "passport": @passport = + authenticate: sinon.stub().returns(sinon.stub()) "../User/UserSessionsManager": @UserSessionsManager = trackSession: sinon.stub() untrackSession: sinon.stub() @@ -42,6 +44,62 @@ describe "AuthenticationController", -> afterEach -> tk.reset() + describe 'passportLogin', -> + + beforeEach -> + @info = null + @req.login = sinon.stub().callsArgWith(1, null) + @res.json = sinon.stub() + @passport.authenticate.callsArgWith(1, null, @user, @info) + + it 'should call passport.authenticate', () -> + @AuthenticationController.passportLogin @req, @res, @next + @passport.authenticate.callCount.should.equal 1 + + describe 'when authenticate produces an error', -> + + beforeEach -> + @err = new Error('woops') + @passport.authenticate.callsArgWith(1, @err) + + it 'should return next with an error', () -> + @AuthenticationController.passportLogin @req, @res, @next + @next.calledWith(@err).should.equal true + + describe 'when authenticate produces a user', -> + + beforeEach -> + @req._redir = 'some_redirect' + @passport.authenticate.callsArgWith(1, null, @user, @info) + + afterEach -> + delete @req._redir + + it 'should call req.login', () -> + @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: @req._redir}).should.equal true + + describe 'when authenticate does not produce a user', -> + + beforeEach -> + @info = {text: 'a', type: 'b'} + @passport.authenticate.callsArgWith(1, null, false, @info) + + it 'should not call req.login', () -> + @AuthenticationController.passportLogin @req, @res, @next + @req.login.callCount.should.equal 0 + + it 'should send a json response with redirect', () -> + @AuthenticationController.passportLogin @req, @res, @next + @res.json.callCount.should.equal 1 + @res.json.calledWith({message: @info}).should.equal true + describe 'getSessionUser', -> it 'should get the user object from session', -> @@ -132,7 +190,7 @@ describe "AuthenticationController", -> @cb.callCount.should.equal 1 @cb.calledWith(null, false) # @res.body.should.exist - expect(@cb.lastCall.args[2]).to.contain.all.keys ['message'] + expect(@cb.lastCall.args[2]).to.contain.all.keys ['text', 'type'] # message: # text: 'Your email or password were incorrect. Please try again', # type: 'error'