add UserGetter#getUserByMainEmail

Use only that method to find users by email.
This commit is contained in:
Tim Alby
2018-05-23 16:12:23 +02:00
parent 5a590aa021
commit bbaca91e57
17 changed files with 134 additions and 70 deletions

View File

@@ -27,7 +27,7 @@ module.exports = CollaboratorsInviteController =
_checkShouldInviteEmail: (email, callback=(err, shouldAllowInvite)->) ->
if Settings.restrictInvitesToExistingAccounts == true
logger.log {email}, "checking if user exists with this email"
UserGetter.getUser {email: email}, {_id: 1}, (err, user) ->
UserGetter.getUserByMainEmail email, {_id: 1}, (err, user) ->
return callback(err) if err?
userExists = user? and user?._id?
callback(null, userExists)

View File

@@ -32,7 +32,7 @@ module.exports = CollaboratorsInviteHandler =
_trySendInviteNotification: (projectId, sendingUser, invite, callback=(err)->) ->
email = invite.email
UserGetter.getUser {email: email}, {_id: 1}, (err, existingUser) ->
UserGetter.getUserByMainEmail email, {_id: 1}, (err, existingUser) ->
if err?
logger.err {projectId, email}, "error checking if user exists"
return callback(err)

View File

@@ -9,7 +9,7 @@ logger = require("logger-sharelatex")
module.exports =
generateAndEmailResetToken:(email, callback = (error, exists) ->)->
UserGetter.getUser email:email, (err, user)->
UserGetter.getUserByMainEmail email, (err, user)->
if err then return callback(err)
if !user? or user.holdingAccount
logger.err email:email, "user could not be found for password reset"

View File

@@ -3,6 +3,7 @@ _ = require("underscore")
SubscriptionUpdater = require("./SubscriptionUpdater")
SubscriptionLocator = require("./SubscriptionLocator")
UserLocator = require("../User/UserLocator")
UserGetter = require("../User/UserGetter")
LimitationsManager = require("./LimitationsManager")
logger = require("logger-sharelatex")
OneTimeTokenHandler = require("../Security/OneTimeTokenHandler")
@@ -21,7 +22,7 @@ module.exports = SubscriptionGroupHandler =
if limitReached
logger.err adminUserId:adminUserId, newEmail:newEmail, "group subscription limit reached not adding user to group"
return callback(limitReached:limitReached)
UserLocator.findByEmail newEmail, (err, user)->
UserGetter.getUserByMainEmail newEmail, (err, user)->
return callback(err) if err?
if user?
SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)->

View File

@@ -6,6 +6,8 @@ ObjectId = mongojs.ObjectId
module.exports = UserGetter =
getUser: (query, projection, callback = (error, user) ->) ->
if query?.email?
return callback(new Error("Don't use getUser to find user by email"), null)
if arguments.length == 2
callback = projection
projection = {}
@@ -19,6 +21,13 @@ module.exports = UserGetter =
db.users.findOne query, projection, callback
getUserByMainEmail: (email, projection, callback = (error, user) ->) ->
email = email.trim()
if arguments.length == 2
callback = projection
projection = {}
db.users.findOne email: email, projection, callback
getUsers: (user_ids, projection, callback = (error, users) ->) ->
try
user_ids = user_ids.map (u) -> ObjectId(u.toString())
@@ -39,6 +48,7 @@ module.exports = UserGetter =
[
'getUser',
'getUserByMainEmail',
'getUsers',
'getUserOrUserStubById'
].map (method) ->

View File

@@ -6,16 +6,10 @@ logger = require('logger-sharelatex')
module.exports = UserLocator =
findByEmail: (email, callback)->
email = email.trim()
db.users.findOne email:email, (err, user)->
callback(err, user)
findById: (_id, callback)->
db.users.findOne _id:ObjectId(_id+""), callback
[
'findById',
'findByEmail'
].map (method) ->
metrics.timeAsyncMethod UserLocator, method, 'mongo.UserLocator', logger

View File

