From 9b8cf7bcfadeee29d3cbdfd6ba9ab55b4455be1a Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 19 Mar 2015 14:22:48 +0000 Subject: [PATCH] Remove public registration and require that a user be registered by an admin --- .../coffee/Features/Email/EmailBuilder.coffee | 48 +++---- .../Layouts/NotificationEmailLayout.coffee | 38 +----- .../PasswordResetTokenHandler.coffee | 9 +- .../ServerAdmin/AdminController.coffee | 5 +- .../Features/User/UserController.coffee | 56 ++++---- .../User/UserRegistrationHandler.coffee | 2 +- services/web/app/coffee/router.coffee | 4 +- .../views/{admin.jade => admin/index.jade} | 2 +- services/web/app/views/admin/register.jade | 40 ++++++ services/web/app/views/layout/navbar.jade | 11 +- services/web/public/coffee/main.coffee | 1 + .../public/coffee/main/register-users.coffee | 32 +++++ .../coffee/Email/EmailBuilderTests.coffee | 14 -- .../PasswordResetTokenHandlerTests.coffee | 6 + .../coffee/User/UserControllerTests.coffee | 121 ++++++++---------- .../User/UserRegistrationHandlerTests.coffee | 5 +- 16 files changed, 217 insertions(+), 177 deletions(-) rename services/web/app/views/{admin.jade => admin/index.jade} (99%) create mode 100644 services/web/app/views/admin/register.jade create mode 100644 services/web/public/coffee/main/register-users.coffee diff --git a/services/web/app/coffee/Features/Email/EmailBuilder.coffee b/services/web/app/coffee/Features/Email/EmailBuilder.coffee index 92f0704a48..7e04ae710f 100644 --- a/services/web/app/coffee/Features/Email/EmailBuilder.coffee +++ b/services/web/app/coffee/Features/Email/EmailBuilder.coffee @@ -6,25 +6,19 @@ settings = require("settings-sharelatex") templates = {} -templates.welcome = - subject: _.template "Welcome to ShareLaTeX" +templates.registered = + subject: _.template "Activate your #{settings.appName} Account" layout: PersonalEmailLayout - type:"lifecycle" - compiledTemplate: _.template ''' -

Hi <%= first_name %>,

+ type: "notification" + compiledTemplate: _.template """ +

Congratulations, you've just had an account created for you on #{settings.appName} with the email address "<%= to %>".

-

Thanks for signing up to ShareLaTeX! If you ever get lost, you can log in again here with the email address "<%= to %>".

+

Click here to set your password and log in.

-

If you're new to LaTeX, take a look at our Help Guides and Templates.

+

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

-

-Regards,
-Henry
-ShareLaTeX Co-founder -

- -

PS. We love talking to our users about ShareLaTeX. Reply to this email to get in touch us with us directly, whatever the reason. Questions, comments, problems, suggestions, all welcome!

-''' +

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

+""" templates.canceledSubscription = subject: _.template "ShareLaTeX thoughts" @@ -44,16 +38,16 @@ ShareLaTeX Co-founder ''' templates.passwordResetRequested = - subject: _.template "Password Reset - ShareLatex.com" + subject: _.template "Password Reset - #{settings.appName}" layout: NotificationEmailLayout type:"notification" - compiledTemplate: _.template ''' -

Password Reset

+ compiledTemplate: _.template """ +

Password Reset

-We got a request to reset your ShareLaTeX password. +We got a request to reset your #{settings.appName} password.

-
+
@@ -70,18 +64,17 @@ If you didn't request a password reset, let us know.

Thank you

-

ShareLatex.com

