Merge pull request #7513 from Darkneon/fix-dont-save-user-when-custom…

…-field-is-invalid [Fix] Don't save user to DB when a custom field is invalid
RocketChat · Jul 27, 2017 · fc8d0e8 · fc8d0e8
2 parents 5fade40 + b560d3b
commit fc8d0e8
Show file tree

Hide file tree

Showing 7 changed files with 323 additions and 170 deletions.
diff --git a/packages/rocketchat-api/server/v1/users.js b/packages/rocketchat-api/server/v1/users.js
@@ -19,12 +19,17 @@ RocketChat.API.v1.addRoute('users.create', { authRequired: true }, {
 			this.bodyParams.joinDefaultChannels = true;
 		}
 
+		if (this.bodyParams.customFields) {
+			RocketChat.validateCustomFields(this.bodyParams.customFields);
+		}
+
 		const newUserId = RocketChat.saveUser(this.userId, this.bodyParams);
 
 		if (this.bodyParams.customFields) {
-			RocketChat.saveCustomFields(newUserId, this.bodyParams.customFields);
+			RocketChat.saveCustomFieldsWithoutValidation(newUserId, this.bodyParams.customFields);
 		}
 
+
 		if (typeof this.bodyParams.active !== 'undefined') {
 			Meteor.runAsUser(this.userId, () => {
 				Meteor.call('setUserActiveStatus', newUserId, this.bodyParams.active);

diff --git a/packages/rocketchat-lib/package.js b/packages/rocketchat-lib/package.js
@@ -82,6 +82,7 @@ Package.onUse(function(api) {
 	api.addFiles('server/functions/removeUserFromRoom.js', 'server');
 	api.addFiles('server/functions/saveUser.js', 'server');
 	api.addFiles('server/functions/saveCustomFields.js', 'server');
+	api.addFiles('server/functions/saveCustomFieldsWithoutValidation.js', 'server');
 	api.addFiles('server/functions/sendMessage.js', 'server');
 	api.addFiles('server/functions/settings.js', 'server');
 	api.addFiles('server/functions/setUserAvatar.js', 'server');
@@ -90,6 +91,7 @@ Package.onUse(function(api) {
 	api.addFiles('server/functions/setEmail.js', 'server');
 	api.addFiles('server/functions/unarchiveRoom.js', 'server');
 	api.addFiles('server/functions/updateMessage.js', 'server');
+	api.addFiles('server/functions/validateCustomFields.js', 'server');
 	api.addFiles('server/functions/Notifications.js', 'server');
 
 	// SERVER LIB

diff --git a/packages/rocketchat-lib/server/functions/saveCustomFields.js b/packages/rocketchat-lib/server/functions/saveCustomFields.js
@@ -1,56 +1,6 @@
 RocketChat.saveCustomFields = function(userId, formData) {
 	if (s.trim(RocketChat.settings.get('Accounts_CustomFields')) !== '') {
-		let customFieldsMeta;
-		try {
-			customFieldsMeta = JSON.parse(RocketChat.settings.get('Accounts_CustomFields'));
-		} catch (e) {
-			throw new Meteor.Error('error-invalid-customfield-json', 'Invalid JSON for Custom Fields');
-		}
-
-		const customFields = {};
-
-		Object.keys(customFieldsMeta).forEach((fieldName) => {
-			const field = customFieldsMeta[fieldName];
-
-			customFields[fieldName] = formData[fieldName];
-			if (field.required && !formData[fieldName]) {
-				throw new Meteor.Error('error-user-registration-custom-field', `Field ${ fieldName } is required`, { method: 'registerUser' });
-			}
-
-			if (field.type === 'select' && field.options.indexOf(formData[fieldName]) === -1) {
-				throw new Meteor.Error('error-user-registration-custom-field', `Value for field ${ fieldName } is invalid`, { method: 'registerUser' });
-			}
-
-			if (field.maxLength && formData[fieldName].length > field.maxLength) {
-				throw new Meteor.Error('error-user-registration-custom-field', `Max length of field ${ fieldName } ${ field.maxLength }`, { method: 'registerUser' });
-			}
-
-			if (field.minLength && formData[fieldName].length < field.minLength) {
-				throw new Meteor.Error('error-user-registration-custom-field', `Min length of field ${ fieldName } ${ field.minLength }`, { method: 'registerUser' });
-			}
-		});
-
-		// for fieldName, field of customFieldsMeta
-		RocketChat.models.Users.setCustomFields(userId, customFields);
-
-		Object.keys(customFields).forEach((fieldName) => {
-			if (!customFieldsMeta[fieldName].modifyRecordField) {
-				return;
-			}
-
-			const modifyRecordField = customFieldsMeta[fieldName].modifyRecordField;
-			const update = {};
-			if (modifyRecordField.array) {
-				update.$addToSet = {};
-				update.$addToSet[modifyRecordField.field] = customFields[fieldName];
-			} else {
-				update.$set = {};
-				update.$set[modifyRecordField.field] = customFields[fieldName];
-			}
-
-			RocketChat.models.Users.update(userId, update);
-		});
-
-		return true;
+		RocketChat.validateCustomFields(formData);
+		return RocketChat.saveCustomFieldsWithoutValidation(userId, formData);
 	}
 };
diff --git a/packages/rocketchat-lib/server/functions/saveCustomFieldsWithoutValidation.js b/packages/rocketchat-lib/server/functions/saveCustomFieldsWithoutValidation.js
@@ -0,0 +1,33 @@
+RocketChat.saveCustomFieldsWithoutValidation = function(userId, formData) {
+	if (s.trim(RocketChat.settings.get('Accounts_CustomFields')) !== '') {
+		let customFieldsMeta;
+		try {
+			customFieldsMeta = JSON.parse(RocketChat.settings.get('Accounts_CustomFields'));
+		} catch (e) {
+			throw new Meteor.Error('error-invalid-customfield-json', 'Invalid JSON for Custom Fields');
+		}
+
+		const customFields = formData;
+
+		// for fieldName, field of customFieldsMeta
+		RocketChat.models.Users.setCustomFields(userId, customFields);
+
+		Object.keys(customFields).forEach((fieldName) => {
+			if (!customFieldsMeta[fieldName].modifyRecordField) {
+				return;
+			}
+
+			const modifyRecordField = customFieldsMeta[fieldName].modifyRecordField;
+			const update = {};
+			if (modifyRecordField.array) {
+				update.$addToSet = {};
+				update.$addToSet[modifyRecordField.field] = customFields[fieldName];
+			} else {
+				update.$set = {};
+				update.$set[modifyRecordField.field] = customFields[fieldName];
+			}
+
+			RocketChat.models.Users.update(userId, update);
+		});
+	}
+};
diff --git a/packages/rocketchat-lib/server/functions/validateCustomFields.js b/packages/rocketchat-lib/server/functions/validateCustomFields.js
@@ -0,0 +1,39 @@
+RocketChat.validateCustomFields = function(fields) {
+	// Special Case:
+	// If an admin didn't set any custom fields there's nothing to validate against so consider any customFields valid
+	if (s.trim(RocketChat.settings.get('Accounts_CustomFields')) === '') {
+		return;
+	}
+
+	let customFieldsMeta;
+	try {
+		customFieldsMeta = JSON.parse(RocketChat.settings.get('Accounts_CustomFields'));
+	} catch (e) {
+		throw new Meteor.Error('error-invalid-customfield-json', 'Invalid JSON for Custom Fields');
+	}
+
+	const customFields = {};
+
+	Object.keys(customFieldsMeta).forEach((fieldName) => {
+		const field = customFieldsMeta[fieldName];
+
+		customFields[fieldName] = fields[fieldName];
+		const fieldValue = s.trim(fields[fieldName]);
+
+		if (field.required && fieldValue === '') {
+			throw new Meteor.Error('error-user-registration-custom-field', `Field ${ fieldName } is required`, {method: 'registerUser'});
+		}
+
+		if (field.type === 'select' && field.options.indexOf(fields[fieldName]) === -1) {
+			throw new Meteor.Error('error-user-registration-custom-field', `Value for field ${ fieldName } is invalid`, {method: 'registerUser'});
+		}
+
+		if (field.maxLength && fieldValue.length > field.maxLength) {
+			throw new Meteor.Error('error-user-registration-custom-field', `Max length of field ${ fieldName } ${ field.maxLength }`, {method: 'registerUser'});
+		}
+
+		if (field.minLength && fieldValue.length < field.minLength) {
+			throw new Meteor.Error('error-user-registration-custom-field', `Min length of field ${ fieldName } ${ field.minLength }`, {method: 'registerUser'});
+		}
+	});
+};
diff --git a/tests/data/custom-fields.js b/tests/data/custom-fields.js
@@ -0,0 +1,28 @@
+import {getCredentials, request, api, credentials} from './api-data.js';
+
+export const customFieldText = {
+	type: 'text',
+	required: true,
+	minLength: 2,
+	maxLength: 10
+};
+
+export function setCustomFields(customFields, done) {
+	getCredentials((error) => {
+		if (error) {
+			return done(error);
+		}
+
+		const stringified = customFields ? JSON.stringify(customFields) : '';
+
+		request.post(api('settings/Accounts_CustomFields'))
+			.set(credentials)
+			.send({ 'value': stringified })
+			.expect(200)
+			.end(done);
+	});
+}
+
+export function clearCustomFields(done = () => {}) {
+	setCustomFields(null, done);
+}