Improve pre-registered account activation process

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"

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

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
+				callback null, true, user_id

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

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"

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

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 || {})}

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")

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(

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", ->
 

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()			
 

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
 

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
+			@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