-''' +

#{settings.appName}

+""" templates.projectSharedWithYou = subject: _.template "<%= owner.email %> wants to share <%= project.name %> with you" layout: NotificationEmailLayout type:"notification" - compiledTemplate: _.template ''' + compiledTemplate: _.template """

Hi, <%= owner.email %> wants to share '<%= project.name %>' with you

-

 

-

Thank you

-

ShareLatex.com

- -''' +

#{settings.appName}

+""" module.exports = diff --git a/services/web/app/coffee/Features/Email/Layouts/NotificationEmailLayout.coffee b/services/web/app/coffee/Features/Email/Layouts/NotificationEmailLayout.coffee index e67ef058e0..295951aa8c 100644 --- a/services/web/app/coffee/Features/Email/Layouts/NotificationEmailLayout.coffee +++ b/services/web/app/coffee/Features/Email/Layouts/NotificationEmailLayout.coffee @@ -1,6 +1,7 @@ _ = require("underscore") +settings = require "settings-sharelatex" -module.exports = _.template ''' +module.exports = _.template """ @@ -311,12 +312,8 @@ module.exports = _.template ''' -
- - - - - + + #{settings.appName}
@@ -346,31 +343,6 @@ module.exports = _.template ''' - - - - - - - -
- - - - - - -
- -
- - -
- - - @@ -380,4 +352,4 @@ module.exports = _.template ''' -''' \ No newline at end of file +""" \ No newline at end of file diff --git a/services/web/app/coffee/Features/PasswordReset/PasswordResetTokenHandler.coffee b/services/web/app/coffee/Features/PasswordReset/PasswordResetTokenHandler.coffee index 522e8dcfd6..1fc5680ec2 100644 --- a/services/web/app/coffee/Features/PasswordReset/PasswordResetTokenHandler.coffee +++ b/services/web/app/coffee/Features/PasswordReset/PasswordResetTokenHandler.coffee @@ -10,12 +10,17 @@ buildKey = (token)-> return "password_token:#{token}" module.exports = - getNewToken: (user_id, callback)-> + getNewToken: (user_id, options = {}, callback)-> + # options is optional + if typeof options == "function" + callback = options + options = {} + expiresIn = options.expiresIn or ONE_HOUR_IN_S logger.log user_id:user_id, "generating token for password reset" token = crypto.randomBytes(32).toString("hex") multi = rclient.multi() multi.set buildKey(token), user_id - multi.expire buildKey(token), ONE_HOUR_IN_S + multi.expire buildKey(token), expiresIn multi.exec (err)-> callback(err, token) diff --git a/services/web/app/coffee/Features/ServerAdmin/AdminController.coffee b/services/web/app/coffee/Features/ServerAdmin/AdminController.coffee index edc423fa8d..e875413908 100755 --- a/services/web/app/coffee/Features/ServerAdmin/AdminController.coffee +++ b/services/web/app/coffee/Features/ServerAdmin/AdminController.coffee @@ -39,10 +39,13 @@ module.exports = AdminController = SystemMessageManager.getMessagesFromDB (error, systemMessages) -> return next(error) if error? - res.render 'admin', + res.render 'admin/index', title: 'System Admin' openSockets: openSockets systemMessages: systemMessages + + registerNewUser: (req, res, next) -> + res.render 'admin/register' dissconectAllUsers: (req, res)=> logger.warn "disconecting everyone" diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index f2af08f703..0b2031f1b6 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -12,6 +12,9 @@ ReferalAllocator = require("../Referal/ReferalAllocator") UserUpdater = require("./UserUpdater") SubscriptionDomainAllocator = require("../Subscription/SubscriptionDomainAllocator") EmailHandler = require("../Email/EmailHandler") +PasswordResetTokenHandler = require "../PasswordReset/PasswordResetTokenHandler" +settings = require "settings-sharelatex" +crypto = require "crypto" module.exports = @@ -81,34 +84,37 @@ module.exports = res.redirect '/login' register : (req, res, next = (error) ->)-> - logger.log email: req.body.email, "attempted register" - redir = Url.parse(req.body.redir or "/project").path - UserRegistrationHandler.registerNewUser req.body, (err, user)-> - if err? and err?.message == "EmailAlreadyRegistered" - return AuthenticationController.login req, res - else if err? - next(err) - else - metrics.inc "user.register.success" - ReferalAllocator.allocate req.session.referal_id, user._id, req.session.referal_source, req.session.referal_medium - SubscriptionDomainAllocator.autoAllocate(user) + email = req.body.email + if !email? or email == "" + res.send 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" - EmailHandler.sendEmail "welcome", { - first_name:user.first_name + # TODO: Make a long term token. + ONE_WEEK = 7 * 24 * 60 * 60 # seconds + PasswordResetTokenHandler.getNewToken user._id, { expiresIn: ONE_WEEK }, (err, token)-> + return next(err) if err? + + setNewPasswordUrl = "#{settings.siteUrl}/user/password/set?passwordResetToken=#{token}" + + EmailHandler.sendEmail "registered", { to: user.email + setNewPasswordUrl: setNewPasswordUrl }, () -> - - AuthenticationController.establishUserSession req, user, (error) -> - return callback(error) if error? - req.session.justRegistered = true - res.send - redir:redir - id:user._id.toString() - first_name: user.first_name - last_name: user.last_name - email: user.email - created: Date.now() - + + 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 cae6483b0f..7dfde21c8a 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee @@ -45,7 +45,7 @@ module.exports = if err? return callback err if user?.holdingAccount == false - return callback(new Error("EmailAlreadyRegistered")) + return callback(new Error("EmailAlreadyRegistered"), user) self._createNewUserIfRequired user, userDetails, (err, user)-> if err? return callback(err) diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index fe0827f516..5e1ee1ba87 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -54,8 +54,8 @@ module.exports = class Router app.get '/logout', UserController.logout app.get '/restricted', SecurityManager.restricted + # Left as a placeholder for implementing a public register page app.get '/register', UserPagesController.registerPage - app.post '/register', UserController.register EditorRouter.apply(app, httpAuth) CollaboratorsRouter.apply(app) @@ -157,6 +157,8 @@ module.exports = class Router #Admin Stuff app.get '/admin', SecurityManager.requestIsAdmin, AdminController.index + app.get '/admin/register', SecurityManager.requestIsAdmin, AdminController.registerNewUser + app.post '/admin/register', SecurityManager.requestIsAdmin, UserController.register app.post '/admin/closeEditor', SecurityManager.requestIsAdmin, AdminController.closeEditor app.post '/admin/dissconectAllUsers', SecurityManager.requestIsAdmin, AdminController.dissconectAllUsers app.post '/admin/syncUserToSubscription', SecurityManager.requestIsAdmin, AdminController.syncUserToSubscription diff --git a/services/web/app/views/admin.jade b/services/web/app/views/admin/index.jade similarity index 99% rename from services/web/app/views/admin.jade rename to services/web/app/views/admin/index.jade index 8b03b60253..d28d88c929 100644 --- a/services/web/app/views/admin.jade +++ b/services/web/app/views/admin/index.jade @@ -1,4 +1,4 @@ -extends layout +extends ../layout block content .content.content-alt diff --git a/services/web/app/views/admin/register.jade b/services/web/app/views/admin/register.jade new file mode 100644 index 0000000000..4078c49484 --- /dev/null +++ b/services/web/app/views/admin/register.jade @@ -0,0 +1,40 @@ +extends ../layout + +block content + .content.content-alt + .container + .row + .col-md-12 + .card(ng-controller="RegisterUsersController") + .page-header + h1 Register New Users + form.form + .row + .col-md-4.col-xs-8 + input.form-control( + name="email", + type="text", + placeholder="jane@example.com, joe@example.com", + ng-model="inputs.emails", + on-enter="registerUsers()" + ) + .col-md-8.col-xs-4 + button.btn.btn-primary(ng-click="registerUsers()") #{translate("register")} + + .row-spaced(ng-show="error").ng-cloak.text-danger + p Sorry, an error occured + + .row-spaced(ng-show="users.length > 0").ng-cloak.text-success + p We've sent out welcome emails to the registered users. + p You can also manually send them URLs below to allow them to reset their password and log in for the first time. + p (Password reset tokens will expire after one week and the user will need registering again). + + hr(ng-show="users.length > 0").ng-cloak + table(ng-show="users.length > 0").table.table-striped.ng-cloak + tr + th #{translate("email")} + th Set Password Url + tr(ng-repeat="user in users") + td {{ user.email }} + td(style="word-break: break-all;") {{ user.setNewPasswordUrl }} + \ No newline at end of file diff --git a/services/web/app/views/layout/navbar.jade b/services/web/app/views/layout/navbar.jade index e91e8cd824..36ce731292 100644 --- a/services/web/app/views/layout/navbar.jade +++ b/services/web/app/views/layout/navbar.jade @@ -9,8 +9,17 @@ nav.navbar.navbar-default a(href='/').navbar-brand .navbar-collapse.collapse(collapse="navCollapsed") - + ul.nav.navbar-nav.navbar-right + if (session && session.user && session.user.isAdmin) + li.dropdown(class="subdued") + a.dropdown-toggle(href) + | Admin + b.caret + ul.dropdown-menu + li + a(href="/admin/register") Register New Users + each item in nav.header if ((item.only_when_logged_in && session && session.user) || (item.only_when_logged_out && (!session || !session.user)) || (!item.only_when_logged_out && !item.only_when_logged_in)) if item.dropdown diff --git a/services/web/public/coffee/main.coffee b/services/web/public/coffee/main.coffee index f8bd54cd4b..c3184044dd 100644 --- a/services/web/public/coffee/main.coffee +++ b/services/web/public/coffee/main.coffee @@ -14,6 +14,7 @@ define [ "main/subscription-dashboard" "main/new-subscription" "main/annual-upgrade" + "main/register-users" "analytics/AbTestingManager" "directives/asyncForm" "directives/stopPropagation" diff --git a/services/web/public/coffee/main/register-users.coffee b/services/web/public/coffee/main/register-users.coffee new file mode 100644 index 0000000000..1084c49e33 --- /dev/null +++ b/services/web/public/coffee/main/register-users.coffee @@ -0,0 +1,32 @@ +define [ + "base" +], (App) -> + App.controller "RegisterUsersController", ($scope, queuedHttp) -> + $scope.users = [] + + $scope.inputs = + emails: "" + + parseEmails = (emailsString)-> + regexBySpaceOrComma = /[\s,]+/ + emails = emailsString.split(regexBySpaceOrComma) + emails = _.map emails, (email)-> + email = email.trim() + emails = _.select emails, (email)-> + email.indexOf("@") != -1 + return emails + + $scope.registerUsers = () -> + emails = parseEmails($scope.inputs.emails) + $scope.error = false + for email in emails + queuedHttp + .post("/admin/register", { + email: email, + _csrf: window.csrfToken + }) + .success (user) -> + $scope.users.push user + $scope.inputs.emails = "" + .error () -> + $scope.error = true \ No newline at end of file diff --git a/services/web/test/UnitTests/coffee/Email/EmailBuilderTests.coffee b/services/web/test/UnitTests/coffee/Email/EmailBuilderTests.coffee index 3b13f99c25..bd572ecd4b 100644 --- a/services/web/test/UnitTests/coffee/Email/EmailBuilderTests.coffee +++ b/services/web/test/UnitTests/coffee/Email/EmailBuilderTests.coffee @@ -18,20 +18,6 @@ describe "Email Templator ", -> "settings-sharelatex":@settings "logger-sharelatex": log:-> - describe "welcomeEmail", -> - - beforeEach -> - @opts = - to:"bob@bob.com" - first_name:"bob" - @email = @EmailBuilder.buildEmail("welcome", @opts) - - it "should insert the first_name into the template", -> - @email.html.indexOf(@opts.first_name).should.not.equal -1 - - it "should not have undefined in it", -> - @email.html.indexOf("undefined").should.equal -1 - describe "projectSharedWithYou", -> beforeEach -> @opts = diff --git a/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetTokenHandlerTests.coffee b/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetTokenHandlerTests.coffee index bbc9ff9ea6..7de011740c 100644 --- a/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetTokenHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/PasswordReset/PasswordResetTokenHandlerTests.coffee @@ -48,6 +48,12 @@ describe "PasswordResetTokenHandler", -> err.should.exist done() + it "should allow the expiry time to be overridden", (done) -> + @redisMulti.exec.callsArgWith(0) + @ttl = 42 + @PasswordResetTokenHandler.getNewToken @user_id, {expiresIn: @ttl}, (err, token) => + @redisMulti.expire.calledWith("password_token:#{@stubbedToken.toString("hex")}", @ttl).should.equal true + done() describe "getUserIdFromTokenAndExpire", -> diff --git a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee index 6af4e53072..eb8033e84e 100644 --- a/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserControllerTests.coffee @@ -42,6 +42,10 @@ describe "UserController", -> changeEmailAddress:sinon.stub() @EmailHandler = sendEmail:sinon.stub().callsArgWith(2) + @PasswordResetTokenHandler = + getNewToken: sinon.stub() + @settings = + siteUrl: "sharelatex.example.com" @UserController = SandboxedModule.require modulePath, requires: "./UserLocator": @UserLocator "./UserDeleter": @UserDeleter @@ -54,6 +58,9 @@ describe "UserController", -> "../Referal/ReferalAllocator":@ReferalAllocator "../Subscription/SubscriptionDomainAllocator":@SubscriptionDomainAllocator "../Email/EmailHandler": @EmailHandler + "../PasswordReset/PasswordResetTokenHandler": @PasswordResetTokenHandler + "crypto": @crypto = {} + "settings-sharelatex": @settings "logger-sharelatex": {log:->} @@ -63,7 +70,9 @@ describe "UserController", -> user : _id : @user_id body:{} - @res = {} + @res = + send: sinon.stub() + json: sinon.stub() @next = sinon.stub() describe "deleteUser", -> @@ -165,76 +174,52 @@ describe "UserController", -> describe "register", -> - - it "should ask the UserRegistrationHandler to register user", (done)-> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @res.send = => - @UserRegistrationHandler.registerNewUser.calledWith(@req.body).should.equal true - done() - @UserController.register @req, @res - - it "should try and log the user in if there is an EmailAlreadyRegistered error", (done)-> - - @UserRegistrationHandler.registerNewUser.callsArgWith(1, new Error("EmailAlreadyRegistered")) - @AuthenticationController.login = (req, res)=> - assert.deepEqual req, @req - assert.deepEqual res, @res - done() - @UserController.register @req, @res - - it "should put the user on the session and mark them as justRegistered", (done)-> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @res.send = => - @AuthenticationController.establishUserSession - .calledWith(@req, @user) - .should.equal true - assert.equal @req.session.justRegistered, true - done() - @UserController.register @req, @res - - it "should redirect to project page", (done)-> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @res.send = (opts)=> - opts.redir.should.equal "/project" - done() - @UserController.register @req, @res - - - it "should redirect passed redir if it exists", (done)-> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @req.body.redir = "/somewhere" - @res.send = (opts)=> - opts.redir.should.equal "/somewhere" - done() - @UserController.register @req, @res - - it "should allocate the referals", (done)-> - @req.session = - referal_id : "23123" - referal_source : "email" - referal_medium : "bob" - - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @req.body.redir = "/somewhere" - @res.send = (opts)=> - @ReferalAllocator.allocate.calledWith(@req.session.referal_id, @user._id, @req.session.referal_source, @req.session.referal_medium).should.equal true - done() - @UserController.register @req, @res + beforeEach -> + @req.body.email = @user.email = "email@example.com" + @crypto.randomBytes = sinon.stub().returns({toString: () => @password = "mock-password"}) + @PasswordResetTokenHandler.getNewToken.callsArgWith(2, null, @token = "mock-token") + + describe "with a new user", -> + beforeEach -> + @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) + @UserController.register @req, @res - it "should auto allocate the subscription for that domain", (done)-> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @res.send = (opts)=> - @SubscriptionDomainAllocator.autoAllocate.calledWith(@user).should.equal true - done() - @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", -> + @PasswordResetTokenHandler.getNewToken + .calledWith(@user_id, expiresIn: 7 * 24 * 60 * 60) + .should.equal true - it "should send a welcome email", (done)-> - @UserRegistrationHandler.registerNewUser.callsArgWith(1, null, @user) - @res.send = (opts)=> - @EmailHandler.sendEmail.calledWith("welcome").should.equal true - done() - @UserController.register @req, @res + it "should send a registered email", -> + @EmailHandler.sendEmail + .calledWith("registered", { + to: @user.email + setNewPasswordUrl: "#{@settings.siteUrl}/user/password/set?passwordResetToken=#{@token}" + }) + .should.equal true + + it "should return the user", -> + @res.json + .calledWith({ + email: @user.email + setNewPasswordUrl: "#{@settings.siteUrl}/user/password/set?passwordResetToken=#{@token}" + }) + .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", -> + @PasswordResetTokenHandler.getNewToken.called.should.equal true + @EmailHandler.sendEmail.called.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 3a7b75441f..72beb10ade 100644 --- a/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserRegistrationHandlerTests.coffee @@ -85,9 +85,10 @@ describe "UserRegistrationHandler", -> done() it "should return email registered in the error if there is a non holdingAccount there", (done)-> - @User.findOne.callsArgWith(1, null, {holdingAccount:false}) - @handler.registerNewUser @passingRequest, (err)=> + @User.findOne.callsArgWith(1, null, @user = {holdingAccount:false}) + @handler.registerNewUser @passingRequest, (err, user)=> err.should.deep.equal new Error("EmailAlreadyRegistered") + user.should.deep.equal @user done() describe "validRequest", ->