[IMPROVE] Don't use regex to find users (#14397)

* Don't use regex to find users * Invert logic on model methods * Escape username regex * Find users in batch * Use only normalizeMessagesForUser * Don't ignore username case to get owners on graphql
RocketChat · May 9, 2019 · 525293e · 525293e
1 parent 7f0a65a
commit 525293e
Show file tree

Hide file tree

Showing 40 changed files with 295 additions and 243 deletions.
diff --git a/app/api/server/helpers/composeRoomWithLastMessage.js b/app/api/server/helpers/composeRoomWithLastMessage.js
@@ -1,9 +1,10 @@
-import { composeMessageObjectWithUser } from '../../../utils';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { API } from '../api';
 
 API.helperMethods.set('composeRoomWithLastMessage', function _composeRoomWithLastMessage(room, userId) {
 	if (room.lastMessage) {
-		room.lastMessage = composeMessageObjectWithUser(room.lastMessage, userId);
+		const [lastMessage] = normalizeMessagesForUser([room.lastMessage], userId);
+		room.lastMessage = lastMessage;
 	}
 	return room;
 });
diff --git a/app/api/server/helpers/getUserFromParams.js b/app/api/server/helpers/getUserFromParams.js
@@ -11,9 +11,9 @@ API.helperMethods.set('getUserFromParams', function _getUserFromParams() {
 	if (params.userId && params.userId.trim()) {
 		user = Users.findOneById(params.userId) || doesntExist;
 	} else if (params.username && params.username.trim()) {
-		user = Users.findOneByUsername(params.username) || doesntExist;
+		user = Users.findOneByUsernameIgnoringCase(params.username) || doesntExist;
 	} else if (params.user && params.user.trim()) {
-		user = Users.findOneByUsername(params.user) || doesntExist;
+		user = Users.findOneByUsernameIgnoringCase(params.user) || doesntExist;
 	} else {
 		throw new Meteor.Error('error-user-param-not-provided', 'The required "userId" or "username" param was not provided');
 	}

diff --git a/app/api/server/v1/channels.js b/app/api/server/v1/channels.js
@@ -1,7 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { Rooms, Subscriptions, Messages, Uploads, Integrations, Users } from '../../../models';
 import { hasPermission } from '../../../authorization';
-import { composeMessageObjectWithUser } from '../../../utils';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { API } from '../api';
 import _ from 'underscore';
 
@@ -28,7 +28,8 @@ function findChannelByIdOrName({ params, checkedArchived = true, userId }) {
 		throw new Meteor.Error('error-room-archived', `The channel, ${ room.name }, is archived`);
 	}
 	if (userId && room.lastMessage) {
-		room.lastMessage = composeMessageObjectWithUser(room.lastMessage, userId);
+		const [lastMessage] = normalizeMessagesForUser([room.lastMessage], userId);
+		room.lastMessage = lastMessage;
 	}
 
 	return room;
@@ -578,7 +579,7 @@ API.v1.addRoute('channels.messages', { authRequired: true }, {
 		const messages = cursor.fetch();
 
 		return API.v1.success({
-			messages: messages.map((record) => composeMessageObjectWithUser(record, this.userId)),
+			messages: normalizeMessagesForUser(messages, this.userId),
 			count: messages.length,
 			offset,
 			total,

diff --git a/app/api/server/v1/chat.js b/app/api/server/v1/chat.js
@@ -2,7 +2,7 @@ import { Meteor } from 'meteor/meteor';
 import { Match, check } from 'meteor/check';
 import { Messages } from '../../../models';
 import { canAccessRoom, hasPermission } from '../../../authorization';
-import { composeMessageObjectWithUser } from '../../../utils';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { processWebhookMessage } from '../../../lib';
 import { API } from '../api';
 import Rooms from '../../../models/server/models/Rooms';
@@ -68,8 +68,8 @@ API.v1.addRoute('chat.syncMessages', { authRequired: true }, {
 
 		return API.v1.success({
 			result: {
-				updated: result.updated.map((message) => composeMessageObjectWithUser(message, this.userId)),
-				deleted: result.deleted.map((message) => composeMessageObjectWithUser(message, this.userId)),
+				updated: normalizeMessagesForUser(result.updated, this.userId),
+				deleted: normalizeMessagesForUser(result.deleted, this.userId),
 			},
 		});
 	},
@@ -90,8 +90,10 @@ API.v1.addRoute('chat.getMessage', { authRequired: true }, {
 			return API.v1.failure();
 		}
 
+		const [message] = normalizeMessagesForUser([msg], this.userId);
+
 		return API.v1.success({
-			message: composeMessageObjectWithUser(msg, this.userId),
+			message,
 		});
 	},
 });
@@ -111,8 +113,10 @@ API.v1.addRoute('chat.pinMessage', { authRequired: true }, {
 		let pinnedMessage;
 		Meteor.runAsUser(this.userId, () => pinnedMessage = Meteor.call('pinMessage', msg));
 
+		const [message] = normalizeMessagesForUser([pinnedMessage], this.userId);
+
 		return API.v1.success({
-			message: composeMessageObjectWithUser(pinnedMessage, this.userId),
+			message,
 		});
 	},
 });
@@ -125,10 +129,12 @@ API.v1.addRoute('chat.postMessage', { authRequired: true }, {
 			return API.v1.failure('unknown-error');
 		}
 
+		const [message] = normalizeMessagesForUser([messageReturn.message], this.userId);
+
 		return API.v1.success({
 			ts: Date.now(),
 			channel: messageReturn.channel,
-			message: composeMessageObjectWithUser(messageReturn.message, this.userId),
+			message,
 		});
 	},
 });
@@ -150,7 +156,7 @@ API.v1.addRoute('chat.search', { authRequired: true }, {
 		Meteor.runAsUser(this.userId, () => result = Meteor.call('messageSearch', searchText, roomId, count).message.docs);
 
 		return API.v1.success({
-			messages: result.map((message) => composeMessageObjectWithUser(message, this.userId)),
+			messages: normalizeMessagesForUser(result, this.userId),
 		});
 	},
 });
@@ -164,11 +170,12 @@ API.v1.addRoute('chat.sendMessage', { authRequired: true }, {
 			throw new Meteor.Error('error-invalid-params', 'The "message" parameter must be provided.');
 		}
 
-		let message;
-		Meteor.runAsUser(this.userId, () => message = Meteor.call('sendMessage', this.bodyParams.message));
+		const sent = Meteor.runAsUser(this.userId, () => Meteor.call('sendMessage', this.bodyParams.message));
+
+		const [message] = normalizeMessagesForUser([sent], this.userId);
 
 		return API.v1.success({
-			message: composeMessageObjectWithUser(message, this.userId),
+			message,
 		});
 	},
 });