@@ -1,6 +1,7 @@
sanitize = require('sanitizer')
User = require("../../models/User").User
UserCreator = require("./UserCreator")
UserGetter = require("./UserGetter")
AuthenticationManager = require("../Authentication/AuthenticationManager")
NewsLetterManager = require("../Newsletter/NewsletterManager")
async = require("async")
@@ -47,7 +48,7 @@ module.exports = UserRegistrationHandler =
if !requestIsValid
return callback(new Error("request is not valid"))
userDetails.email = userDetails.email?.trim()?.toLowerCase()
User.findOne email:userDetails.email, (err, user)->
UserGetter.getUserByMainEmail userDetails.email, (err, user) =>
if err?
return callback err
if user?.holdingAccount == false

View File

@@ -3,7 +3,7 @@ mongojs = require("../../infrastructure/mongojs")
metrics = require("metrics-sharelatex")
db = mongojs.db
ObjectId = mongojs.ObjectId
UserLocator = require("./UserLocator")
UserGetter = require("./UserGetter")
module.exports = UserUpdater =
updateUser: (query, update, callback = (error) ->) ->
@@ -18,7 +18,7 @@ module.exports = UserUpdater =
changeEmailAddress: (user_id, newEmail, callback)->
self = @
logger.log user_id:user_id, newEmail:newEmail, "updaing email address of user"
UserLocator.findByEmail newEmail, (error, user) ->
UserGetter.getUserByMainEmail newEmail, (error, user) ->
if user?
return callback({message:"alread_exists"})
self.updateUser user_id.toString(), {

View File

@@ -24,11 +24,14 @@ describe "CollaboratorsInviteController", ->
addCount: sinon.stub
@LimitationsManager = {}
@UserGetter =
getUserByMainEmail: sinon.stub()
getUser: sinon.stub()
@CollaboratorsInviteController = SandboxedModule.require modulePath, requires:
"../Project/ProjectGetter": @ProjectGetter = {}
'../Subscription/LimitationsManager' : @LimitationsManager
'../User/UserGetter': @UserGetter = {getUser: sinon.stub()}
'../User/UserGetter': @UserGetter
"./CollaboratorsHandler": @CollaboratorsHandler = {}
"./CollaboratorsInviteHandler": @CollaboratorsInviteHandler = {}
'logger-sharelatex': @logger = {err: sinon.stub(), error: sinon.stub(), log: sinon.stub()}
@@ -713,7 +716,7 @@ describe "CollaboratorsInviteController", ->
beforeEach ->
@user = {_id: ObjectId().toString()}
@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user)
@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @user)
it 'should callback with `true`', (done) ->
@call (err, shouldAllow) =>
@@ -725,7 +728,7 @@ describe "CollaboratorsInviteController", ->
beforeEach ->
@user = null
@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user)
@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @user)
it 'should callback with `false`', (done) ->
@call (err, shouldAllow) =>
@@ -735,15 +738,15 @@ describe "CollaboratorsInviteController", ->
it 'should have called getUser', (done) ->
@call (err, shouldAllow) =>
@UserGetter.getUser.callCount.should.equal 1
@UserGetter.getUser.calledWith({email: @email}, {_id: 1}).should.equal true
@UserGetter.getUserByMainEmail.callCount.should.equal 1
@UserGetter.getUserByMainEmail.calledWith(@email, {_id: 1}).should.equal true
done()
describe 'when getUser produces an error', ->
beforeEach ->
@user = null
@UserGetter.getUser = sinon.stub().callsArgWith(2, new Error('woops'))
@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, new Error('woops'))
it 'should callback with an error', (done) ->
@call (err, shouldAllow) =>

View File

