[IMPROVE] Add error messages to the creation of channels or usernames containing reserved words (RocketChat#21016)

matheusbsilva137 · sampaiodiego · web-flow · commit 49cc2c9f35fa · 2021-04-20T16:41:54.000-03:00
Co-authored-by: Diego Sampaio &lt;chinello@gmail.com&gt;
diff --git a/app/lib/server/functions/checkUsernameAvailability.js b/app/lib/server/functions/checkUsernameAvailability.js
@@ -4,6 +4,7 @@ import s from 'underscore.string';
 import { escapeRegExp } from '../../../../lib/escapeRegExp';
 import { settings } from '../../../settings';
 import { Team } from '../../../../server/sdk';
+import { validateName } from './validateName';
 
 let usernameBlackList = [];
 
@@ -17,7 +18,7 @@ const usernameIsBlocked = (username, usernameBlackList) => usernameBlackList.len
 	&& usernameBlackList.some((restrictedUsername) => restrictedUsername.test(s.trim(escapeRegExp(username))));
 
 export const checkUsernameAvailability = function(username) {
-	if (usernameIsBlocked(username, usernameBlackList)) {
+	if (usernameIsBlocked(username, usernameBlackList) || !validateName(username)) {
 		return false;
 	}
 
diff --git a/app/lib/server/functions/index.js b/app/lib/server/functions/index.js
@@ -33,4 +33,5 @@ export { _setUsername, setUsername } from './setUsername';
 export { unarchiveRoom } from './unarchiveRoom';
 export { updateMessage } from './updateMessage';
 export { validateCustomFields } from './validateCustomFields';
+export { validateName } from './validateName';
 export { getAvatarSuggestionForUser } from './getAvatarSuggestionForUser';
diff --git a/app/lib/server/functions/saveUserIdentity.js b/app/lib/server/functions/saveUserIdentity.js
@@ -3,6 +3,7 @@ import { setRealName } from './setRealName';
 import { Messages, Rooms, Subscriptions, LivechatDepartmentAgents, Users } from '../../../models/server';
 import { FileUpload } from '../../../file-upload/server';
 import { updateGroupDMsName } from './updateGroupDMsName';
+import { validateName } from './validateName';
 
 /**
  *
@@ -25,6 +26,10 @@ export function saveUserIdentity(userId, { _id, name: rawName, username: rawUser
 	const usernameChanged = previousUsername !== username;
 
 	if (typeof rawUsername !== 'undefined' && usernameChanged) {
+		if (!validateName(username)) {
+			return false;
+		}
+
 		if (!setUsername(_id, username, user)) {
 			return false;
 		}
diff --git a/app/lib/server/functions/validateName.ts b/app/lib/server/functions/validateName.ts
@@ -0,0 +1,14 @@
+import { settings } from '../../../settings/server';
+
+export const validateName = function(name: string): boolean {
+	const blockedNames = settings.get('Accounts_SystemBlockedUsernameList');
+	if (!blockedNames || typeof blockedNames !== 'string') {
+		return true;
+	}
+
+	if (blockedNames.split(',').includes(name.toLowerCase())) {
+		return false;
+	}
+
+	return true;
+};
diff --git a/app/lib/server/startup/settings.js b/app/lib/server/startup/settings.js
@@ -164,6 +164,10 @@ settings.addGroup('Accounts', function() {
 		this.add('Accounts_BlockedUsernameList', '', {
 			type: 'string',
 		});
+		this.add('Accounts_SystemBlockedUsernameList', 'admin,administrator,system,user', {
+			type: 'string',
+			hidden: true,
+		});
 		this.add('Accounts_UseDefaultBlockedDomainsList', true, {
 			type: 'boolean',
 		});
diff --git a/app/utils/lib/getValidRoomName.js b/app/utils/lib/getValidRoomName.js
@@ -3,6 +3,7 @@ import limax from 'limax';
 
 import { settings } from '../../settings';
 import { Rooms } from '../../models';
+import { validateName } from '../../lib/server/functions/validateName';
 
 export const getValidRoomName = (displayName, rid = '', options = {}) => {
 	let slugifiedName = displayName;
@@ -33,7 +34,7 @@ export const getValidRoomName = (displayName, rid = '', options = {}) => {
 		}
 	}
 
-	if (!nameValidation.test(slugifiedName)) {
+	if (!nameValidation.test(slugifiedName) || !validateName(slugifiedName)) {
 		throw new Meteor.Error('error-invalid-room-name', `${ slugifiedName } is not a valid room name.`, {
 			function: 'RocketChat.getValidRoomName',
 			channel_name: slugifiedName,
diff --git a/tests/data/api-data.js b/tests/data/api-data.js
@@ -21,6 +21,12 @@ export const apiRoleNameSubscriptions = `api${ roleNameSubscriptions }`;
 export const apiRoleScopeUsers = `${ roleScopeUsers }`;
 export const apiRoleScopeSubscriptions = `${ roleScopeSubscriptions }`;
 export const apiRoleDescription = `api${ roleDescription }`;
+export const reservedWords = [
+	'admin',
+	'administrator',
+	'system',
+	'user',
+];
 
 export const targetUser = {};
 export const channel = {};
diff --git a/tests/end-to-end/api/01-users.js b/tests/end-to-end/api/01-users.js
@@ -12,6 +12,7 @@ import {
 	targetUser,
 	log,
 	wait,
+	reservedWords,
 } from '../../data/api-data.js';
 import { adminEmail, preferences, password, adminUsername } from '../../data/user.js';
 import { imgURL } from '../../data/interactions.js';
@@ -156,6 +157,30 @@ describe('[Users]', function() {
 			});
 		});
 
+		function failCreateUser(name) {
+			it(`should not create a new user if username is the reserved word ${ name }`, (done) => {
+				request.post(api('users.create'))
+					.set(credentials)
+					.send({
+						email: `create_user_fail_${ apiEmail }`,
+						name: `create_user_fail_${ apiUsername }`,
+						username: name,
+						password,
+						active: true,
+						roles: ['user'],
+						joinDefaultChannels: true,
+						verified: true,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(400)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', false);
+						expect(res.body).to.have.property('error', `${ name } is already in use :( [error-field-unavailable]`);
+					})
+					.end(done);
+			});
+		}
+
 		function failUserWithCustomField(field) {
 			it(`should not create a user if a custom field ${ field.reason }`, (done) => {
 				setCustomFields({ customFieldText }, (error) => {
@@ -197,6 +222,10 @@ describe('[Users]', function() {
 		].forEach((field) => {
 			failUserWithCustomField(field);
 		});
+
+		reservedWords.forEach((name) => {
+			failCreateUser(name);
+		});
 	});
 
 	describe('[/users.register]', () => {
@@ -1073,6 +1102,30 @@ describe('[Users]', function() {
 					});
 			});
 		});
+
+		function failUpdateUser(name) {
+			it(`should not update an user if the new username is the reserved word ${ name }`, (done) => {
+				request.post(api('users.update'))
+					.set(credentials)
+					.send({
+						userId: targetUser._id,
+						data: {
+							username: name,
+						},
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(400)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', false);
+						expect(res.body).to.have.property('error', 'Could not save user identity [error-could-not-save-identity]');
+					})
+					.end(done);
+			});
+		}
+
+		reservedWords.forEach((name) => {
+			failUpdateUser(name);
+		});
 	});
 
 	describe('[/users.updateOwnBasicInfo]', () => {
@@ -1238,6 +1291,29 @@ describe('[Users]', function() {
 				})
 				.end(done);
 		});
+
+		function failUpdateUserOwnBasicInfo(name) {
+			it(`should not update an user's basic info if the new username is the reserved word ${ name }`, (done) => {
+				request.post(api('users.updateOwnBasicInfo'))
+					.set(credentials)
+					.send({
+						data: {
+							username: name,
+						},
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(400)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', false);
+						expect(res.body).to.have.property('error', 'Could not save user identity [error-could-not-save-identity]');
+					})
+					.end(done);
+			});
+		}
+
+		reservedWords.forEach((name) => {
+			failUpdateUserOwnBasicInfo(name);
+		});
 	});
 
 	describe('[/users.setPreferences]', () => {
diff --git a/tests/end-to-end/api/02-channels.js b/tests/end-to-end/api/02-channels.js
@@ -7,6 +7,7 @@ import {
 	credentials,
 	apiPublicChannelName,
 	channel,
+	reservedWords,
 } from '../../data/api-data.js';
 import { adminUsername, password } from '../../data/user.js';
 import { createUser, login } from '../../data/users.helper';
@@ -836,6 +837,28 @@ describe('[Channels]', function() {
 	it('/channels.rename', async () => {
 		const roomInfo = await getRoomInfo(channel._id);
 
+		function failRenameChannel(name) {
+			it(`should not rename a channel to the reserved name ${ name }`, (done) => {
+				request.post(api('channels.rename'))
+					.set(credentials)
+					.send({
+						roomId: channel._id,
+						name,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(400)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', false);
+						expect(res.body).to.have.property('error', `${ name } is already in use :( [error-field-unavailable]`);
+					})
+					.end(done);
+			});
+		}
+
+		reservedWords.forEach((name) => {
+			failRenameChannel(name);
+		});
+
 		return request.post(api('channels.rename'))
 			.set(credentials)
 			.send({

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@ import limax from 'limax';`
`3`	`3`
`4`	`4`	`import { settings } from '../../settings';`
`5`	`5`	`import { Rooms } from '../../models';`
	`6`	`+import { validateName } from '../../lib/server/functions/validateName';`
`6`	`7`
`7`	`8`	`export const getValidRoomName = (displayName, rid = '', options = {}) => {`
`8`	`9`	`let slugifiedName = displayName;`
`@@ -33,7 +34,7 @@ export const getValidRoomName = (displayName, rid = '', options = {}) => {`
`33`	`34`	`}`
`34`	`35`	`}`
`35`	`36`
`36`		`- if (!nameValidation.test(slugifiedName)) {`
	`37`	`+ if (!nameValidation.test(slugifiedName) \|\| !validateName(slugifiedName)) {`
`37`	`38`	throw new Meteor.Error('error-invalid-room-name', `${ slugifiedName } is not a valid room name.`, {
`38`	`39`	`function: 'RocketChat.getValidRoomName',`
`39`	`40`	`channel_name: slugifiedName,`