From 6f93362fdcf36b5c771b84ad159fb46e140f5ac9 Mon Sep 17 00:00:00 2001 From: Geri Morales Date: Sun, 8 Nov 2015 23:02:41 -0600 Subject: [PATCH 1/5] Updated package warning parser --- .../web/public/js/libs/latex-log-parser.js | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/services/web/public/js/libs/latex-log-parser.js b/services/web/public/js/libs/latex-log-parser.js index 2ccd6b5450..4badc5be80 100644 --- a/services/web/public/js/libs/latex-log-parser.js +++ b/services/web/public/js/libs/latex-log-parser.js @@ -3,10 +3,11 @@ define(function() { var LOG_WRAP_LIMIT = 79; var LATEX_WARNING_REGEX = /^LaTeX Warning: (.*)$/; var HBOX_WARNING_REGEX = /^(Over|Under)full \\(v|h)box/; - var BIBER_WARNING_REGEX = /^Package biblatex Warning: (.*)$/; - var NATBIB_WARNING_REGEX = /^Package natbib Warning: (.*)$/; + var PACKAGE_WARNING_REGEX = /^(Package \b.+\b Warning:.*)$/; // This is used to parse the line number from common latex warnings var LINES_REGEX = /lines? ([0-9]+)/; + // This is used to parse the package name from the package warnings + var PACKAGE_REGEX = /^Package (\b.+\b) Warning/; var LogText = function(text) { this.text = text.replace(/(\r\n)|\r/g, "\n"); @@ -101,10 +102,8 @@ define(function() { this.parseSingleWarningLine(LATEX_WARNING_REGEX); } else if (this.currentLineIsHboxWarning()) { this.parseHboxLine(); - } else if (this.currentLineIsBiberWarning()) { - this.parseBiberWarningLine(); - } else if (this.currentLineIsNatbibWarning()) { - this.parseSingleWarningLine(NATBIB_WARNING_REGEX); + } else if (this.currentLineIsPackageWarning()) { + this.parseMultipleWarningLine(); } else { this.parseParensForFilenames(); } @@ -140,12 +139,8 @@ define(function() { return !!(this.currentLine.match(LATEX_WARNING_REGEX)); }; - this.currentLineIsBiberWarning = function () { - return !!(this.currentLine.match(BIBER_WARNING_REGEX)); - }; - - this.currentLineIsNatbibWarning = function () { - return !!(this.currentLine.match(NATBIB_WARNING_REGEX)); + this.currentLineIsPackageWarning = function () { + return !!(this.currentLine.match(PACKAGE_WARNING_REGEX)); }; this.currentLineIsHboxWarning = function() { @@ -169,22 +164,31 @@ define(function() { }); }; - this.parseBiberWarningLine = function() { - // Biber warnings are multiple lines, let's parse the first line - var warningMatch = this.currentLine.match(BIBER_WARNING_REGEX); + this.parseMultipleWarningLine = function() { + // Some package warnings are multiple lines, let's parse the first line + var warningMatch = this.currentLine.match(PACKAGE_WARNING_REGEX); if (!warningMatch) return; // Something strange happened, return early - // Now loop over the other output and just grab the message part - // Each line is prefiex with: (biblatex) var warning_lines = [warningMatch[1]]; - while (((this.currentLine = this.log.nextLine()) !== false) && - (warningMatch = this.currentLine.match(/^\(biblatex\)[\s]+(.*)$/))) { - warning_lines.push(warningMatch[1]) + var lineMatch = this.currentLine.match(LINES_REGEX); + var line = lineMatch ? parseInt(lineMatch[1], 10) : null; + var packageMatch = this.currentLine.match(PACKAGE_REGEX); + var packageName = packageMatch[1]; + + // Regex to get rid of the unnecesary (packagename) prefix in most multi-line warnings + var prefixRegex = new RegExp("(?:\\(" + packageName + "\\))*[\\s]*(.*)", "i"); + + // After every warning message there's a blank line, let's use it + while (!!(this.currentLine = this.log.nextLine())) { + lineMatch = this.currentLine.match(LINES_REGEX); + line = lineMatch ? parseInt(lineMatch[1], 10) : line; + warningMatch = this.currentLine.match(prefixRegex) + warning_lines.push(warningMatch[1]); } var raw_message = warning_lines.join(' '); this.data.push({ - line : null, // Unfortunately, biber doesn't return a line number + line : line, file : this.currentFilePath, level : "warning", message : raw_message, From ba6c361afa17226db0268f4c54df6f855edcdab2 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 10 Dec 2015 14:18:20 +0000 Subject: [PATCH 2/5] decrease sentry sample rate to 1% --- services/web/app/views/sentry.jade | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/views/sentry.jade b/services/web/app/views/sentry.jade index 397e5bfafc..9fe5dd3b7d 100644 --- a/services/web/app/views/sentry.jade +++ b/services/web/app/views/sentry.jade @@ -57,7 +57,7 @@ ], shouldSendCallback: function(data) { // only send a fraction of errors - var sampleRate = 0.05; + var sampleRate = 0.01; return (Math.random() <= sampleRate); }, dataCallback: function(data) { From 1e8ab5357b0ed8f189131d1cafb6b41e07e467e1 Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 11 Dec 2015 11:30:06 +0000 Subject: [PATCH 3/5] Improve pre-registered account activation process --- .../AuthenticationController.coffee | 9 ++- .../PasswordResetController.coffee | 15 +++-- .../PasswordReset/PasswordResetHandler.coffee | 6 +- .../Features/User/UserController.coffee | 2 +- .../Features/User/UserPagesController.coffee | 26 +++++++- services/web/app/coffee/router.coffee | 2 + services/web/app/views/user/activate.jade | 64 +++++++++++++++++++ services/web/app/views/user/login.jade | 1 + services/web/app/views/user/setPassword.jade | 9 +-- .../PasswordResetControllerTests.coffee | 18 +++++- .../PasswordResetHandlerTests.coffee | 3 +- .../coffee/User/UserControllerTests.coffee | 4 +- .../User/UserPagesControllerTests.coffee | 51 ++++++++++++++- 13 files changed, 188 insertions(+), 22 deletions(-) create mode 100644 services/web/app/views/user/activate.jade diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index c992fec578..3afad1360c 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -12,9 +12,12 @@ basicAuth = require('basic-auth-connect') module.exports = AuthenticationController = login: (req, res, next = (error) ->) -> - email = req.body?.email?.toLowerCase() - password = req.body?.password - redir = Url.parse(req.body?.redir or "/project").path + AuthenticationController.doLogin req.body, req, res, next + + doLogin: (options, req, res, next) -> + 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" diff --git a/services/web/app/coffee/Features/PasswordReset/PasswordResetController.coffee b/services/web/app/coffee/Features/PasswordReset/PasswordResetController.coffee index d574a6838f..fb0f75beee 100644 --- a/services/web/app/coffee/Features/PasswordReset/PasswordResetController.coffee +++ b/services/web/app/coffee/Features/PasswordReset/PasswordResetController.coffee @@ -1,5 +1,7 @@ PasswordResetHandler = require("./PasswordResetHandler") RateLimiter = require("../../infrastructure/RateLimiter") +AuthenticationController = require("../Authentication/AuthenticationController") +UserGetter = require("../User/UserGetter") logger = require "logger-sharelatex" module.exports = @@ -37,14 +39,19 @@ module.exports = title:"set_password" passwordResetToken: req.session.resetToken - setNewUserPassword: (req, res)-> + setNewUserPassword: (req, res, next)-> {passwordResetToken, password} = req.body if !password? or password.length == 0 or !passwordResetToken? or passwordResetToken.length == 0 return res.sendStatus 400 delete req.session.resetToken - PasswordResetHandler.setNewUserPassword passwordResetToken?.trim(), password?.trim(), (err, found) -> + PasswordResetHandler.setNewUserPassword passwordResetToken?.trim(), password?.trim(), (err, found, user_id) -> return next(err) if err? if found - res.sendStatus 200 + if req.body.login_after + UserGetter.getUser user_id, {email: 1}, (err, user) -> + return next(err) if err? + AuthenticationController.doLogin {email:user.email, password: password}, req, res, next + else + res.sendStatus 200 else - res.send 404, {message: req.i18n.translate("password_reset_token_expired")} + res.sendStatus 404 diff --git a/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee b/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee index f4bad9c593..4e67e9f1f4 100644 --- a/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee +++ b/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee @@ -23,11 +23,11 @@ module.exports = return callback(error) if error? callback null, true - setNewUserPassword: (token, password, callback = (error, found) ->)-> + setNewUserPassword: (token, password, callback = (error, found, user_id) ->)-> OneTimeTokenHandler.getValueFromTokenAndExpire token, (err, user_id)-> if err then return callback(err) if !user_id? - return callback null, false + return callback null, false, null AuthenticationManager.setUserPassword user_id, password, (err) -> if err then return callback(err) - callback null, true \ No newline at end of file + callback null, true, user_id \ No newline at end of file diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 94dc811e99..516b8a9546 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -100,7 +100,7 @@ module.exports = OneTimeTokenHandler.getNewToken user._id, { expiresIn: ONE_WEEK }, (err, token)-> return next(err) if err? - setNewPasswordUrl = "#{settings.siteUrl}/user/password/set?passwordResetToken=#{token}&email=#{encodeURIComponent(email)}" + setNewPasswordUrl = "#{settings.siteUrl}/user/activate?token=#{token}&user_id=#{user._id}" EmailHandler.sendEmail "registered", { to: user.email diff --git a/services/web/app/coffee/Features/User/UserPagesController.coffee b/services/web/app/coffee/Features/User/UserPagesController.coffee index 6ffb8ecdb8..5a967605fe 100644 --- a/services/web/app/coffee/Features/User/UserPagesController.coffee +++ b/services/web/app/coffee/Features/User/UserPagesController.coffee @@ -1,4 +1,6 @@ UserLocator = require("./UserLocator") +UserGetter = require("./UserGetter") +ErrorController = require("../Errors/ErrorController") logger = require("logger-sharelatex") Settings = require("settings-sharelatex") fs = require('fs') @@ -20,11 +22,33 @@ module.exports = sharedProjectData: sharedProjectData newTemplateData: newTemplateData new_email:req.query.new_email || "" + + activateAccountPage: (req, res) -> + # An 'activation' is actually just a password reset on an account that + # was set with a random password originally. + if !req.query?.user_id? or !req.query?.token? + return ErrorController.notFound(req, res) + + UserGetter.getUser req.query.user_id, {email: 1, loginCount: 1}, (error, user) -> + return next(error) if error? + if !user + return ErrorController.notFound(req, res) + if user.loginCount > 0 + # Already seen this user, so account must be activate + # This lets users keep clicking the 'activate' link in their email + # as a way to log in which, if I know our users, they will. + res.redirect "/login?email=#{encodeURIComponent(user.email)}" + else + res.render 'user/activate', + title: 'activate_account' + email: user.email, + token: req.query.token loginPage : (req, res)-> res.render 'user/login', title: 'login', - redir: req.query.redir + redir: req.query.redir, + email: req.query.email settingsPage : (req, res, next)-> logger.log user: req.session.user, "loading settings page" diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index a0c1b50d21..a34864f62b 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -77,6 +77,8 @@ module.exports = class Router webRouter.get '/blog', BlogController.getIndexPage webRouter.get '/blog/*', BlogController.getPage + webRouter.get '/user/activate', UserPagesController.activateAccountPage + webRouter.get '/user/settings', AuthenticationController.requireLogin(), UserPagesController.settingsPage webRouter.post '/user/settings', AuthenticationController.requireLogin(), UserController.updateUserSettings webRouter.post '/user/password/update', AuthenticationController.requireLogin(), UserController.changePassword diff --git a/services/web/app/views/user/activate.jade b/services/web/app/views/user/activate.jade new file mode 100644 index 0000000000..82543b17f8 --- /dev/null +++ b/services/web/app/views/user/activate.jade @@ -0,0 +1,64 @@ +extends ../layout + +block content + .content.content-alt + .container + .row + .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4 + .alert.alert-success You're one step away from activating your DataJoy account! + .row + .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4 + .card + .page-header + h1 Please set a password + form( + async-form="activate", + name="activationForm", + action="/user/password/set", + method="POST", + ng-cloak + ) + input(name='_csrf', type='hidden', value=csrfToken) + input( + type="hidden", + name="passwordResetToken", + value=token + ) + input(name='login_after', type='hidden', value="true") + .alert.alert-danger(ng-show="activationForm.response.error") + | #{translate("activation_token_expired")} + + .form-group + label(for='email') #{translate("email")} + input.form-control( + type='email', + name='email', + placeholder="email@example.com" + required, + ng-model="email", + ng-init="email = #{JSON.stringify(email)}", + ng-model-options="{ updateOn: 'blur' }", + disabled + ) + .form-group + label(for='password') #{translate("password")} + input.form-control#passwordField( + type='password', + name='password', + placeholder="********", + required, + ng-model="password", + complex-password, + focus="true" + ) + span.small.text-primary(ng-show="activationForm.password.$error.complexPassword", ng-bind-html="complexPasswordErrorMessage") + .actions + button.btn-primary.btn( + type='submit' + ng-disabled="activationForm.inflight || activationForm.password.$error.required|| activationForm.password.$error.complexPassword" + ) + span(ng-show="!activationForm.inflight") #{translate("activate")} + span(ng-show="activationForm.inflight") #{translate("activating")}... + + script(type='text/javascript'). + window.passwordStrengthOptions = !{JSON.stringify(settings.passwordStrengthOptions || {})} diff --git a/services/web/app/views/user/login.jade b/services/web/app/views/user/login.jade index 4d8848d8a6..a6587782bf 100644 --- a/services/web/app/views/user/login.jade +++ b/services/web/app/views/user/login.jade @@ -20,6 +20,7 @@ block content placeholder='email@example.com', ng-model="email", ng-model-options="{ updateOn: 'blur' }", + ng-init="email = #{JSON.stringify(email)}", focus="true" ) span.small.text-primary(ng-show="loginForm.email.$invalid && loginForm.email.$dirty") diff --git a/services/web/app/views/user/setPassword.jade b/services/web/app/views/user/setPassword.jade index 9da2beb065..3a6a588f34 100644 --- a/services/web/app/views/user/setPassword.jade +++ b/services/web/app/views/user/setPassword.jade @@ -16,10 +16,11 @@ block content ng-cloak ) input(type="hidden", name="_csrf", value=csrfToken) - form-messages(for="passwordResetForm") - .alert.alert-success(ng-show="passwordResetForm.response.success") - | #{translate("password_has_been_reset")}. - a(href='/login') #{translate("login_here")} + .alert.alert-success(ng-show="passwordResetForm.response.success") + | #{translate("password_has_been_reset")}. + a(href='/login') #{translate("login_here")} + .alert.alert-danger(ng-show="passwordResetForm.response.error") + | #{translate("password_reset_token_expired")} .form-group input.form-control#passwordField( diff --git a/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetControllerTests.coffee b/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetControllerTests.coffee index 140bf0b9d4..27852e3e07 100644 --- a/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetControllerTests.coffee @@ -1,4 +1,5 @@ should = require('chai').should() +expect = require("chai").expect SandboxedModule = require('sandboxed-module') assert = require('assert') path = require('path') @@ -21,6 +22,8 @@ describe "PasswordResetController", -> "./PasswordResetHandler":@PasswordResetHandler "logger-sharelatex": log:-> "../../infrastructure/RateLimiter":@RateLimiter + "../Authentication/AuthenticationController": @AuthenticationController = {} + "../User/UserGetter": @UserGetter = {} @email = "bob@bob.com " @token = "my security token that was emailed to me" @@ -101,7 +104,7 @@ describe "PasswordResetController", -> it "should send 404 if the token didn't work", (done)-> @PasswordResetHandler.setNewUserPassword.callsArgWith(2, null, false) - @res.send = (code)=> + @res.sendStatus = (code)=> code.should.equal 404 done() @PasswordResetController.setNewUserPassword @req, @res @@ -131,6 +134,19 @@ describe "PasswordResetController", -> @req.session.should.not.have.property 'resetToken' done() @PasswordResetController.setNewUserPassword @req, @res + + it "should login user if login_after is set", (done) -> + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, { email: "joe@example.com" }) + @PasswordResetHandler.setNewUserPassword.callsArgWith(2, null, true, @user_id = "user-id-123") + @req.body.login_after = "true" + @AuthenticationController.doLogin = (options, req, res, next)=> + @UserGetter.getUser.calledWith(@user_id).should.equal true + expect(options).to.deep.equal { + email: "joe@example.com", + password: @password + } + done() + @PasswordResetController.setNewUserPassword @req, @res describe "renderSetPasswordForm", -> diff --git a/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetHandlerTests.coffee b/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetHandlerTests.coffee index 47fa4f2836..b29839246a 100644 --- a/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetHandlerTests.coffee @@ -80,8 +80,9 @@ describe "PasswordResetHandler", -> it "should set the user password", (done)-> @OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1, null, @user_id) @AuthenticationManager.setUserPassword.callsArgWith(2) - @PasswordResetHandler.setNewUserPassword @token, @password, (err, found) => + @PasswordResetHandler.setNewUserPassword @token, @password, (err, found, user_id) => found.should.equal true + user_id.should.equal @user_id @AuthenticationManager.setUserPassword.calledWith(@user_id, @password).should.equal true done() diff --git a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee index e1883cb262..a96e8ba209 100644 --- a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee @@ -200,7 +200,7 @@ describe "UserController", -> @EmailHandler.sendEmail .calledWith("registered", { to: @user.email - setNewPasswordUrl: "#{@settings.siteUrl}/user/password/set?passwordResetToken=#{@token}&email=#{encodeURIComponent(@user.email)}" + setNewPasswordUrl: "#{@settings.siteUrl}/user/activate?token=#{@token}&user_id=#{@user_id}" }) .should.equal true @@ -208,7 +208,7 @@ describe "UserController", -> @res.json .calledWith({ email: @user.email - setNewPasswordUrl: "#{@settings.siteUrl}/user/password/set?passwordResetToken=#{@token}&email=#{encodeURIComponent(@user.email)}" + setNewPasswordUrl: "#{@settings.siteUrl}/user/activate?token=#{@token}&user_id=#{@user_id}" }) .should.equal true diff --git a/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee index a65be3e777..4b36ace39c 100644 --- a/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserPagesControllerTests.coffee @@ -12,18 +12,25 @@ describe "UserPagesController", -> @settings = {} @user = - _id:"kwjewkl" + _id: @user_id = "kwjewkl" features:{} + email: "joe@example.com" @UserLocator = findById: sinon.stub().callsArgWith(1, null, @user) + @UserGetter = + getUser: sinon.stub().callsArgWith(2, null, @user) @dropboxStatus = {} @DropboxHandler = getUserRegistrationStatus : sinon.stub().callsArgWith(1, null, @dropboxStatus) + @ErrorController = + notFound: sinon.stub() @UserPagesController = SandboxedModule.require modulePath, requires: "settings-sharelatex":@settings "logger-sharelatex": log:-> "./UserLocator": @UserLocator + "./UserGetter": @UserGetter + "../Errors/ErrorController": @ErrorController '../Dropbox/DropboxHandler': @DropboxHandler @req = query:{} @@ -103,4 +110,44 @@ describe "UserPagesController", -> @res.render = (page, opts)=> opts.user.should.equal @user done() - @UserPagesController.settingsPage @req, @res \ No newline at end of file + @UserPagesController.settingsPage @req, @res + + describe "activateAccountPage", -> + beforeEach -> + @req.query.user_id = @user_id + @req.query.token = @token = "mock-token-123" + + it "should 404 without a user_id", (done) -> + delete @req.query.user_id + @ErrorController.notFound = () -> + done() + @UserPagesController.activateAccountPage @req, @res + + it "should 404 without a token", (done) -> + delete @req.query.token + @ErrorController.notFound = () -> + done() + @UserPagesController.activateAccountPage @req, @res + + it "should 404 without a valid user_id", (done) -> + @UserGetter.getUser = sinon.stub().callsArgWith(2, null, null) + @ErrorController.notFound = () -> + done() + @UserPagesController.activateAccountPage @req, @res + + it "should redirect activated users to login", (done) -> + @user.loginCount = 1 + @res.redirect = (url) => + @UserGetter.getUser.calledWith(@user_id).should.equal true + url.should.equal "/login?email=#{encodeURIComponent(@user.email)}" + done() + @UserPagesController.activateAccountPage @req, @res + + it "render the activation page if the user has not logged in before", (done) -> + @user.loginCount = 0 + @res.render = (page, opts) => + page.should.equal "user/activate" + opts.email.should.equal @user.email + opts.token.should.equal @token + done() + @UserPagesController.activateAccountPage @req, @res \ No newline at end of file From c0dfdb3bd83d2d74a41934aea9238e8990f4230b Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 11 Dec 2015 14:25:17 +0000 Subject: [PATCH 4/5] Use translations for activation page --- services/web/app/coffee/Features/Email/EmailBuilder.coffee | 2 -- services/web/app/views/user/activate.jade | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 8d37f38c85..b6967cc4a8 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -15,8 +15,6 @@ templates.registered =

Click here to set your password and log in.

-

Once you have reset your password you can log in here.

-

If you have any questions or problems, please contact #{settings.adminEmail}.

""" diff --git a/services/web/app/views/user/activate.jade b/services/web/app/views/user/activate.jade index 82543b17f8..7961876389 100644 --- a/services/web/app/views/user/activate.jade +++ b/services/web/app/views/user/activate.jade @@ -5,12 +5,12 @@ block content .container .row .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4 - .alert.alert-success You're one step away from activating your DataJoy account! + .alert.alert-success #{translate("nearly_activated")} .row .col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4 .card .page-header - h1 Please set a password + h1 #{translate("please_set_a_password")} form( async-form="activate", name="activationForm", From e8f21986dd42c1920c6f8e52ee8d9f89c7a8364f Mon Sep 17 00:00:00 2001 From: James Allen Date: Fri, 11 Dec 2015 17:11:13 +0000 Subject: [PATCH 5/5] Refactor registration so it can be called from modules --- .../Features/User/UserController.coffee | 35 ++-------- .../User/UserRegistrationHandler.coffee | 31 ++++++++- .../AuthenticationManagerTests.coffee | 2 +- .../coffee/User/UserControllerTests.coffee | 67 +++++-------------- .../User/UserRegistrationHandlerTests.coffee | 56 +++++++++++++++- 5 files changed, 108 insertions(+), 83 deletions(-) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index 516b8a9546..f342e74351 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -8,10 +8,7 @@ metrics = require("../../infrastructure/Metrics") Url = require("url") AuthenticationManager = require("../Authentication/AuthenticationManager") UserUpdater = require("./UserUpdater") -EmailHandler = require("../Email/EmailHandler") -OneTimeTokenHandler = require "../Security/OneTimeTokenHandler" settings = require "settings-sharelatex" -crypto = require "crypto" module.exports = @@ -85,32 +82,12 @@ module.exports = if !email? or email == "" res.sendStatus 422 # Unprocessable Entity return - logger.log {email}, "registering new user" - UserRegistrationHandler.registerNewUser { - email: email - password: crypto.randomBytes(32).toString("hex") - }, (err, user)-> - if err? and err?.message != "EmailAlreadyRegistered" - return next(err) - - if err?.message == "EmailAlreadyRegistered" - logger.log {email}, "user already exists, resending welcome email" - - ONE_WEEK = 7 * 24 * 60 * 60 # seconds - OneTimeTokenHandler.getNewToken user._id, { expiresIn: ONE_WEEK }, (err, token)-> - return next(err) if err? - - setNewPasswordUrl = "#{settings.siteUrl}/user/activate?token=#{token}&user_id=#{user._id}" - - EmailHandler.sendEmail "registered", { - to: user.email - setNewPasswordUrl: setNewPasswordUrl - }, () -> - - res.json { - email: user.email - setNewPasswordUrl: setNewPasswordUrl - } + UserRegistrationHandler.registerNewUserAndSendActivationEmail email, (error, user, setNewPasswordUrl) -> + return next(error) if error? + res.json { + email: user.email + setNewPasswordUrl: setNewPasswordUrl + } changePassword : (req, res, next = (error) ->)-> metrics.inc "user.password-change" diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index 7dfde21c8a..7465268023 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -5,8 +5,12 @@ AuthenticationManager = require("../Authentication/AuthenticationManager") NewsLetterManager = require("../Newsletter/NewsletterManager") async = require("async") logger = require("logger-sharelatex") +crypto = require("crypto") +EmailHandler = require("../Email/EmailHandler") +OneTimeTokenHandler = require "../Security/OneTimeTokenHandler" +settings = require "settings-sharelatex" -module.exports = +module.exports = UserRegistrationHandler = validateEmail : (email) -> re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\ ".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA -Z\-0-9]+\.)+[a-zA-Z]{2,}))$/ return re.test(email) @@ -58,6 +62,31 @@ module.exports = ], (err)-> logger.log user: user, "registered" callback(err, user) + + registerNewUserAndSendActivationEmail: (email, callback = (error, user, setNewPasswordUrl) ->) -> + logger.log {email}, "registering new user" + UserRegistrationHandler.registerNewUser { + email: email + password: crypto.randomBytes(32).toString("hex") + }, (err, user)-> + if err? and err?.message != "EmailAlreadyRegistered" + return next(err) + + if err?.message == "EmailAlreadyRegistered" + logger.log {email}, "user already exists, resending welcome email" + + ONE_WEEK = 7 * 24 * 60 * 60 # seconds + OneTimeTokenHandler.getNewToken user._id, { expiresIn: ONE_WEEK }, (err, token)-> + return next(err) if err? + + setNewPasswordUrl = "#{settings.siteUrl}/user/activate?token=#{token}&user_id=#{user._id}" + + EmailHandler.sendEmail "registered", { + to: user.email + setNewPasswordUrl: setNewPasswordUrl + }, () -> + + callback null, user, setNewPasswordUrl diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee index 3360099cef..3a5c60cf66 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee @@ -71,7 +71,7 @@ describe "AuthenticationManager", -> @bcrypt.genSalt = sinon.stub().callsArgWith(1, null, @salt) @bcrypt.hash = sinon.stub().callsArgWith(2, null, @hashedPassword) @db.users.update = sinon.stub().callsArg(2) - @AuthenticationManager.setUserPassword(@user_id, @unencryptedPassword, @callback) + @AuthenticationManager.setUserPassword(@user_id, @password, @callback) it "should update the user's password in the database", -> @db.users.update diff --git a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee index a96e8ba209..5010f91299 100644 --- a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee @@ -40,10 +40,6 @@ describe "UserController", -> autoAllocate:sinon.stub() @UserUpdater = changeEmailAddress:sinon.stub() - @EmailHandler = - sendEmail:sinon.stub().callsArgWith(2) - @OneTimeTokenHandler = - getNewToken: sinon.stub() @settings = siteUrl: "sharelatex.example.com" @UserController = SandboxedModule.require modulePath, requires: @@ -57,9 +53,6 @@ describe "UserController", -> "../Authentication/AuthenticationManager": @AuthenticationManager "../Referal/ReferalAllocator":@ReferalAllocator "../Subscription/SubscriptionDomainHandler":@SubscriptionDomainHandler - "../Email/EmailHandler": @EmailHandler - "../Security/OneTimeTokenHandler": @OneTimeTokenHandler - "crypto": @crypto = {} "settings-sharelatex": @settings "logger-sharelatex": {log:->} @@ -175,51 +168,23 @@ describe "UserController", -> describe "register", -> beforeEach -> - @req.body.email = @user.email = "email@example.com" - @crypto.randomBytes = sinon.stub().returns({toString: () => @password = "mock-password"}) - @OneTimeTokenHandler.getNewToken.callsArgWith(2, null, @token = "mock-token") + @UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon.stub().callsArgWith(1, null, @user, @url = "mock/url") + @req.body.email = @user.email = @email = "email@example.com" + @UserController.register @req, @res - describe "with a new user", -> - beforeEach -> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @UserController.register @req, @res - - it "should ask the UserRegistrationHandler to register user", -> - @UserRegistrationHandler.registerNewUser - .calledWith({ - email: @req.body.email - password: @password - }).should.equal true - - it "should generate a new password reset token", -> - @OneTimeTokenHandler.getNewToken - .calledWith(@user_id, expiresIn: 7 * 24 * 60 * 60) - .should.equal true - - it "should send a registered email", -> - @EmailHandler.sendEmail - .calledWith("registered", { - to: @user.email - setNewPasswordUrl: "#{@settings.siteUrl}/user/activate?token=#{@token}&user_id=#{@user_id}" - }) - .should.equal true - - it "should return the user", -> - @res.json - .calledWith({ - email: @user.email - setNewPasswordUrl: "#{@settings.siteUrl}/user/activate?token=#{@token}&user_id=#{@user_id}" - }) - .should.equal true - - describe "with a user that already exists", -> - beforeEach -> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, new Error("EmailAlreadyRegistered"), @user) - @UserController.register @req, @res - - it "should still generate a new password token and email", -> - @OneTimeTokenHandler.getNewToken.called.should.equal true - @EmailHandler.sendEmail.called.should.equal true + it "should register the user and send them an email", -> + @UserRegistrationHandler.registerNewUserAndSendActivationEmail + .calledWith(@email) + .should.equal true + + it "should return the user and activation url", -> + console.log @res.json.args + @res.json + .calledWith({ + email: @email, + setNewPasswordUrl: @url + }) + .should.equal true describe "changePassword", -> diff --git a/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee index 72beb10ade..60ae1719a7 100644 --- a/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee @@ -10,7 +10,7 @@ describe "UserRegistrationHandler", -> beforeEach -> @user = - _id: "31j2lk21kjl" + _id: @user_id = "31j2lk21kjl" @User = findOne:sinon.stub() update: sinon.stub().callsArgWith(2) @@ -20,12 +20,20 @@ describe "UserRegistrationHandler", -> setUserPassword: sinon.stub().callsArgWith(2) @NewsLetterManager = subscribe: sinon.stub().callsArgWith(1) + @EmailHandler = + sendEmail:sinon.stub().callsArgWith(2) + @OneTimeTokenHandler = + getNewToken: sinon.stub() @handler = SandboxedModule.require modulePath, requires: "../../models/User": {User:@User} "./UserCreator": @UserCreator "../Authentication/AuthenticationManager":@AuthenticationManager "../Newsletter/NewsletterManager":@NewsLetterManager "logger-sharelatex": @logger = { log: sinon.stub() } + "crypto": @crypto = {} + "../Email/EmailHandler": @EmailHandler + "../Security/OneTimeTokenHandler": @OneTimeTokenHandler + "settings-sharelatex": @settings = {siteUrl: "http://sl.example.com"} @passingRequest = {email:"something@email.com", password:"123"} @@ -128,4 +136,50 @@ describe "UserRegistrationHandler", -> it "should call the ReferalAllocator", (done)-> done() + describe "registerNewUserAndSendActivationEmail", -> + beforeEach -> + @email = "email@example.com" + @crypto.randomBytes = sinon.stub().returns({toString: () => @password = "mock-password"}) + @OneTimeTokenHandler.getNewToken.callsArgWith(2, null, @token = "mock-token") + @handler.registerNewUser = sinon.stub() + @callback = sinon.stub() + + describe "with a new user", -> + beforeEach -> + @handler.registerNewUser.callsArgWith(1, null, @user) + @handler.registerNewUserAndSendActivationEmail @email, @callback + + it "should ask the UserRegistrationHandler to register user", -> + @handler.registerNewUser + .calledWith({ + email: @email + password: @password + }).should.equal true + + it "should generate a new password reset token", -> + + @OneTimeTokenHandler.getNewToken + .calledWith(@user_id, expiresIn: 7 * 24 * 60 * 60) + .should.equal true + it "should send a registered email", -> + @EmailHandler.sendEmail + .calledWith("registered", { + to: @user.email + setNewPasswordUrl: "#{@settings.siteUrl}/user/activate?token=#{@token}&user_id=#{@user_id}" + }) + .should.equal true + + it "should return the user", -> + @callback + .calledWith(null, @user, "#{@settings.siteUrl}/user/activate?token=#{@token}&user_id=#{@user_id}") + .should.equal true + + describe "with a user that already exists", -> + beforeEach -> + @handler.registerNewUser.callsArgWith(1, new Error("EmailAlreadyRegistered"), @user) + @handler.registerNewUserAndSendActivationEmail @email, @callback + + it "should still generate a new password token and email", -> + @OneTimeTokenHandler.getNewToken.called.should.equal true + @EmailHandler.sendEmail.called.should.equal true \ No newline at end of file