Merge pull request #8408 from RocketChat/rest-api-improve-json-query-…

…helper [FIX] Duplicate code in rest api letting in a few bugs with the rest api # Conflicts: # packages/rocketchat-api/server/api.js # packages/rocketchat-api/server/v1/channels.js # packages/rocketchat-api/server/v1/groups.js # packages/rocketchat-api/server/v1/im.js
RocketChat · Oct 6, 2017 · c56d52d · c56d52d
1 parent 3f1aa4e
commit c56d52d
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 39 deletions.
diff --git a/packages/rocketchat-api/server/api.js b/packages/rocketchat-api/server/api.js
@@ -5,10 +5,28 @@ class API extends Restivus {
 		this.logger = new Logger(`API ${ properties.version ? properties.version : 'default' } Logger`, {});
 		this.authMethods = [];
 		this.helperMethods = new Map();
+		this.fieldSeparator = '.';
 		this.defaultFieldsToExclude = {
 			joinCode: 0,
 			$loki: 0,
-			meta: 0
+			meta: 0,
+			members: 0,
+			importIds: 0
+		};
+		this.limitedUserFieldsToExclude = {
+			avatarOrigin: 0,
+			emails: 0,
+			phone: 0,
+			statusConnection: 0,
+			createdAt: 0,
+			lastLogin: 0,
+			services: 0,
+			requirePasswordChange: 0,
+			requirePasswordChangeReason: 0,
+			roles: 0,
+			statusDefault: 0,
+			_updatedAt: 0,
+			customFields: 0
 		};
 
 		this._config.defaultOptionsEndpoint = function() {

diff --git a/packages/rocketchat-api/server/v1/channels.js b/packages/rocketchat-api/server/v1/channels.js
@@ -208,7 +208,7 @@ RocketChat.API.v1.addRoute('channels.getIntegrations', { authRequired: true }, {
 			sort: sort ? sort : { _createdAt: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		}).fetch();
 
 		return RocketChat.API.v1.success({
@@ -352,7 +352,7 @@ RocketChat.API.v1.addRoute('channels.list', { authRequired: true }, {
 				sort: sort ? sort : { name: 1 },
 				skip: offset,
 				limit: count,
-				fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+				fields
 			}).fetch();
 
 			return RocketChat.API.v1.success({
@@ -376,7 +376,7 @@ RocketChat.API.v1.addRoute('channels.list.joined', { authRequired: true }, {
 			sort: sort ? sort : { name: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		});
 
 		return RocketChat.API.v1.success({

diff --git a/packages/rocketchat-api/server/v1/groups.js b/packages/rocketchat-api/server/v1/groups.js
@@ -168,7 +168,7 @@ RocketChat.API.v1.addRoute('groups.getIntegrations', { authRequired: true }, {
 			sort: sort ? sort : { _createdAt: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		}).fetch();
 
 		return RocketChat.API.v1.success({
@@ -284,7 +284,7 @@ RocketChat.API.v1.addRoute('groups.list', { authRequired: true }, {
 			sort: sort ? sort : { name: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		});
 
 		return RocketChat.API.v1.success({

diff --git a/packages/rocketchat-api/server/v1/helpers/parseJsonQuery.js b/packages/rocketchat-api/server/v1/helpers/parseJsonQuery.js
@@ -19,6 +19,26 @@ RocketChat.API.v1.helperMethods.set('parseJsonQuery', function _parseJsonQuery()
 		}
 	}
 
+	// Verify the user's selected fields only contains ones which their role allows
+	if (typeof fields === 'object') {
+		let nonSelectableFields = Object.keys(RocketChat.API.v1.defaultFieldsToExclude);
+		if (!RocketChat.authz.hasPermission(this.userId, 'view-full-other-user-info') && this.request.route.includes('/v1/users.')) {
+			nonSelectableFields = nonSelectableFields.concat(Object.keys(RocketChat.API.v1.limitedUserFieldsToExclude));
+		}
+
+		Object.keys(fields).forEach((k) => {
+			if (nonSelectableFields.includes(k) || nonSelectableFields.includes(k.split(RocketChat.API.v1.fieldSeparator)[0])) {
+				delete fields[k];
+			}
+		});
+	}
+
+	// Limit the fields by default
+	fields = Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude);
+	if (!RocketChat.authz.hasPermission(this.userId, 'view-full-other-user-info') && this.request.route.includes('/v1/users.')) {
+		fields = Object.assign(fields, RocketChat.API.v1.limitedUserFieldsToExclude);
+	}
+
 	let query;
 	if (this.queryParams.query) {
 		try {
@@ -29,6 +49,20 @@ RocketChat.API.v1.helperMethods.set('parseJsonQuery', function _parseJsonQuery()
 		}
 	}
 
+	// Verify the user has permission to query the fields they are
+	if (typeof query === 'object') {
+		let nonQuerableFields = Object.keys(RocketChat.API.v1.defaultFieldsToExclude);
+		if (!RocketChat.authz.hasPermission(this.userId, 'view-full-other-user-info') && this.request.route.includes('/v1/users.')) {
+			nonQuerableFields = nonQuerableFields.concat(Object.keys(RocketChat.API.v1.limitedUserFieldsToExclude));
+		}
+
+		Object.keys(query).forEach((k) => {
+			if (nonQuerableFields.includes(k) || nonQuerableFields.includes(k.split(RocketChat.API.v1.fieldSeparator)[0])) {
+				delete query[k];
+			}
+		});
+	}
+
 	return {
 		sort,
 		fields,

diff --git a/packages/rocketchat-api/server/v1/im.js b/packages/rocketchat-api/server/v1/im.js
@@ -106,7 +106,7 @@ RocketChat.API.v1.addRoute(['dm.messages.others', 'im.messages.others'], { authR
 			sort: sort ? sort : { ts: -1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		}).fetch();
 
 		return RocketChat.API.v1.success({
@@ -129,7 +129,7 @@ RocketChat.API.v1.addRoute(['dm.list', 'im.list'], { authRequired: true }, {
 			sort: sort ? sort : { name: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		});
 
 		return RocketChat.API.v1.success({
@@ -156,7 +156,7 @@ RocketChat.API.v1.addRoute(['dm.list.everyone', 'im.list.everyone'], { authRequi
 			sort: sort ? sort : { name: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		}).fetch();
 
 		return RocketChat.API.v1.success({

diff --git a/packages/rocketchat-api/server/v1/stats.js b/packages/rocketchat-api/server/v1/stats.js
@@ -25,20 +25,18 @@ RocketChat.API.v1.addRoute('statistics.list', { authRequired: true }, {
 		const { offset, count } = this.getPaginationItems();
 		const { sort, fields, query } = this.parseJsonQuery();
 
-		const ourQuery = Object.assign({}, query);
-
-		const statistics = RocketChat.models.Statistics.find(ourQuery, {
+		const statistics = RocketChat.models.Statistics.find(query, {
 			sort: sort ? sort : { name: 1 },
 			skip: offset,
 			limit: count,
-			fields: Object.assign({}, fields, RocketChat.API.v1.defaultFieldsToExclude)
+			fields
 		}).fetch();
 
 		return RocketChat.API.v1.success({
 			statistics,
 			count: statistics.length,
 			offset,
-			total: RocketChat.models.Statistics.find(ourQuery).count()
+			total: RocketChat.models.Statistics.find(query).count()
 		});
 	}
 });
diff --git a/packages/rocketchat-api/server/v1/users.js b/packages/rocketchat-api/server/v1/users.js
@@ -112,40 +112,18 @@ RocketChat.API.v1.addRoute('users.list', { authRequired: true }, {
 		const { offset, count } = this.getPaginationItems();
 		const { sort, fields, query } = this.parseJsonQuery();
 
-		let fieldsToKeepFromRegularUsers;
-		if (!RocketChat.authz.hasPermission(this.userId, 'view-full-other-user-info')) {
-			fieldsToKeepFromRegularUsers = {
-				avatarOrigin: 0,
-				emails: 0,
-				phone: 0,
-				statusConnection: 0,
-				createdAt: 0,
-				lastLogin: 0,
-				services: 0,
-				requirePasswordChange: 0,
-				requirePasswordChangeReason: 0,
-				roles: 0,
-				statusDefault: 0,
-				_updatedAt: 0,
-				customFields: 0
-			};
-		}
-
-		const ourQuery = Object.assign({}, query);
-		const ourFields = Object.assign({}, fields, fieldsToKeepFromRegularUsers, RocketChat.API.v1.defaultFieldsToExclude);
-
-		const users = RocketChat.models.Users.find(ourQuery, {
+		const users = RocketChat.models.Users.find(query, {
 			sort: sort ? sort : { username: 1 },
 			skip: offset,
 			limit: count,
-			fields: ourFields
+			fields
 		}).fetch();
 
 		return RocketChat.API.v1.success({
 			users,
 			count: users.length,
 			offset,
-			total: RocketChat.models.Users.find(ourQuery).count()
+			total: RocketChat.models.Users.find(query).count()
 		});
 	}
 });