fix: Coerce values on appearance endpoint from setting data types (#3…

…1528)
RocketChat · Jan 26, 2024 · baf5694 · baf5694
1 parent e0982dc
commit baf5694
Show file tree

Hide file tree

Showing 4 changed files with 115 additions and 6 deletions.
diff --git a/apps/meteor/app/livechat/imports/server/rest/appearance.ts b/apps/meteor/app/livechat/imports/server/rest/appearance.ts
@@ -1,6 +1,7 @@
 import { Settings } from '@rocket.chat/models';
 import { isPOSTLivechatAppearanceParams } from '@rocket.chat/rest-typings';
 
+import { isTruthy } from '../../../../../lib/isTruthy';
 import { API } from '../../../../api/server';
 import { findAppearance } from '../../../server/api/lib/appearance';
 
@@ -52,8 +53,35 @@ API.v1.addRoute(
 				throw new Error('invalid-setting');
 			}
 
+			const dbSettings = await Settings.findByIds(validSettingList, { projection: { _id: 1, value: 1, type: 1 } })
+				.map((dbSetting) => {
+					const setting = settings.find(({ _id }) => _id === dbSetting._id);
+					if (!setting || dbSetting.value === setting.value) {
+						return;
+					}
+
+					switch (dbSetting?.type) {
+						case 'boolean':
+							return {
+								_id: dbSetting._id,
+								value: setting.value === 'true' || setting.value === true,
+							};
+						case 'int':
+							return {
+								_id: dbSetting._id,
+								value: coerceInt(setting.value),
+							};
+						default:
+							return {
+								_id: dbSetting._id,
+								value: setting?.value,
+							};
+					}
+				})
+				.toArray();
+
 			await Promise.all(
-				settings.map((setting) => {
+				dbSettings.filter(isTruthy).map((setting) => {
 					return Settings.updateValueById(setting._id, setting.value);
 				}),
 			);
@@ -62,3 +90,20 @@ API.v1.addRoute(
 		},
 	},
 );
+
+function coerceInt(value: string | number | boolean): number {
+	if (typeof value === 'number') {
+		return value;
+	}
+
+	if (typeof value === 'boolean') {
+		return 0;
+	}
+
+	const parsedValue = parseInt(value, 10);
+	if (Number.isNaN(parsedValue)) {
+		return 0;
+	}
+
+	return parsedValue;
+}
diff --git a/apps/meteor/server/models/raw/Settings.ts b/apps/meteor/server/models/raw/Settings.ts
@@ -1,6 +1,6 @@
 import type { ISetting, ISettingColor, ISettingSelectOption, RocketChatRecordDeleted } from '@rocket.chat/core-typings';
 import type { ISettingsModel } from '@rocket.chat/model-typings';
-import type { Collection, FindCursor, Db, Filter, UpdateFilter, UpdateResult, Document } from 'mongodb';
+import type { Collection, FindCursor, Db, Filter, UpdateFilter, UpdateResult, Document, FindOptions } from 'mongodb';
 
 import { BaseRaw } from './BaseRaw';
 
@@ -36,7 +36,7 @@ export class SettingsRaw extends BaseRaw<ISetting> implements ISettingsModel {
 		return this.findOne(query);
 	}
 
