Skip to content

Commit 6259f58

Browse files
committed
[#1314][#1489] Add static methods to user model
- Add new static methods to user model - `findByEmailAndUsername` - renames `findByMailOrName` to `findByEmailOrUsername` - `findByUsername` - `findByEmail` - Reverts case insensitive behavior for username
1 parent 15ad07d commit 6259f58

File tree

7 files changed

+132
-79
lines changed

7 files changed

+132
-79
lines changed

server/config/passport.js

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ passport.deserializeUser((id, done) => {
2424
* Sign in using Email/Username and Password.
2525
*/
2626
passport.use(new LocalStrategy({ usernameField: 'email' }, (email, password, done) => {
27-
User.findByMailOrName(email)
27+
User.findByEmailOrUsername(email)
2828
.then((user) => { // eslint-disable-line consistent-return
2929
if (!user) {
3030
return done(null, false, { msg: `Email ${email} not found.` });
@@ -43,7 +43,7 @@ passport.use(new LocalStrategy({ usernameField: 'email' }, (email, password, don
4343
* Authentificate using Basic Auth (Username + Api Key)
4444
*/
4545
passport.use(new BasicStrategy((userid, key, done) => {
46-
User.findOne({ username: userid }).collation({ locale: 'en', strength: 2 }).exec((err, user) => { // eslint-disable-line consistent-return
46+
User.findByUsername(userid, (err, user) => { // eslint-disable-line consistent-return
4747
if (err) { return done(err); }
4848
if (!user) { return done(null, false); }
4949
user.findMatchingKey(key, (innerErr, isMatch, keyDocument) => {
@@ -98,9 +98,7 @@ passport.use(new GitHubStrategy({
9898
const emails = getVerifiedEmails(profile.emails);
9999
const primaryEmail = getPrimaryEmail(profile.emails);
100100

101-
User.findOne({
102-
email: { $in: emails },
103-
}).collation({ locale: 'en', strength: 2 }).exec((findByEmailErr, existingEmailUser) => {
101+
User.findByEmail(emails, (findByEmailErr, existingEmailUser) => {
104102
if (existingEmailUser) {
105103
existingEmailUser.email = existingEmailUser.email || primaryEmail;
106104
existingEmailUser.github = profile.id;
@@ -141,47 +139,44 @@ passport.use(new GoogleStrategy({
141139

142140
const primaryEmail = profile._json.emails[0].value;
143141

144-
User.findOne({
145-
email: primaryEmail,
146-
}).collation({ locale: 'en', strength: 2 }).exec((findByEmailErr, existingEmailUser) => {
142+
User.findByEmail(primaryEmail, (findByEmailErr, existingEmailUser) => {
147143
let username = profile._json.emails[0].value.split('@')[0];
148-
User.findOne({ username }).collation({ locale: 'en', strength: 2 })
149-
.exec((findByUsernameErr, existingUsernameUser) => {
150-
if (existingUsernameUser) {
151-
const adj = friendlyWords.predicates[Math.floor(Math.random() * friendlyWords.predicates.length)];
152-
username = slugify(`${username} ${adj}`);
153-
}
154-
// what if a username is already taken from the display name too?
155-
// then, append a random friendly word?
156-
if (existingEmailUser) {
157-
existingEmailUser.email = existingEmailUser.email || primaryEmail;
158-
existingEmailUser.google = profile._json.emails[0].value;
159-
existingEmailUser.username = existingEmailUser.username || username;
160-
existingEmailUser.tokens.push({ kind: 'google', accessToken });
161-
existingEmailUser.name = existingEmailUser.name || profile._json.displayName;
162-
existingEmailUser.verified = User.EmailConfirmation.Verified;
163-
existingEmailUser.save((saveErr) => {
164-
if (saveErr) {
165-
console.log(saveErr);
166-
}
167-
done(null, existingEmailUser);
168-
});
169-
} else {
170-
const user = new User();
171-
user.email = primaryEmail;
172-
user.google = profile._json.emails[0].value;
173-
user.username = username;
174-
user.tokens.push({ kind: 'google', accessToken });
175-
user.name = profile._json.displayName;
176-
user.verified = User.EmailConfirmation.Verified;
177-
user.save((saveErr) => {
178-
if (saveErr) {
179-
console.log(saveErr);
180-
}
181-
done(null, user);
182-
});
183-
}
184-
});
144+
User.findByUsername(username, (findByUsernameErr, existingUsernameUser) => {
145+
if (existingUsernameUser) {
146+
const adj = friendlyWords.predicates[Math.floor(Math.random() * friendlyWords.predicates.length)];
147+
username = slugify(`${username} ${adj}`);
148+
}
149+
// what if a username is already taken from the display name too?
150+
// then, append a random friendly word?
151+
if (existingEmailUser) {
152+
existingEmailUser.email = existingEmailUser.email || primaryEmail;
153+
existingEmailUser.google = profile._json.emails[0].value;
154+
existingEmailUser.username = existingEmailUser.username || username;
155+
existingEmailUser.tokens.push({ kind: 'google', accessToken });
156+
existingEmailUser.name = existingEmailUser.name || profile._json.displayName;
157+
existingEmailUser.verified = User.EmailConfirmation.Verified;
158+
existingEmailUser.save((saveErr) => {
159+
if (saveErr) {
160+
console.log(saveErr);
161+
}
162+
done(null, existingEmailUser);
163+
});
164+
} else {
165+
const user = new User();
166+
user.email = primaryEmail;
167+
user.google = profile._json.emails[0].value;
168+
user.username = username;
169+
user.tokens.push({ kind: 'google', accessToken });
170+
user.name = profile._json.displayName;
171+
user.verified = User.EmailConfirmation.Verified;
172+
user.save((saveErr) => {
173+
if (saveErr) {
174+
console.log(saveErr);
175+
}
176+
done(null, user);
177+
});
178+
}
179+
});
185180
});
186181
});
187182
}));

server/controllers/collection.controller/collectionForUserExists.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export default function collectionForUserExists(username, collectionId, callback
1111
}
1212

1313
function findUser() {
14-
return User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec();
14+
return User.findByUsername(username);
1515
}
1616

1717
function findCollection(owner) {

server/controllers/collection.controller/listCollections.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import User from '../../models/user';
44
async function getOwnerUserId(req) {
55
if (req.params.username) {
66
const user =
7-
await User.findOne({ username: req.params.username }).collation({ locale: 'en', strength: 2 }).exec();
7+
await User.findByUsername(req.params.username);
88
if (user && user._id) {
99
return user._id;
1010
}

server/controllers/project.controller.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export function updateProject(req, res) {
6464

6565
export function getProject(req, res) {
6666
const { project_id: projectId, username } = req.params;
67-
User.findOne({ username }).collation({ locale: "en", strength: 2 }).exec((err, user) => { // eslint-disable-line
67+
User.findByUsername(username, (err, user) => { // eslint-disable-line
6868
if (!user) {
6969
return res.status(404).send({ message: 'Project with that username does not exist' });
7070
}
@@ -141,7 +141,7 @@ export function projectExists(projectId, callback) {
141141
}
142142

143143
export function projectForUserExists(username, projectId, callback) {
144-
User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec((err, user) => {
144+
User.findByUsername(username, (err, user) => {
145145
if (!user) {
146146
callback(false);
147147
return;

server/controllers/project.controller/getProjectsForUser.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const UserNotFoundError = createApplicationErrorClass('UserNotFoundError');
77

88
function getProjectsForUserName(username) {
99
return new Promise((resolve, reject) => {
10-
User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec((err, user) => {
10+
User.findByUsername(username, (err, user) => {
1111
if (err) {
1212
reject(err);
1313
return;

server/controllers/user.controller.js

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const random = (done) => {
3030
};
3131

3232
export function findUserByUsername(username, cb) {
33-
User.findOne({ username }).collation({ locale: 'en', strength: 2 }).exec((err, user) => {
33+
User.findByUsername(username, (err, user) => {
3434
cb(user);
3535
});
3636
}
@@ -50,12 +50,7 @@ export function createUser(req, res, next) {
5050
verifiedTokenExpires: EMAIL_VERIFY_TOKEN_EXPIRY_TIME,
5151
});
5252

53-
User.findOne({
54-
$or: [
55-
{ email },
56-
{ username }
57-
]
58-
}).collation({ locale: 'en', strength: 2 }).exec((err, existingUser) => {
53+
User.findByEmailAndUsername(email, username, (err, existingUser) => {
5954
if (err) {
6055
res.status(404).send({ error: err });
6156
return;
@@ -100,6 +95,9 @@ export function duplicateUserCheck(req, res) {
10095
const value = req.query[checkType];
10196
const query = {};
10297
query[checkType] = value;
98+
// Don't want to use findByEmailOrUsername here, because in this case we do
99+
// want to use case-insensitive search for usernames to prevent username
100+
// duplicates, which overrides the default behavior.
103101
User.findOne(query).collation({ locale: 'en', strength: 2 }).exec((err, user) => {
104102
if (user) {
105103
return res.json({
@@ -144,19 +142,18 @@ export function resetPasswordInitiate(req, res) {
144142
async.waterfall([
145143
random,
146144
(token, done) => {
147-
User.findOne({ email: req.body.email.toLowerCase() })
148-
.collation({ locale: 'en', strength: 2 }).exec((err, user) => {
149-
if (!user) {
150-
res.json({ success: true, message: 'If the email is registered with the editor, an email has been sent.' });
151-
return;
152-
}
153-
user.resetPasswordToken = token;
154-
user.resetPasswordExpires = Date.now() + 3600000; // 1 hour
145+
User.findByEmail(req.body.email, (err, user) => {
146+
if (!user) {
147+
res.json({ success: true, message: 'If the email is registered with the editor, an email has been sent.' });
148+
return;
149+
}
150+
user.resetPasswordToken = token;
151+
user.resetPasswordExpires = Date.now() + 3600000; // 1 hour
155152

156-
user.save((saveErr) => {
157-
done(saveErr, token, user);
158-
});
153+
user.save((saveErr) => {
154+
done(saveErr, token, user);
159155
});
156+
});
160157
},
161158
(token, user, done) => {
162159
const protocol = process.env.NODE_ENV === 'production' ? 'https' : 'http';
@@ -275,7 +272,7 @@ export function updatePassword(req, res) {
275272
}
276273

277274
export function userExists(username, callback) {
278-
User.findOne(username).collation({ locale: 'en', strength: 2 }).exec((err, user) => (
275+
User.findByUsername(username, (err, user) => (
279276
user ? callback(true) : callback(false)
280277
));
281278
}

server/models/user.js

Lines changed: 71 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -141,20 +141,81 @@ userSchema.methods.findMatchingKey = function findMatchingKey(candidateKey, cb)
141141
if (!foundOne) cb('Matching API key not found !', false, null);
142142
};
143143

144-
userSchema.statics.findByMailOrName = function findByMailOrName(email) {
145-
const isEmail = email.indexOf('@') > -1;
146-
if (isEmail) {
147-
const query = {
148-
email: email.toLowerCase()
144+
/**
145+
*
146+
* Queries User collection by email and returns one User document.
147+
*
148+
* @param {string|string[]} email - Email string or array of email strings
149+
* @callback [cb] - Optional error-first callback that passes User document
150+
* @return {Promise<Object>} - Returns Promise fulfilled by User document
151+
*/
152+
userSchema.statics.findByEmail = function findByEmail(email, cb) {
153+
let query;
154+
if (Array.isArray(email)) {
155+
query = {
156+
email: { $in: email }
149157
};
150-
// once emails are all lowercase, won't need to do collation
151-
// but maybe it's not even necessary to make all emails lowercase??
152-
return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec();
158+
} else {
159+
query = {
160+
email
161+
};
162+
}
163+
// Email addresses should be case-insensitive unique
164+
// In MongoDB, you must use collation in order to do a case-insensitive query
165+
return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb);
166+
};
167+
168+
/**
169+
*
170+
* Queries User collection by username and returns one User document.
171+
*
172+
* @param {string} username - Username string
173+
* @callback [cb] - Optional error-first callback that passes User document
174+
* @return {Promise<Object>} - Returns Promise fulfilled by User document
175+
*/
176+
userSchema.statics.findByUsername = function findByUsername(username, cb) {
177+
const query = {
178+
username
179+
};
180+
return this.findOne(query, cb);
181+
};
182+
183+
/**
184+
*
185+
* Queries User collection using email or username with optional callback.
186+
* This function will determine automatically whether the data passed is
187+
* a username or email.
188+
*
189+
* @param {string} value - Email or username
190+
* @callback [cb] - Optional error-first callback that passes User document
191+
* @return {Promise<Object>} - Returns Promise fulfilled by User document
192+
*/
193+
userSchema.statics.findByEmailOrUsername = function findByEmailOrUsername(value, cb) {
194+
const isEmail = value.indexOf('@') > -1;
195+
if (isEmail) {
196+
return this.findByEmail(value, cb);
153197
}
198+
return this.findByUsername(value, cb);
199+
};
200+
201+
/**
202+
*
203+
* Queries User collection, performing a MongoDB logical or with the email
204+
* and username (i.e. if either one matches, will return the first document).
205+
*
206+
* @param {string} email
207+
* @param {string} username
208+
* @callback [cb] - Optional error-first callback that passes User document
209+
* @return {Promise<Object>} - Returns Promise fulfilled by User document
210+
*/
211+
userSchema.statics.findByEmailAndUsername = function findByEmailAndUsername(email, username, cb) {
154212
const query = {
155-
username: email
213+
$or: [
214+
{ email },
215+
{ username }
216+
]
156217
};
157-
return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec();
218+
return this.findOne(query).collation({ locale: 'en', strength: 2 }).exec(cb);
158219
};
159220

160221
userSchema.statics.EmailConfirmation = EmailConfirmationStates;

0 commit comments

Comments
 (0)