@@ -259,8 +266,10 @@ API.v1.addRoute('chat.update', { authRequired: true }, {
 			Meteor.call('updateMessage', { _id: msg._id, msg: this.bodyParams.text, rid: msg.rid });
 		});
 
+		const [message] = normalizeMessagesForUser([Messages.findOneById(msg._id)], this.userId);
+
 		return API.v1.success({
-			message: composeMessageObjectWithUser(Messages.findOneById(msg._id), this.userId),
+			message,
 		});
 	},
 });

diff --git a/app/api/server/v1/groups.js b/app/api/server/v1/groups.js
@@ -4,7 +4,7 @@ import { Meteor } from 'meteor/meteor';
 
 import { Subscriptions, Rooms, Messages, Uploads, Integrations, Users } from '../../../models/server';
 import { hasPermission, canAccessRoom } from '../../../authorization/server';
-import { composeMessageObjectWithUser } from '../../../utils/server';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 
 import { API } from '../api';
 
@@ -528,7 +528,7 @@ API.v1.addRoute('groups.messages', { authRequired: true }, {
 		}).fetch();
 
 		return API.v1.success({
-			messages: messages.map((message) => composeMessageObjectWithUser(message, this.userId)),
+			messages: normalizeMessagesForUser(messages, this.userId),
 			count: messages.length,
 			offset,
 			total: Messages.find(ourQuery).count(),

diff --git a/app/api/server/v1/im.js b/app/api/server/v1/im.js
@@ -2,7 +2,7 @@ import { Meteor } from 'meteor/meteor';
 import { getRoomByNameOrIdWithOptionToJoin } from '../../../lib';
 import { Subscriptions, Uploads, Users, Messages, Rooms } from '../../../models';
 import { hasPermission } from '../../../authorization';
-import { composeMessageObjectWithUser } from '../../../utils';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import { settings } from '../../../settings';
 import { API } from '../api';
 
@@ -234,7 +234,7 @@ API.v1.addRoute(['dm.messages', 'im.messages'], { authRequired: true }, {
 		}).fetch();
 
 		return API.v1.success({
-			messages: messages.map((message) => composeMessageObjectWithUser(message, this.userId)),
+			messages: normalizeMessagesForUser(messages, this.userId),
 			count: messages.length,
 			offset,
 			total: Messages.find(ourQuery).count(),
@@ -274,7 +274,7 @@ API.v1.addRoute(['dm.messages.others', 'im.messages.others'], { authRequired: tr
 		}).fetch();
 
 		return API.v1.success({
-			messages: msgs.map((message) => composeMessageObjectWithUser(message, this.userId)),
+			messages: normalizeMessagesForUser(msgs, this.userId),
 			offset,
 			count: msgs.length,
 			total: Messages.find(ourQuery).count(),

diff --git a/app/api/server/v1/users.js b/app/api/server/v1/users.js
@@ -282,7 +282,7 @@ API.v1.addRoute('users.setAvatar', { authRequired: true }, {
 						return Users.findOneById(fields.userId, { _id: 1 });
 					}
 					if (fields.username) {
-						return Users.findOneByUsername(fields.username, { _id: 1 });
+						return Users.findOneByUsernameIgnoringCase(fields.username, { _id: 1 });
 					}
 				};
 

diff --git a/app/authorization/server/methods/addUserToRole.js b/app/authorization/server/methods/addUserToRole.js
@@ -27,7 +27,7 @@ Meteor.methods({
 			});
 		}
 
-		const user = Users.findOneByUsername(username, {
+		const user = Users.findOneByUsernameIgnoringCase(username, {
 			fields: {
 				_id: 1,
 			},

diff --git a/app/channel-settings-mail-messages/server/methods/mailMessages.js b/app/channel-settings-mail-messages/server/methods/mailMessages.js
@@ -41,7 +41,7 @@ Meteor.methods({
 		const missing = [];
 		if (data.to_users.length > 0) {
 			_.each(data.to_users, (username) => {
-				const user = Users.findOneByUsername(username);
+				const user = Users.findOneByUsernameIgnoringCase(username);
 				if (user && user.emails && user.emails[0] && user.emails[0].address) {
 					emails.push(user.emails[0].address);
 				} else {

diff --git a/app/custom-oauth/server/custom_oauth_server.js b/app/custom-oauth/server/custom_oauth_server.js
@@ -345,7 +345,7 @@ export class CustomOAuth {
 			}
 
 			if (serviceData.username) {
-				const user = Users.findOneByUsername(serviceData.username);
+				const user = Users.findOneByUsernameIgnoringCase(serviceData.username);
 				if (!user) {
 					return;
 				}

diff --git a/app/importer-csv/server/importer.js b/app/importer-csv/server/importer.js
@@ -185,7 +185,7 @@ export class CsvImporter extends Base {
 
 						// If we couldn't find one by their email address, try to find an existing user by their username
 						if (!existantUser) {
-							existantUser = Users.findOneByUsername(u.username);
+							existantUser = Users.findOneByUsernameIgnoringCase(u.username);
 						}
 
 						if (existantUser) {
@@ -271,7 +271,7 @@ export class CsvImporter extends Base {
 							for (const msgs of messagesMap.values()) {
 								for (const msg of msgs.messages) {
 									if (!this.getUserFromUsername(msg.username)) {
-										const user = Users.findOneByUsername(msg.username);
+										const user = Users.findOneByUsernameIgnoringCase(msg.username);
 										if (user) {
 											this.users.users.push({
 												rocketId: user._id,

diff --git a/app/importer-hipchat-enterprise/server/importer.js b/app/importer-hipchat-enterprise/server/importer.js
@@ -710,7 +710,7 @@ export class HipChatEnterpriseImporter extends Base {
 
 	_importUser(userToImport, startedByUserId) {
 		Meteor.runAsUser(startedByUserId, () => {
-			let existingUser = Users.findOneByUsername(userToImport.username);
+			let existingUser = Users.findOneByUsernameIgnoringCase(userToImport.username);
 			if (!existingUser) {
 				// If there's no user with that username, but there's an imported user with the same original ID and no username, use that
 				existingUser = Users.findOne({

diff --git a/app/importer-slack-users/server/importer.js b/app/importer-slack-users/server/importer.js
@@ -94,7 +94,7 @@ export class SlackUsersImporter extends Base {
 					}
 
 					Meteor.runAsUser(startedByUserId, () => {
-						const existantUser = Users.findOneByEmailAddress(u.email) || Users.findOneByUsername(u.username);
+						const existantUser = Users.findOneByEmailAddress(u.email) || Users.findOneByUsernameIgnoringCase(u.username);
 
 						let userId;
 						if (existantUser) {

diff --git a/app/importer-slack/server/importer.js b/app/importer-slack/server/importer.js
@@ -163,7 +163,7 @@ export class SlackImporter extends Base {
 					}
 
 					Meteor.runAsUser(startedByUserId, () => {
-						const existantUser = Users.findOneByEmailAddress(user.profile.email) || Users.findOneByUsername(user.name);
+						const existantUser = Users.findOneByEmailAddress(user.profile.email) || Users.findOneByUsernameIgnoringCase(user.name);
 						if (existantUser) {
 							user.rocketId = existantUser._id;
 							Users.update({ _id: user.rocketId }, { $addToSet: { importIds: user.id } });

diff --git a/app/integrations/server/lib/triggerHandler.js b/app/integrations/server/lib/triggerHandler.js
@@ -166,13 +166,13 @@ integrations.triggerHandler = new class RocketChatIntegrationHandler {
 		let user;
 		// Try to find the user who we are impersonating
 		if (trigger.impersonateUser) {
-			user = Models.Users.findOneByUsername(data.user_name);
+			user = Models.Users.findOneByUsernameIgnoringCase(data.user_name);
 		}
 
 		// If they don't exist (aka the trigger didn't contain a user) then we set the user based upon the
 		// configured username for the integration since this is required at all times.
 		if (!user) {
-			user = Models.Users.findOneByUsername(trigger.username);
+			user = Models.Users.findOneByUsernameIgnoringCase(trigger.username);
 		}
 
 		let tmpRoom;

diff --git a/app/lib/server/functions/createRoom.js b/app/lib/server/functions/createRoom.js
@@ -66,7 +66,7 @@ export const createRoom = function(type, name, owner, members, readOnly, extraDa
 		throw new Meteor.Error('error-invalid-name', 'Invalid name', { function: 'RocketChat.createRoom' });
 	}
 
-	owner = Users.findOneByUsername(owner, { fields: { username: 1 } });
+	owner = Users.findOneByUsernameIgnoringCase(owner, { fields: { username: 1 } });
 	if (!owner) {
 		throw new Meteor.Error('error-invalid-user', 'Invalid user', { function: 'RocketChat.createRoom' });
 	}

diff --git a/app/lib/server/functions/getUsernameSuggestion.js b/app/lib/server/functions/getUsernameSuggestion.js
@@ -15,7 +15,7 @@ function usernameIsAvaliable(username) {
 		return false;
 	}
 
-	return !Users.findOneByUsername(username);
+	return !Users.findOneByUsernameIgnoringCase(username);
 }
 
 

diff --git a/app/lib/server/functions/loadMessageHistory.js b/app/lib/server/functions/loadMessageHistory.js
@@ -1,6 +1,6 @@
 import { settings } from '../../../settings';
 import { Messages } from '../../../models';
-import { composeMessageObjectWithUser } from '../../../utils';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 
 const hideMessagesOfType = [];
 
@@ -41,7 +41,7 @@ export const loadMessageHistory = function loadMessageHistory({ userId, rid, end
 	} else {
 		records = Messages.findVisibleByRoomIdNotContainingTypes(rid, hideMessagesOfType, options).fetch();
 	}
-	const messages = records.map((record) => composeMessageObjectWithUser(record, userId));
+	const messages = normalizeMessagesForUser(records, userId);
 	let unreadNotLoaded = 0;
 	let firstUnread;
 

diff --git a/app/lib/server/methods/addUsersToRoom.js b/app/lib/server/methods/addUsersToRoom.js
@@ -59,7 +59,7 @@ Meteor.methods({
 		// Validate each user, then add to room
 		const user = Meteor.user();
 		data.users.forEach((username) => {
-			const newUser = Users.findOneByUsername(username);
+			const newUser = Users.findOneByUsernameIgnoringCase(username);
 			if (!newUser) {
 				throw new Meteor.Error('error-invalid-username', 'Invalid username', {
 					method: 'addUsersToRoom',

diff --git a/app/lib/server/methods/getChannelHistory.js b/app/lib/server/methods/getChannelHistory.js
@@ -3,7 +3,7 @@ import { check } from 'meteor/check';
 import { hasPermission } from '../../../authorization';
 import { Subscriptions, Messages } from '../../../models';
 import { settings } from '../../../settings';
-import { composeMessageObjectWithUser } from '../../../utils';
+import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser';
 import _ from 'underscore';
 
 Meteor.methods({
@@ -58,7 +58,7 @@ Meteor.methods({
 			records = Messages.findVisibleByRoomIdBetweenTimestamps(rid, oldest, latest, options).fetch();
 		}
 
-		const messages = records.map((record) => composeMessageObjectWithUser(record, fromUserId));
+		const messages = normalizeMessagesForUser(records, fromUserId);
 
 		if (unreads) {
 			let unreadNotLoaded = 0;

diff --git a/app/livechat/server/methods/searchAgent.js b/app/livechat/server/methods/searchAgent.js
@@ -13,7 +13,7 @@ Meteor.methods({
 			throw new Meteor.Error('error-invalid-arguments', 'Invalid arguments', { method: 'livechat:searchAgent' });
 		}
 
-		const user = Users.findOneByUsername(username, { fields: { _id: 1, username: 1 } });
+		const user = Users.findOneByUsernameIgnoringCase(username, { fields: { _id: 1, username: 1 } });
 
 		if (!user) {
 			throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'livechat:searchAgent' });

diff --git a/app/models/server/models/Users.js b/app/models/server/models/Users.js
@@ -364,16 +364,22 @@ export class Users extends Base {
 		return this.findOne({ importIds: _id }, options);
 	}
 
-	findOneByUsername(username, options) {
+	findOneByUsernameIgnoringCase(username, options) {
 		if (typeof username === 'string') {
-			username = new RegExp(`^${ username }$`, 'i');
+			username = new RegExp(`^${ s.escapeRegExp(username) }$`, 'i');
 		}
 
 		const query = { username };
 
 		return this.findOne(query, options);
 	}
 
+	findOneByUsername(username, options) {
+		const query = { username };
+
+		return this.findOne(query, options);
+	}
+
 	findOneByEmailAddress(emailAddress, options) {
 		const query = { 'emails.address': new RegExp(`^${ s.escapeRegExp(emailAddress) }$`, 'i') };
 

diff --git a/app/slackbridge/server/RocketAdapter.js b/app/slackbridge/server/RocketAdapter.js
@@ -355,9 +355,9 @@ export default class RocketAdapter {
 				const email = (rocketUserData.profile && rocketUserData.profile.email) || '';
 				let existingRocketUser;
 				if (!isBot) {
-					existingRocketUser = Users.findOneByEmailAddress(email) || Users.findOneByUsername(rocketUserData.name);
+					existingRocketUser = Users.findOneByEmailAddress(email) || Users.findOneByUsernameIgnoringCase(rocketUserData.name);
 				} else {
-					existingRocketUser = Users.findOneByUsername(rocketUserData.name);
+					existingRocketUser = Users.findOneByUsernameIgnoringCase(rocketUserData.name);
 				}
 
 				if (existingRocketUser) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -27,7 +27,7 @@ Meteor.methods({ @@
     			});
     		}
-    		const user = Users.findOneByUsername(username, {
+    		const user = Users.findOneByUsernameIgnoringCase(username, {
     			fields: {
     				_id: 1,
     			},
@@ Expand Down @@