-	findByIds(_id: string[] | string = []): FindCursor<ISetting> {
+	findByIds(_id: string[] | string = [], options?: FindOptions<ISetting>): FindCursor<ISetting> {
 		if (typeof _id === 'string') {
 			_id = [_id];
 		}
@@ -47,7 +47,7 @@ export class SettingsRaw extends BaseRaw<ISetting> implements ISettingsModel {
 			},
 		};
 
-		return this.find(query);
+		return this.find(query, options);
 	}
 
 	updateValueById(

diff --git a/apps/meteor/tests/end-to-end/api/livechat/02-appearance.ts b/apps/meteor/tests/end-to-end/api/livechat/02-appearance.ts
@@ -116,5 +116,69 @@ describe('LIVECHAT - appearance', function () {
 			expect(body.config.settings.limitTextLength).to.be.equal(100);
 			await updateSetting('Livechat_enable_message_character_limit', false);
 		});
+		it('should coerce the value of a setting based on its stored datatype (int)', async () => {
+			await updateSetting('Livechat_enable_message_character_limit', true);
+			await request
+				.post(api('livechat/appearance'))
+				.set(credentials)
+				.send([{ _id: 'Livechat_message_character_limit', value: '100' }])
+				.expect(200);
+
+			// Get data from livechat/config
+			const { body } = await request.get(api('livechat/config')).set(credentials).expect(200);
+			expect(body.config.settings.limitTextLength).to.be.equal(100);
+			await updateSetting('Livechat_enable_message_character_limit', false);
+		});
+		it('should coerce the value of a setting based on its stored datatype (boolean)', async () => {
+			await request
+				.post(api('livechat/appearance'))
+				.set(credentials)
+				.send([{ _id: 'Livechat_registration_form', value: 'true' }])
+				.expect(200);
+
+			// Get data from livechat/config
+			const { body } = await request.get(api('livechat/config')).set(credentials).expect(200);
+			expect(body.config.settings.registrationForm).to.be.true;
+		});
+		it('should coerce an invalid number value to zero', async () => {
+			await request
+				.post(api('livechat/appearance'))
+				.set(credentials)
+				.send([
+					{ _id: 'Livechat_message_character_limit', value: 'xxxx' },
+					{ _id: 'Livechat_enable_message_character_limit', value: true },
+				])
+				.expect(200);
+
+			// Get data from livechat/config
+			const { body } = await request.get(api('livechat/config')).set(credentials).expect(200);
+			// When setting is 0, we default to Message_MaxAllowedSize value
+			expect(body.config.settings.limitTextLength).to.be.equal(5000);
+		});
+		it('should coerce a boolean value on an int setting to 0', async () => {
+			await request
+				.post(api('livechat/appearance'))
+				.set(credentials)
+				.send([
+					{ _id: 'Livechat_message_character_limit', value: true },
+					{ _id: 'Livechat_enable_message_character_limit', value: true },
+				])
+				.expect(200);
+
+			// Get data from livechat/config
+			const { body } = await request.get(api('livechat/config')).set(credentials).expect(200);
+			expect(body.config.settings.limitTextLength).to.be.equal(5000);
+		});
+		it('should coerce a non boolean value on a boolean setting to false', async () => {
+			await request
+				.post(api('livechat/appearance'))
+				.set(credentials)
+				.send([{ _id: 'Livechat_enable_message_character_limit', value: 'xxxx' }])
+				.expect(200);
+
+			// Get data from livechat/config
+			const { body } = await request.get(api('livechat/config')).set(credentials).expect(200);
+			expect(body.config.settings.limitTextLength).to.be.false;
+		});
 	});
 });
diff --git a/packages/model-typings/src/models/ISettingsModel.ts b/packages/model-typings/src/models/ISettingsModel.ts
@@ -1,5 +1,5 @@
 import type { ISetting, ISettingColor, ISettingSelectOption } from '@rocket.chat/core-typings';
-import type { FindCursor, UpdateFilter, UpdateResult, Document } from 'mongodb';
+import type { FindCursor, UpdateFilter, UpdateResult, Document, FindOptions } from 'mongodb';
 
 import type { IBaseModel } from './IBaseModel';
 
@@ -10,7 +10,7 @@ export interface ISettingsModel extends IBaseModel<ISetting> {
 
 	findOneNotHiddenById(_id: string): Promise<ISetting | null>;
 
-	findByIds(_id?: string[] | string): FindCursor<ISetting>;
+	findByIds(_id?: string[] | string, options?: FindOptions<ISetting>): FindCursor<ISetting>;
 
 	updateValueById(
 		_id: string,