Merge remote-tracking branch 'origin/master' into afc-email-tokens

services/web/app/coffee/Features/User/UserGetter.coffee

@@ -21,6 +21,10 @@ module.exports = UserGetter =
 
 		db.users.findOne query, projection, callback
 
+	getUserEmail: (userId, callback = (error, email) ->) ->
+		@getUser userId, { email: 1 }, (error, user) ->
+			callback(error, user?.email)
+
 	getUserByMainEmail: (email, projection, callback = (error, user) ->) ->
 		email = email.trim()
 		if arguments.length == 2
@@ -28,6 +32,20 @@ module.exports = UserGetter =
 			projection = {}
 		db.users.findOne email: email, projection, callback
 
+	getUserByAnyEmail: (email, projection, callback = (error, user) ->) ->
+		email = email.trim()
+		if arguments.length == 2
+			callback = projection
+			projection = {}
+		# $exists: true MUST be set to use the partial index
+		query = emails: { $exists: true }, 'emails.email': email
+		db.users.findOne query, projection, (error, user) =>
+			return callback(error, user) if error? or user?
+
+			# While multiple emails are being rolled out, check for the main email as
+			# well
+			@getUserByMainEmail email, projection, callback
+
 	getUsers: (user_ids, projection, callback = (error, users) ->) ->
 		try
 			user_ids = user_ids.map (u) -> ObjectId(u.toString())
@@ -48,7 +66,9 @@ module.exports = UserGetter =
 
 [
 	'getUser',
+	'getUserEmail',
 	'getUserByMainEmail',
+	'getUserByAnyEmail',
 	'getUsers',
 	'getUserOrUserStubById'
 ].map (method) ->

services/web/app/coffee/Features/User/UserUpdater.coffee

@@ -2,6 +2,7 @@ logger = require("logger-sharelatex")
 mongojs = require("../../infrastructure/mongojs")
 metrics = require("metrics-sharelatex")
 db = mongojs.db
+async = require("async")
 ObjectId = mongojs.ObjectId
 UserGetter = require("./UserGetter")
 
@@ -11,23 +12,89 @@ module.exports = UserUpdater =
 			query = _id: ObjectId(query)
 		else if query instanceof ObjectId
 			query = _id: query
+		else if typeof query._id == "string"
+			query._id = ObjectId(query._id)
 
 		db.users.update query, update, callback
 
 
-	changeEmailAddress: (user_id, newEmail, callback)->
-		self = @
-		logger.log user_id:user_id, newEmail:newEmail, "updaing email address of user"
-		UserGetter.getUserByMainEmail newEmail, (error, user) ->
-			if user?
-				return callback({message:"alread_exists"})
-			self.updateUser user_id.toString(), {
-				$set: { "email": newEmail},
-			}, (err) ->
-				if err?
-					logger.err err:err, "problem updating users email"
-					return callback(err)
+	#
+	# DEPRECATED
+	#
+	# Change the user's main email address by adding a new email, switching the
+	# default email and removing the old email.  Prefer manipulating multiple
+	# emails and the default rather than calling this method directly
+	#
+	changeEmailAddress: (userId, newEmail, callback)->
+		logger.log userId: userId, newEmail: newEmail, "updaing email address of user"
+
+		oldEmail = null
+		async.series [
+			(cb) ->
+				UserGetter.getUserEmail userId, (error, email) ->
+					oldEmail = email
+					cb(error)
+			(cb) -> UserUpdater.addEmailAddress userId, newEmail, cb
+			(cb) -> UserUpdater.setDefaultEmailAddress userId, newEmail, cb
+			(cb) -> UserUpdater.removeEmailAddress userId, oldEmail, cb
+		], callback
+
+
+	# Add a new email address for the user. Email cannot be already used by this
+	# or any other user
+	addEmailAddress: (userId, newEmail, callback) ->
+		@_ensureUniqueEmailAddress newEmail, (error) =>
+			return callback(error) if error?
+
+			update = $push: emails: email: newEmail, createdAt: new Date()
+			@updateUser userId, update, (error) ->
+				if error?
+					logger.err error: error, 'problem updating users emails'
+					return callback(error)
 				callback()
 
 