@@ -605,7 +605,7 @@ describe "CollaboratorsInviteHandler", ->
_id: ObjectId()
first_name: "jim"
@existingUser = {_id: ObjectId()}
@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @existingUser)
@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @existingUser)
@fakeProject =
_id: @project_id
name: "some project"
@@ -626,8 +626,8 @@ describe "CollaboratorsInviteHandler", ->
it 'should call getUser', (done) ->
@call (err) =>
@UserGetter.getUser.callCount.should.equal 1
@UserGetter.getUser.calledWith({email: @invite.email}).should.equal true
@UserGetter.getUserByMainEmail.callCount.should.equal 1
@UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true
done()
it 'should call getProject', (done) ->
@@ -671,7 +671,7 @@ describe "CollaboratorsInviteHandler", ->
describe 'when the user does not exist', ->
beforeEach ->
@UserGetter.getUser = sinon.stub().callsArgWith(2, null, null)
@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, null)
it 'should not produce an error', (done) ->
@call (err) =>
@@ -680,8 +680,8 @@ describe "CollaboratorsInviteHandler", ->
it 'should call getUser', (done) ->
@call (err) =>
@UserGetter.getUser.callCount.should.equal 1
@UserGetter.getUser.calledWith({email: @invite.email}).should.equal true
@UserGetter.getUserByMainEmail.callCount.should.equal 1
@UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true
done()
it 'should not call getProject', (done) ->
@@ -698,7 +698,7 @@ describe "CollaboratorsInviteHandler", ->
describe 'when the getUser produces an error', ->
beforeEach ->
@UserGetter.getUser = sinon.stub().callsArgWith(2, new Error('woops'))
@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, new Error('woops'))
it 'should produce an error', (done) ->
@call (err) =>
@@ -707,8 +707,8 @@ describe "CollaboratorsInviteHandler", ->
it 'should call getUser', (done) ->
@call (err) =>
@UserGetter.getUser.callCount.should.equal 1
@UserGetter.getUser.calledWith({email: @invite.email}).should.equal true
@UserGetter.getUserByMainEmail.callCount.should.equal 1
@UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true
done()
it 'should not call getProject', (done) ->

View File

@@ -16,7 +16,7 @@ describe "PasswordResetHandler", ->
getNewToken:sinon.stub()
getValueFromTokenAndExpire:sinon.stub()
@UserGetter =
getUser:sinon.stub()
getUserByMainEmail:sinon.stub()
@EmailHandler =
sendEmail:sinon.stub()
@AuthenticationManager =
@@ -40,7 +40,7 @@ describe "PasswordResetHandler", ->
describe "generateAndEmailResetToken", ->
it "should check the user exists", (done)->
@UserGetter.getUser.callsArgWith(1)
@UserGetter.getUserByMainEmail.callsArgWith(1)
@OneTimeTokenHandler.getNewToken.callsArgWith(1)
@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
exists.should.equal false
@@ -49,7 +49,7 @@ describe "PasswordResetHandler", ->
it "should send the email with the token", (done)->
@UserGetter.getUser.callsArgWith(1, null, @user)
@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
@OneTimeTokenHandler.getNewToken.callsArgWith(1, null, @token)
@EmailHandler.sendEmail.callsArgWith(2)
@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
@@ -62,7 +62,7 @@ describe "PasswordResetHandler", ->
it "should return exists = false for a holdingAccount", (done) ->
@user.holdingAccount = true
@UserGetter.getUser.callsArgWith(1, null, @user)
@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
@OneTimeTokenHandler.getNewToken.callsArgWith(1)
@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
exists.should.equal false

View File

@@ -32,7 +32,9 @@ describe "SubscriptionGroupHandler", ->
@UserLocator =
findById: sinon.stub()
findByEmail: sinon.stub()
@UserGetter =
getUserByMainEmail: sinon.stub()
@LimitationsManager =
hasGroupMembersLimitReached: sinon.stub()
@@ -57,6 +59,7 @@ describe "SubscriptionGroupHandler", ->
"./SubscriptionUpdater": @SubscriptionUpdater
"./SubscriptionLocator": @SubscriptionLocator
"../User/UserLocator": @UserLocator
"../User/UserGetter": @UserGetter
"./LimitationsManager": @LimitationsManager
"../Security/OneTimeTokenHandler":@OneTimeTokenHandler
"../Email/EmailHandler":@EmailHandler
@@ -71,11 +74,11 @@ describe "SubscriptionGroupHandler", ->
describe "addUserToGroup", ->
beforeEach ->
@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
@UserLocator.findByEmail.callsArgWith(1, null, @user)
@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
it "should find the user", (done)->
@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
@UserLocator.findByEmail.calledWith(@newEmail).should.equal true
@UserGetter.getUserByMainEmail.calledWith(@newEmail).should.equal true
done()
it "should add the user to the group", (done)->
@@ -102,7 +105,7 @@ describe "SubscriptionGroupHandler", ->
done()
it "should add an email invite if no user is found", (done) ->
@UserLocator.findByEmail.callsArgWith(1, null, null)
@UserGetter.getUserByMainEmail.callsArgWith(1, null, null)
@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
@SubscriptionUpdater.addEmailInviteToGroup.calledWith(@adminUser_id, @newEmail).should.equal true
done()

