diff --git a/server/config/passport.js b/server/config/passport.js index 4c436160..163db374 100644 --- a/server/config/passport.js +++ b/server/config/passport.js @@ -24,7 +24,7 @@ passport.deserializeUser((id, done) => { * Sign in using Email/Username and Password. */ passport.use(new LocalStrategy({ usernameField: 'email' }, (email, password, done) => { - User.findByMailOrName(email) + User.findByEmailOrUsername(email) .then((user) => { // eslint-disable-line consistent-return if (!user) { return done(null, false, { msg: `Email ${email} not found.` }); @@ -43,7 +43,7 @@ passport.use(new LocalStrategy({ usernameField: 'email' }, (email, password, don * Authentificate using Basic Auth (Username + Api Key) */ passport.use(new BasicStrategy((userid, key, done) => { - User.findOne({ username: userid }).collation({ locale: 'en', strength: 2 }).exec((err, user) => { // eslint-disable-line consistent-return + User.findByUsername(userid, (err, user) => { // eslint-disable-line consistent-return if (err) { return done(err); } if (!user) { return done(null, false); } user.findMatchingKey(key, (innerErr, isMatch, keyDocument) => { @@ -98,9 +98,7 @@ passport.use(new GitHubStrategy({ const emails = getVerifiedEmails(profile.emails); const primaryEmail = getPrimaryEmail(profile.emails); - User.findOne({ - email: { $in: emails }, - }).collation({ locale: 'en', strength: 2 }).exec((findByEmailErr, existingEmailUser) => { + User.findByEmail(emails, (findByEmailErr, existingEmailUser) => { if (existingEmailUser) { existingEmailUser.email = existingEmailUser.email || primaryEmail; existingEmailUser.github = profile.id; @@ -141,47 +139,44 @@ passport.use(new GoogleStrategy({ const primaryEmail = profile._json.emails[0].value; - User.findOne({ - email: primaryEmail, - }).collation({ locale: 'en', strength: 2 }).exec((findByEmailErr, existingEmailUser) => { + User.findByEmail(primaryEmail, (findByEmailErr, existingEmailUser) => { let username = profile._json.emails[0].value.split('@')[0]; - User.findOne({ username }).collation({ locale: 'en', strength: 2 }) - .exec((findByUsernameErr, existingUsernameUser) => { - if (existingUsernameUser) { - const adj = friendlyWords.predicates[Math.floor(Math.random() * friendlyWords.predicates.length)]; - username = slugify(`${username} ${adj}`); - } - // what if a username is already taken from the display name too? - // then, append a random friendly word? - if (existingEmailUser) { - existingEmailUser.email = existingEmailUser.email || primaryEmail; - existingEmailUser.google = profile._json.emails[0].value; - existingEmailUser.username = existingEmailUser.username || username; - existingEmailUser.tokens.push({ kind: 'google', accessToken }); - existingEmailUser.name = existingEmailUser.name || profile._json.displayName; - existingEmailUser.verified = User.EmailConfirmation.Verified; - existingEmailUser.save((saveErr) => { - if (saveErr) { - console.log(saveErr); - } - done(null, existingEmailUser); - }); - } else { - const user = new User(); - user.email = primaryEmail; - user.google = profile._json.emails[0].value; - user.username = username; - user.tokens.push({ kind: 'google', accessToken }); - user.name = profile._json.displayName; - user.verified = User.EmailConfirmation.Verified; - user.save((saveErr) => { - if (saveErr) { - console.log(saveErr); - } - done(null, user); - }); - } - }); + User.findByUsername(username, (findByUsernameErr, existingUsernameUser) => { + if (existingUsernameUser) { + const adj = friendlyWords.predicates[Math.floor(Math.random() * friendlyWords.predicates.length)]; + username = slugify(`${username} ${adj}`); + } + // what if a username is already taken from the display name too? + // then, append a random friendly word? + if (existingEmailUser) { + existingEmailUser.email = existingEmailUser.email || primaryEmail; + existingEmailUser.google = profile._json.emails[0].value; + existingEmailUser.username = existingEmailUser.username || username; + existingEmailUser.tokens.push({ kind: 'google', accessToken }); + existingEmailUser.name = existingEmailUser.name || profile._json.displayName; + existingEmailUser.verified = User.EmailConfirmation.Verified; + existingEmailUser.save((saveErr) => { + if (saveErr) { + console.log(saveErr); + } + done(null, existingEmailUser); + }); + } else { + const user = new User(); + user.email = primaryEmail; + user.google = profile._json.emails[0].value; + user.username = username; + user.tokens.push({ kind: 'google', accessToken }); + user.name = profile._json.displayName; + user.verified = User.EmailConfirmation.Verified; + user.save((saveErr) => { + if (saveErr) { + console.log(saveErr); + } + done(null, user); + }); + } + }); }); }); })); diff --git a/server/controllers/collection.controller/collectionForUserExists.js b/server/controllers/collection.controller/collectionForUserExists.js index 8315d50c..4dd2d40f 100644 --- a/server/controllers/collection.controller/collectionForUserExists.js +++ b/server/controllers/collection.controller/collectionForUserExists.js @@ -11,7 +11,7 @@ export default function collectionForUserExists(username, collectionId, callback } function findUser() { - return User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec(); + return User.findByUsername(username); } function findCollection(owner) { diff --git a/server/controllers/collection.controller/listCollections.js b/server/controllers/collection.controller/listCollections.js index 1def86aa..b1a60ee9 100644 --- a/server/controllers/collection.controller/listCollections.js +++ b/server/controllers/collection.controller/listCollections.js @@ -4,7 +4,7 @@ import User from '../../models/user'; async function getOwnerUserId(req) { if (req.params.username) { const user = - await User.findOne({ username: req.params.username }).collation({ locale: 'en', strength: 2 }).exec(); + await User.findByUsername(req.params.username); if (user && user._id) { return user._id; } diff --git a/server/controllers/project.controller.js b/server/controllers/project.controller.js index 9cc58fd2..7f118c23 100644 --- a/server/controllers/project.controller.js +++ b/server/controllers/project.controller.js @@ -64,7 +64,7 @@ export function updateProject(req, res) { export function getProject(req, res) { const { project_id: projectId, username } = req.params; - User.findOne({ username }).collation({ locale: "en", strength: 2 }).exec((err, user) => { // eslint-disable-line + User.findByUsername(username, (err, user) => { // eslint-disable-line if (!user) { return res.status(404).send({ message: 'Project with that username does not exist' }); } @@ -141,7 +141,7 @@ export function projectExists(projectId, callback) { } export function projectForUserExists(username, projectId, callback) { - User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec((err, user) => { + User.findByUsername(username, (err, user) => { if (!user) { callback(false); return; diff --git a/server/controllers/project.controller/getProjectsForUser.js b/server/controllers/project.controller/getProjectsForUser.js index 453e2ebe..7b563b2c 100644 --- a/server/controllers/project.controller/getProjectsForUser.js +++ b/server/controllers/project.controller/getProjectsForUser.js @@ -7,7 +7,7 @@ const UserNotFoundError = createApplicationErrorClass('UserNotFoundError'); function getProjectsForUserName(username) { return new Promise((resolve, reject) => { - User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec((err, user) => { + User.findByUsername(username, (err, user) => { if (err) { reject(err); return; diff --git a/server/controllers/user.controller.js b/server/controllers/user.controller.js index 56aa574e..0ff791c6 100644 --- a/server/controllers/user.controller.js +++ b/server/controllers/user.controller.js @@ -30,7 +30,7 @@ const random = (done) => { }; export function findUserByUsername(username, cb) { - User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec((err, user) => { + User.findByUsername(username, (err, user) => { cb(user); }); } @@ -50,12 +50,7 @@ export function createUser(req, res, next) { verifiedTokenExpires: EMAIL_VERIFY_TOKEN_EXPIRY_TIME, }); - User.findOne({ - $or: [ - { email }, - { username } - ] - }).collation({ locale: 'en', strength: 2 }).exec((err, existingUser) => { + User.findByEmailAndUsername(email, username, (err, existingUser) => { if (err) { res.status(404).send({ error: err }); return; @@ -100,6 +95,9 @@ export function duplicateUserCheck(req, res) { const value = req.query[checkType]; const query = {}; query[checkType] = value; + // Don't want to use findByEmailOrUsername here, because in this case we do + // want to use case-insensitive search for usernames to prevent username + // duplicates, which overrides the default behavior. User.findOne(query).collation({ locale: 'en', strength: 2 }).exec((err, user) => { if (user) { return res.json({ @@ -144,19 +142,18 @@ export function resetPasswordInitiate(req, res) { async.waterfall([ random, (token, done) => { - User.findOne({ email: req.body.email.toLowerCase() }) - .collation({ locale: 'en', strength: 2 }).exec((err, user) => { - if (!user) { - res.json({ success: true, message: 'If the email is registered with the editor, an email has been sent.' }); - return; - } - user.resetPasswordToken = token; - user.resetPasswordExpires = Date.now() + 3600000; // 1 hour + User.findByEmail(req.body.email, (err, user) => { + if (!user) { + res.json({ success: true, message: 'If the email is registered with the editor, an email has been sent.' }); + return; + } + user.resetPasswordToken = token; + user.resetPasswordExpires = Date.now() + 3600000; // 1 hour - user.save((saveErr) => { - done(saveErr, token, user); - }); + user.save((saveErr) => { + done(saveErr, token, user); }); + }); }, (token, user, done) => { const protocol = process.env.NODE_ENV === 'production' ? 'https' : 'http'; @@ -275,7 +272,7 @@ export function updatePassword(req, res) { } export function userExists(username, callback) { - User.findOne(username).collation({ locale: 'en', strength: 2 }).exec((err, user) => ( + User.findByUsername(username, (err, user) => ( user ? callback(true) : callback(false) )); } diff --git a/server/models/user.js b/server/models/user.js index 1f91c104..c4097e87 100644 --- a/server/models/user.js +++ b/server/models/user.js @@ -141,20 +141,81 @@ userSchema.methods.findMatchingKey = function findMatchingKey(candidateKey, cb) if (!foundOne) cb('Matching API key not found !', false, null); }; -userSchema.statics.findByMailOrName = function findByMailOrName(email) { - const isEmail = email.indexOf('@') > -1; - if (isEmail) { - const query = { - email: email.toLowerCase() +/** + * + * Queries User collection by email and returns one User document. + * + * @param {string|string[]} email - Email string or array of email strings + * @callback [cb] - Optional error-first callback that passes User document + * @return {Promise} - Returns Promise fulfilled by User document + */ +userSchema.statics.findByEmail = function findByEmail(email, cb) { + let query; + if (Array.isArray(email)) { + query = { + email: { $in: email } + }; + } else { + query = { + email }; - // once emails are all lowercase, won't need to do collation - // but maybe it's not even necessary to make all emails lowercase?? - return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(); } + // Email addresses should be case-insensitive unique + // In MongoDB, you must use collation in order to do a case-insensitive query + return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb); +}; + +/** + * + * Queries User collection by username and returns one User document. + * + * @param {string} username - Username string + * @callback [cb] - Optional error-first callback that passes User document + * @return {Promise} - Returns Promise fulfilled by User document + */ +userSchema.statics.findByUsername = function findByUsername(username, cb) { const query = { - username: email + username }; - return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(); + return this.findOne(query, cb); +}; + +/** + * + * Queries User collection using email or username with optional callback. + * This function will determine automatically whether the data passed is + * a username or email. + * + * @param {string} value - Email or username + * @callback [cb] - Optional error-first callback that passes User document + * @return {Promise} - Returns Promise fulfilled by User document + */ +userSchema.statics.findByEmailOrUsername = function findByEmailOrUsername(value, cb) { + const isEmail = value.indexOf('@') > -1; + if (isEmail) { + return this.findByEmail(value, cb); + } + return this.findByUsername(value, cb); +}; + +/** + * + * Queries User collection, performing a MongoDB logical or with the email + * and username (i.e. if either one matches, will return the first document). + * + * @param {string} email + * @param {string} username + * @callback [cb] - Optional error-first callback that passes User document + * @return {Promise} - Returns Promise fulfilled by User document + */ +userSchema.statics.findByEmailAndUsername = function findByEmailAndUsername(email, username, cb) { + const query = { + $or: [ + { email }, + { username } + ] + }; + return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb); }; userSchema.statics.EmailConfirmation = EmailConfirmationStates;