-metrics.timeAsyncMethod UserUpdater, 'updateUser', 'mongo.UserUpdater', logger
+	# remove one of the user's email addresses. The email cannot be the user's
+	# default email address
+	removeEmailAddress: (userId, email, callback) ->
+		query = _id: userId, email: $ne: email
+		update = $pull: emails: email: email
+		@updateUser query, update, (error, res) ->
+			if error?
+				logger.err error:error, 'problem removing users email'
+				return callback(error)
+			if res.nMatched == 0
+				return callback(new Error('Cannot remove default email'))
+			callback()
+
+
+	# set the default email address by setting the `email` attribute. The email
+	# must be one of the user's multiple emails (`emails` attribute)
+	setDefaultEmailAddress: (userId, email, callback) ->
+		query = _id: userId, 'emails.email': email
+		update = $set: email: email
+		@updateUser query, update, (error, res) ->
+			if error?
+				logger.err error:error, 'problem setting default emails'
+				return callback(error)
+			if res.nMatched == 0
+				return callback(new Error('Default email does not belong to user'))
+			callback()
+
+
+	# check for duplicate email address. This is also enforced at the DB level
+	_ensureUniqueEmailAddress: (newEmail, callback) ->
+		UserGetter.getUserByAnyEmail newEmail, (error, user) ->
+			return callback(message: 'alread_exists') if user?
+			callback()
+
+
+[
+	'updateUser'
+	'changeEmailAddress'
+	'setDefaultEmailAddress'
+	'addEmailAddress'
+	'removeEmailAddress'
+	'_ensureUniqueEmailAddress'
+].map (method) ->
+	metrics.timeAsyncMethod(UserUpdater, method, 'mongo.UserUpdater', logger)

services/web/test/acceptance/coffee/RegistrationTests.coffee

@@ -146,6 +146,15 @@ describe "LoginViaRegistration", ->
 
 	describe "[Security] Trying to register/login as another user", ->
 
+		it 'should not allow sign in with secondary email', (done) ->
+			secondaryEmail = "acceptance-test-secondary@example.com"
+			@user1.addEmail secondaryEmail, (err) =>
+				@user1.loginWith secondaryEmail, (err) =>
+					expect(err?).to.equal false
+					@user1.isLoggedIn (err, isLoggedIn) ->
+						expect(isLoggedIn).to.equal false
+						done()
+
 		it 'should have user1 login', (done) ->
 			@user1.login (err) ->
 				expect(err?).to.equal false

services/web/test/acceptance/coffee/SettingsTests.coffee

@@ -0,0 +1,28 @@
+should = require('chai').should()
+async = require("async")
+User = require "./helpers/User"
+
+describe 'SettingsPage', ->
+
+	before (done) ->
+		@user = new User()
+		async.series [
+			@user.ensureUserExists.bind(@user)
+			@user.login.bind(@user)
+			@user.activateSudoMode.bind(@user)
+		], done
+
+	it 'load settigns page', (done) ->
+		@user.getUserSettingsPage (err, statusCode) ->
+			statusCode.should.equal 200
+			done()
+
+	it 'update main email address', (done) ->
+		newEmail = 'foo@bar.com'
+		@user.updateSettings email: newEmail, (error) =>
+			should.not.exist error
+			@user.get (error, user) ->
+				user.email.should.equal newEmail
+				user.emails.length.should.equal 1
+				user.emails[0].email.should.equal newEmail
+				done()

services/web/test/acceptance/coffee/helpers/User.coffee

@@ -3,13 +3,18 @@ _ = require("underscore")
 settings = require("settings-sharelatex")
 {db, ObjectId} = require("../../../../app/js/infrastructure/mongojs")
 UserModel = require("../../../../app/js/models/User").User
+UserUpdater = require("../../../../app/js/Features/User/UserUpdater")
 AuthenticationManager = require("../../../../app/js/Features/Authentication/AuthenticationManager")
 
 count = 0
 
 class User
 	constructor: (options = {}) ->
-		@email = "acceptance-test-#{count}@example.com"
+		@emails = [
+			email: "acceptance-test-#{count}@example.com"
+			createdAt: new Date()
+		]
+		@email = @emails[0].email
 		@password = "acceptance-test-#{count}-password"
 		count++
 		@jar = request.jar()
@@ -17,14 +22,20 @@ class User
 			jar: @jar
 		})
 
+	get: (callback = (error, user)->) ->
+		db.users.findOne { _id: ObjectId(@_id) }, callback
+
 	login: (callback = (error) ->) ->
+		@loginWith(@email, callback)
+
+	loginWith: (email, callback = (error) ->) ->
 		@ensureUserExists (error) =>
 			return callback(error) if error?
 			@getCsrfToken (error) =>
 				return callback(error) if error?
 				@request.post {
 					url: "/login"
-					json: { @email, @password }
+					json: { email, @password }
 				}, callback
 
 	ensureUserExists: (callback = (error) ->) ->