View File

@@ -15,11 +15,11 @@ describe "UserCreator", ->
constructor: ->
return self.user
@UserLocator =
findByEmail: sinon.stub()
@UserGetter =
getUserByMainEmail: sinon.stub()
@UserCreator = SandboxedModule.require modulePath, requires:
"../../models/User": User:@UserModel
"./UserLocator":@UserLocator
"./UserGetter":@UserGetter
"logger-sharelatex":{log:->}
'metrics-sharelatex': {timeAsyncMethod: ()->}

View File

@@ -0,0 +1,58 @@
should = require('chai').should()
SandboxedModule = require('sandboxed-module')
assert = require('assert')
path = require('path')
sinon = require('sinon')
modulePath = path.join __dirname, "../../../../app/js/Features/User/UserGetter"
expect = require("chai").expect
describe "UserGetter", ->
beforeEach ->
@fakeUser = {_id:"12390i"}
@findOne = sinon.stub().callsArgWith(2, null, @fakeUser)
@Mongo =
db: users: findOne: @findOne
ObjectId: (id) -> return id
@UserGetter = SandboxedModule.require modulePath, requires:
"logger-sharelatex": log:->
"../../infrastructure/mongojs": @Mongo
"metrics-sharelatex": timeAsyncMethod: sinon.stub()
describe "getUser", ->
it "should get user", (done)->
query = _id: 'foo'
projection = email: 1
@UserGetter.getUser query, projection, (error, user) =>
@findOne.called.should.equal true
@findOne.calledWith(query, projection).should.equal true
user.should.deep.equal @fakeUser
done()
it "should not allow email in query", (done)->
@UserGetter.getUser email: 'foo@bar.com', {}, (error, user) =>
error.should.exist
done()
describe "getUserbyMainEmail", ->
it "query user by main email", (done)->
email = 'hello@world.com'
projection = emails: 1
@UserGetter.getUserByMainEmail email, projection, (error, user) =>
@findOne.called.should.equal true
@findOne.calledWith(email: email, projection).should.equal true
done()
it "return user if found", (done)->
email = 'hello@world.com'
@UserGetter.getUserByMainEmail email, (error, user) =>
user.should.deep.equal @fakeUser
done()
it "trim email", (done)->
email = 'hello@world.com'
@UserGetter.getUserByMainEmail " #{email} ", (error, user) =>
@findOne.called.should.equal true
@findOne.calledWith(email: email).should.equal true
done()

View File

@@ -7,33 +7,25 @@ SandboxedModule = require('sandboxed-module')
describe "UserLocator", ->
beforeEach ->
@user = {_id:"12390i"}
@fakeUser = {_id:"12390i"}
@findOne = sinon.stub().callsArgWith(1, null, @fakeUser)
@Mongo =
db: users: findOne: @findOne
ObjectId: (id) -> return id
@UserLocator = SandboxedModule.require modulePath, requires:
"../../infrastructure/mongojs": db: @db = { users: {} }
"../../infrastructure/mongojs": @Mongo
"metrics-sharelatex": timeAsyncMethod: sinon.stub()
'logger-sharelatex' : { log: sinon.stub() }
@db.users =
findOne : sinon.stub().callsArgWith(1, null, @user)
@email = "bob.oswald@gmail.com"
describe "findByEmail", ->
it "should try and find a user with that email address", (done)->
@UserLocator.findByEmail @email, (err, user)=>
@db.users.findOne.calledWith(email:@email).should.equal true
done()
it "should trim white space", (done)->
@UserLocator.findByEmail "#{@email} ", (err, user)=>
@db.users.findOne.calledWith(email:@email).should.equal true
describe "findById", ->
it "should try and find a user with that id", (done)->
_id = '123e'
@UserLocator.findById _id, (err, user)=>
@findOne.calledWith(_id: _id).should.equal true
done()
it "should return the user if found", (done)->
@UserLocator.findByEmail @email, (err, user)=>
user.should.deep.equal @user
@UserLocator.findById '123e', (err, user)=>
user.should.deep.equal @fakeUser
done()

