[FIX] Remove properties from users.info response (RocketChat#17238)

app/api/server/v1/users.js

@@ -16,7 +16,7 @@ import {
 	setUserAvatar,
 	saveCustomFields,
 } from '../../../lib';
-import { getFullUserData, getFullUserDataById } from '../../../lib/server/functions/getFullUserData';
+import { getFullUserDataByIdOrUsername } from '../../../lib/server/functions/getFullUserData';
 import { API } from '../api';
 import { setStatusText } from '../../../lib/server';
 import { findUsersToAutocomplete } from '../lib/users';
@@ -180,20 +180,12 @@ API.v1.addRoute('users.info', { authRequired: true }, {
 	get() {
 		const { username, userId } = this.requestParams();
 		const { fields } = this.parseJsonQuery();
-		const params = {
-			userId: this.userId,
-			filter: username,
-			limit: 1,
-		};
 
-		const result = userId
-			? getFullUserDataById({ userId: this.userId, filterId: userId })
-			: getFullUserData(params);
+		const user = getFullUserDataByIdOrUsername({ userId: this.userId, filterId: userId, filterUsername: username });
 
-		if (!result || result.count() !== 1) {
+		if (!user) {
 			return API.v1.failure('User not found.');
 		}
-		const [user] = result.fetch();
 		const myself = user._id === this.userId;
 		if (fields.userRooms === 1 && (myself || hasPermission(this.userId, 'view-other-user-channels'))) {
 			user.rooms = Subscriptions.findByUserId(user._id, {

app/lib/server/functions/getFullUserData.js

@@ -64,16 +64,32 @@ const getFields = (canViewAllInfo) => ({
 	...getCustomFields(canViewAllInfo),
 });
 
-export function getFullUserDataById({ userId, filterId }) {
-	const canViewAllInfo = userId === filterId || hasPermission(userId, 'view-full-other-user-info');
+const removePasswordInfo = (user) => {
+	if (user && user.services) {
+		delete user.services.password;
+		delete user.services.email;
+		delete user.services.resume;
+		delete user.services.emailCode;
+		delete user.services.cloud;
+		delete user.services.email2fa;
+		delete user.services.totp;
+	}
+	return user;
+};
+
+export function getFullUserDataByIdOrUsername({ userId, filterId, filterUsername }) {
+	const caller = Users.findOneById(userId, { fields: { username: 1 } });
+	const myself = userId === filterId || filterUsername === caller.username;
+	const canViewAllInfo = myself || hasPermission(userId, 'view-full-other-user-info');
 
 	const fields = getFields(canViewAllInfo);
 
 	const options = {
 		fields,
 	};
+	const user = Users.findOneByIdOrUsername(filterId || filterUsername, options);
 
-	return Users.findById(filterId, options);
+	return myself ? user : removePasswordInfo(user);
 }
 
 export const getFullUserData = function({ userId, filter, limit: l }) {

app/models/server/models/Users.js

@@ -549,6 +549,18 @@ export class Users extends Base {
 		return this.findOne(query, options);
 	}
 
+	findOneByIdOrUsername(idOrUsername, options) {
+		const query = {
+			$or: [{
+				_id: idOrUsername,
+			}, {
+				username: idOrUsername,
+			}],
+		};
+
+		return this.findOne(query, options);
+	}
+
 	// FIND
 	findByIds(users, options) {
 		const query = { _id: { $in: users } };

tests/end-to-end/api/01-users.js

@@ -196,7 +196,10 @@ describe('[Users]', function() {
 	});
 
 	describe('[/users.info]', () => {
-		after(() => updatePermission('view-other-user-channels', ['admin']));
+		after(() => {
+			updatePermission('view-other-user-channels', ['admin']);
+			updatePermission('view-full-other-user-info', ['admin']);
+		});
 
 		it('should return an error when the user does not exist', (done) => {
 			request.get(api('users.info'))
@@ -212,7 +215,6 @@ describe('[Users]', function() {
 				})
 				.end(done);
 		});
-
 		it('should query information about a user by userId', (done) => {
 			request.get(api('users.info'))
 				.set(credentials)
@@ -293,6 +295,45 @@ describe('[Users]', function() {
 					.end(done);
 			});
 		});
+		it('should NOT return some services fields when request to another user\'s info even if the user has the necessary permission', (done) => {
+			updatePermission('view-full-other-user-info', ['admin']).then(() => {
+				request.get(api('users.info'))
+					.set(credentials)
+					.query({
+						userId: targetUser._id,
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+						expect(res.body).to.not.have.nested.property('user.services.emailCode');
+						expect(res.body).to.not.have.nested.property('user.services.cloud');
+						expect(res.body).to.not.have.nested.property('user.services.email2fa');
+						expect(res.body).to.not.have.nested.property('user.services.totp');
+						expect(res.body).to.not.have.nested.property('user.services.password');
+						expect(res.body).to.not.have.nested.property('user.services.email');
+						expect(res.body).to.not.have.nested.property('user.services.resume');
+					})
+					.end(done);
+			});
+		});
+		it('should return all services fields when request for myself data even without privileged permission', (done) => {
+			updatePermission('view-full-other-user-info', []).then(() => {
+				request.get(api('users.info'))
+					.set(credentials)
+					.query({
+						userId: credentials['X-User-Id'],
+					})
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+						expect(res.body).to.have.nested.property('user.services.password');
+						expect(res.body).to.have.nested.property('user.services.resume');
+					})
+					.end(done);
+			});
+		});
 	});
 	describe('[/users.getPresence]', () => {
 		it('should query a user\'s presence by userId', (done) => {