@@ -33,11 +44,14 @@ class User
 		UserModel.findOneAndUpdate filter, {}, options, (error, user) =>
 			return callback(error) if error?
 			AuthenticationManager.setUserPassword user._id, @password, (error) =>
+				return callback(error) if error?
+				UserUpdater.updateUser user._id, $set: emails: @emails, (error) =>
 					return callback(error) if error?
 					@id = user?._id?.toString()
 					@_id = user?._id?.toString()
 					@first_name = user?.first_name
 					@referal_id = user?.referal_id
+
 					callback(null, @password)
 
 	setFeatures: (features, callback = (error) ->) ->
@@ -62,6 +76,10 @@ class User
 					@_id = user?._id?.toString()
 					callback()
 
+	addEmail: (email, callback = (error) ->) ->
+		@emails.push(email: email, createdAt: new Date())
+		UserUpdater.addEmailAddress @id, email, callback
+
 	ensure_admin: (callback = (error) ->) ->
 		db.users.update {_id: ObjectId(@id)}, { $set: { isAdmin: true }}, callback
 
@@ -226,6 +244,23 @@ class User
 				return callback(error) if error?
 				callback(null, response.statusCode)
 
+	activateSudoMode: (callback = (error)->) ->
+		@getCsrfToken (error) =>
+			return callback(error) if error?
+			@request.post {
+				uri: '/confirm-password',
+				json:
+					password: @password
+			}, callback
+
+	updateSettings: (newSettings, callback = (error, response, body) ->) ->
+		@getCsrfToken (error) =>
+			return callback(error) if error?
+			@request.post {
+				url: '/user/settings'
+				json: newSettings
+			}, callback
+
 	getProjectListPage: (callback=(error, statusCode)->) ->
 		@getCsrfToken (error) =>
 			return callback(error) if error?

services/web/test/unit/coffee/User/UserGetterTests.coffee

@@ -56,3 +56,32 @@ describe "UserGetter", ->
 				@findOne.called.should.equal true
 				@findOne.calledWith(email: email).should.equal true
 				done()
+
+	describe "getUserByAnyEmail", ->
+		it "query user for any email", (done)->
+			email = 'hello@world.com'
+			expectedQuery =
+				emails: { $exists: true }
+				'emails.email': email
+			projection = emails: 1
+			@UserGetter.getUserByAnyEmail " #{email} ", projection, (error, user) =>
+				@findOne.calledWith(expectedQuery, projection).should.equal true
+				user.should.deep.equal @fakeUser
+				done()
+
+		it "query contains $exists:true so partial index is used", (done)->
+			expectedQuery =
+				emails: { $exists: true }
+				'emails.email': ''
+			@UserGetter.getUserByAnyEmail '', {}, (error, user) =>
+				@findOne.calledWith(expectedQuery, {}).should.equal true
+				done()
+
+		it "checks main email as well", (done)->
+			@findOne.callsArgWith(2, null, null)
+			email = 'hello@world.com'
+			projection = emails: 1
+			@UserGetter.getUserByAnyEmail " #{email} ", projection, (error, user) =>
+				@findOne.calledTwice.should.equal true
+				@findOne.calledWith(email: email, projection).should.equal true
+				done()

services/web/test/unit/coffee/User/UserUpdaterTests.coffee

@@ -15,35 +15,135 @@ describe "UserUpdater", ->
 			db:{}
 			ObjectId:(id)-> return id
 		@UserGetter =
-			getUserByMainEmail: sinon.stub()
+			getUserEmail: sinon.stub()
+			getUserByAnyEmail: sinon.stub()
+		@logger = err: sinon.stub(), log: ->
 		@UserUpdater = SandboxedModule.require modulePath, requires:
 			"settings-sharelatex":@settings
-			"logger-sharelatex": log:->
+			"logger-sharelatex": @logger
 			"./UserGetter": @UserGetter
 			"../../infrastructure/mongojs":@mongojs
 			"metrics-sharelatex": timeAsyncMethod: sinon.stub()
 
 		@stubbedUser = 
+			_id: "3131231"
 			name:"bob"
 			email:"hello@world.com"
-		@user_id = "3131231"
 		@newEmail = "bob@bob.com"
 
