Handle email duplicates for OAuth signup

Author: catarak

server/config/passport.js

@@ -10,6 +10,11 @@ import { BasicStrategy } from 'passport-http';
 
 import User from '../models/user';
 
+function generateUniqueUsername(username) {
+  const adj = friendlyWords.predicates[Math.floor(Math.random() * friendlyWords.predicates.length)];
+  return slugify(`${username} ${adj}`);
+}
+
 passport.serializeUser((user, done) => {
   done(null, user.id);
 });
@@ -108,14 +113,20 @@ passport.use(new GitHubStrategy({
         existingEmailUser.verified = User.EmailConfirmation.Verified;
         existingEmailUser.save(saveErr => done(null, existingEmailUser));
       } else {
-        const user = new User();
-        user.email = primaryEmail;
-        user.github = profile.id;
-        user.username = profile.username;
-        user.tokens.push({ kind: 'github', accessToken });
-        user.name = profile.displayName;
-        user.verified = User.EmailConfirmation.Verified;
-        user.save(saveErr => done(null, user));
+        let { username } = profile;
+        User.findByUsername(username, true, (findByUsernameErr, existingUsernameUser) => {
+          if (existingUsernameUser) {
+            username = generateUniqueUsername(username);
+          }
+          const user = new User();
+          user.email = primaryEmail;
+          user.github = profile.id;
+          user.username = profile.username;
+          user.tokens.push({ kind: 'github', accessToken });
+          user.name = profile.displayName;
+          user.verified = User.EmailConfirmation.Verified;
+          user.save(saveErr => done(null, user));
+        });
       }
     });
   });
@@ -141,10 +152,9 @@ passport.use(new GoogleStrategy({
 
     User.findByEmail(primaryEmail, (findByEmailErr, existingEmailUser) => {
       let username = profile._json.emails[0].value.split('@')[0];
-      User.findByUsername(username, (findByUsernameErr, existingUsernameUser) => {
+      User.findByUsername(username, true, (findByUsernameErr, existingUsernameUser) => {
         if (existingUsernameUser) {
-          const adj = friendlyWords.predicates[Math.floor(Math.random() * friendlyWords.predicates.length)];
-          username = slugify(`${username} ${adj}`);
+          username = generateUniqueUsername(username);
         }
         // what if a username is already taken from the display name too?
         // then, append a random friendly word?

server/controllers/user.controller.js

@@ -93,12 +93,7 @@ export function createUser(req, res, next) {
 export function duplicateUserCheck(req, res) {
   const checkType = req.query.check_type;
   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) => {
+  User.findByEmailOrUsername(value, true, (err, user) => {
     if (user) {
       return res.json({
         exists: true,

server/models/user.js

@@ -170,14 +170,19 @@ userSchema.statics.findByEmail = function findByEmail(email, cb) {
  * Queries User collection by username and returns one User document.
  *
  * @param {string} username - Username string
+ * @param {boolean} [caseInsensitive]  - Does a caseInsensitive query, defaults to false
  * @callback [cb] - Optional error-first callback that passes User document
  * @return {Promise<Object>} - Returns Promise fulfilled by User document
  */
-userSchema.statics.findByUsername = function findByUsername(username, cb) {
+userSchema.statics.findByUsername = function findByUsername(username, caseInsensitive, cb) {
   const query = {
     username
   };
-  return this.findOne(query, cb);
+  if (arguments.length === 3
+    || (arguments.length === 2 && typeof caseInsensitive === 'boolean' && caseInsensitive)) {
+    return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb);
+  }
+  return this.findOne(query, cb || caseInsensitive);
 };
 
 /**
@@ -187,15 +192,23 @@ userSchema.statics.findByUsername = function findByUsername(username, cb) {
  * a username or email.
  *
  * @param {string} value - Email or username
+ * @param {boolean} caseInsensitive - Does a caseInsensitive query rather than
+ *                                    default query for username or email, defaults
+ *                                    to false
  * @callback [cb] - Optional error-first callback that passes User document
  * @return {Promise<Object>} - Returns Promise fulfilled by User document
  */
-userSchema.statics.findByEmailOrUsername = function findByEmailOrUsername(value, cb) {
+userSchema.statics.findByEmailOrUsername = function findByEmailOrUsername(value, caseInsensitive, cb) {
   const isEmail = value.indexOf('@') > -1;
+  if (arguments.length === 3
+    || (arguments.length === 2 && typeof caseInsensitive === 'boolean' && caseInsensitive)) {
+    const query = isEmail ? { email: value } : { username: value };
+    return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb);
+  }
   if (isEmail) {
-    return this.findByEmail(value, cb);
+    return this.findByEmail(value, cb || caseInsensitive);
   }
-  return this.findByUsername(value, cb);
+  return this.findByUsername(value, cb || caseInsensitive);
 };
 
 /**