View File

@@ -12,8 +12,9 @@ describe "UserRegistrationHandler", ->
@user =
_id: @user_id = "31j2lk21kjl"
@User =
findOne:sinon.stub()
update: sinon.stub().callsArgWith(2)
@UserGetter =
getUserByMainEmail: sinon.stub()
@UserCreator =
createNewUser:sinon.stub().callsArgWith(1, null, @user)
@AuthenticationManager =
@@ -26,6 +27,7 @@ describe "UserRegistrationHandler", ->
getNewToken: sinon.stub()
@handler = SandboxedModule.require modulePath, requires:
"../../models/User": {User:@User}
"./UserGetter": @UserGetter
"./UserCreator": @UserCreator
"../Authentication/AuthenticationManager":@AuthenticationManager
"../Newsletter/NewsletterManager":@NewsLetterManager
@@ -70,7 +72,7 @@ describe "UserRegistrationHandler", ->
beforeEach ->
@user.holdingAccount = true
@handler._registrationRequestIsValid = sinon.stub().returns true
@User.findOne.callsArgWith(1, null, @user)
@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
it "should not create a new user if there is a holding account there", (done)->
@handler.registerNewUser @passingRequest, (err)=>
@@ -94,7 +96,7 @@ describe "UserRegistrationHandler", ->
done()
it "should return email registered in the error if there is a non holdingAccount there", (done)->
@User.findOne.callsArgWith(1, null, @user = {holdingAccount:false})
@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user = {holdingAccount:false})
@handler.registerNewUser @passingRequest, (err, user)=>
err.should.deep.equal new Error("EmailAlreadyRegistered")
user.should.deep.equal @user
@@ -103,7 +105,7 @@ describe "UserRegistrationHandler", ->
describe "validRequest", ->
beforeEach ->
@handler._registrationRequestIsValid = sinon.stub().returns true
@User.findOne.callsArgWith 1
@UserGetter.getUserByMainEmail.callsArgWith 1
it "should create a new user", (done)->
@handler.registerNewUser @passingRequest, (err)=>

View File

@@ -14,12 +14,12 @@ describe "UserUpdater", ->
@mongojs =
db:{}
ObjectId:(id)-> return id
@UserLocator =
findByEmail:sinon.stub()
@UserGetter =
getUserByMainEmail: sinon.stub()
@UserUpdater = SandboxedModule.require modulePath, requires:
"settings-sharelatex":@settings
"logger-sharelatex": log:->
"./UserLocator":@UserLocator
"./UserGetter": @UserGetter
"../../infrastructure/mongojs":@mongojs
"metrics-sharelatex": timeAsyncMethod: sinon.stub()
@@ -34,7 +34,7 @@ describe "UserUpdater", ->
@UserUpdater.updateUser = sinon.stub().callsArgWith(2)
it "should check if the new email already has an account", (done)->
@UserLocator.findByEmail.callsArgWith(1, null, @stubbedUser)
@UserGetter.getUserByMainEmail.callsArgWith(1, null, @stubbedUser)
@UserUpdater.changeEmailAddress @user_id, @stubbedUser.email, (err)=>
@UserUpdater.updateUser.called.should.equal false
should.exist(err)
@@ -42,7 +42,7 @@ describe "UserUpdater", ->
it "should set the users password", (done)->
@UserLocator.findByEmail.callsArgWith(1, null)
@UserGetter.getUserByMainEmail.callsArgWith(1, null)
@UserUpdater.changeEmailAddress @user_id, @newEmail, (err)=>
@UserUpdater.updateUser.calledWith(@user_id, $set: { "email": @newEmail}).should.equal true
done()