-	describe "changeEmailAddress", ->
+	describe 'changeEmailAddress', ->
 		beforeEach ->
-			@UserUpdater.updateUser = sinon.stub().callsArgWith(2)
+			@UserGetter.getUserEmail.callsArgWith(1, null, @stubbedUser.email)
+			@UserUpdater.addEmailAddress = sinon.stub().callsArgWith(2)
+			@UserUpdater.setDefaultEmailAddress = sinon.stub().callsArgWith(2)
+			@UserUpdater.removeEmailAddress = sinon.stub().callsArgWith(2)
 
-		it "should check if the new email already has an account", (done)->
-			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @stubbedUser)
-			@UserUpdater.changeEmailAddress @user_id, @stubbedUser.email, (err)=>
-				@UserUpdater.updateUser.called.should.equal false
+		it 'change email', (done)->
+			@UserUpdater.changeEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.not.exist(err)
+				@UserUpdater.addEmailAddress.calledWith(
+					@stubbedUser._id, @newEmail
+				).should.equal true
+				@UserUpdater.setDefaultEmailAddress.calledWith(
+					@stubbedUser._id, @newEmail
+				).should.equal true
+				@UserUpdater.removeEmailAddress.calledWith(
+					@stubbedUser._id, @stubbedUser.email
+				).should.equal true
+				done()
+
+		it 'handle error', (done)->
+			@UserUpdater.removeEmailAddress.callsArgWith(2, new Error('nope'))
+			@UserUpdater.changeEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.exist(err)
+				done()
+
+	describe 'addEmailAddress', ->
+		beforeEach ->
+			@UserUpdater._ensureUniqueEmailAddress = sinon.stub().callsArgWith(1)
+
+		it 'add email', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null)
+
+			@UserUpdater.addEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				@UserUpdater._ensureUniqueEmailAddress.called.should.equal true
+				should.not.exist(err)
+				@UserUpdater.updateUser.calledWith(
+					@stubbedUser._id,
+					$push: { emails: { email: @newEmail, createdAt: sinon.match.date } }
+				).should.equal true
+				done()
+
+		it 'handle error', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, new Error('nope'))
+
+			@UserUpdater.addEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				@logger.err.called.should.equal true
+				should.exist(err)
+				done()
+
+	describe 'removeEmailAddress', ->
+		it 'remove email', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 1)
+
+			@UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.not.exist(err)
+				@UserUpdater.updateUser.calledWith(
+					{ _id: @stubbedUser._id, email: { $ne: @newEmail } },
+					$pull: { emails: { email: @newEmail } }
+				).should.equal true
+				done()
+
+		it 'handle error', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, new Error('nope'))
+
+			@UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.exist(err)
+				done()
+
+		it 'handle missed update', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 0)
+
+			@UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.exist(err)
+				done()
+
+	describe 'setDefaultEmailAddress', ->
+		it 'set default', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 1)
+
+			@UserUpdater.setDefaultEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.not.exist(err)
+				@UserUpdater.updateUser.calledWith(
+					{ _id: @stubbedUser._id, 'emails.email': @newEmail },
+					$set: { email: @newEmail }
+				).should.equal true
+				done()
+
+		it 'handle error', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, new Error('nope'))
+
+			@UserUpdater.setDefaultEmailAddress @stubbedUser._id, @newEmail, (err)=>
+				should.exist(err)
+				done()
+
+		it 'handle missed update', (done)->
+			@UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 0)
+
+			@UserUpdater.setDefaultEmailAddress @stubbedUser._id, @newEmail, (err)=>
 				should.exist(err)
 				done()
 
 
-		it "should set the users password", (done)->
-			@UserGetter.getUserByMainEmail.callsArgWith(1, null)
-			@UserUpdater.changeEmailAddress @user_id, @newEmail, (err)=>
-				@UserUpdater.updateUser.calledWith(@user_id, $set: { "email": @newEmail}).should.equal true
+	describe '_ensureUniqueEmailAddress', ->
+		it 'should return error if existing user is found', (done)->
+			@UserGetter.getUserByAnyEmail.callsArgWith(1, null, @stubbedUser)
+			@UserUpdater._ensureUniqueEmailAddress @newEmail, (err)=>
+				should.exist(err)
 				done()
 
+		it 'should return null if no user is found', (done)->
+			@UserGetter.getUserByAnyEmail.callsArgWith(1)
+			@UserUpdater._ensureUniqueEmailAddress @newEmail, (err)=>
+				should.not.exist(err)
+				done()