[FIX] Do not allow admin users edit other federated users (#26528)

* fix: do not allow edit federate users * fix: fix bad typing
RocketChat · Aug 10, 2022 · 8b0a018 · 8b0a018
1 parent 3ebbc82
commit 8b0a018
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 6 deletions.
diff --git a/apps/meteor/app/lib/server/functions/getFullUserData.ts b/apps/meteor/app/lib/server/functions/getFullUserData.ts
@@ -20,6 +20,7 @@ const defaultFields = {
 	statusText: 1,
 	avatarETag: 1,
 	extension: 1,
+	federated: 1,
 } as const;
 
 const fullFields = {

diff --git a/apps/meteor/app/lib/server/functions/saveUser.js b/apps/meteor/app/lib/server/functions/saveUser.js
@@ -3,6 +3,7 @@ import { Accounts } from 'meteor/accounts-base';
 import _ from 'underscore';
 import s from 'underscore.string';
 import { Gravatar } from 'meteor/jparker:gravatar';
+import { isUserFederated } from '@rocket.chat/core-typings';
 
 import * as Mailer from '../../../mailer';
 import { getRoles, hasPermission } from '../../../authorization';
@@ -329,6 +330,10 @@ const saveNewUser = function (userData, sendPassword) {
 };
 
 export const saveUser = function (userId, userData) {
+	const oldUserData = Users.findOneById(userData._id);
+	if (oldUserData && isUserFederated(oldUserData)) {
+		throw new Meteor.Error('Edit_Federated_User_Not_Allowed', 'Not possible to edit a federated user');
+	}
 	validateUserData(userId, userData);
 	let sendPassword = false;
 
@@ -348,8 +353,6 @@ export const saveUser = function (userId, userData) {
 
 	validateUserEditing(userId, userData);
 
-	const oldUserData = Users.findOneById(userData._id);
-
 	// update user
 	if (userData.hasOwnProperty('username') || userData.hasOwnProperty('name')) {
 		if (

diff --git a/apps/meteor/client/views/admin/users/AdminUserInfoActions.tsx b/apps/meteor/client/views/admin/users/AdminUserInfoActions.tsx
@@ -14,13 +14,22 @@ import { useResetTOTPAction } from './hooks/useResetTOTPAction';
 type AdminUserInfoActionsProps = {
 	username: IUser['username'];
 	userId: IUser['_id'];
+	isAFederatedUser: IUser['federated'];
 	isActive: boolean;
 	isAdmin: boolean;
 	onChange: () => void;
 	onReload: () => void;
 };
 
-const AdminUserInfoActions = ({ username, userId, isActive, isAdmin, onChange, onReload }: AdminUserInfoActionsProps): ReactElement => {
+const AdminUserInfoActions = ({
+	username,
+	userId,
+	isAFederatedUser,
+	isActive,
+	isAdmin,
+	onChange,
+	onReload,
+}: AdminUserInfoActionsProps): ReactElement => {
 	const t = useTranslation();
 	const directRoute = useRoute('direct');
 	const userRoute = useRoute('admin-users');
@@ -57,14 +66,17 @@ const AdminUserInfoActions = ({ username, userId, isActive, isAdmin, onChange, o
 				directMessage: {
 					icon: 'balloon',
 					label: t('Direct_Message'),
+					title: t('Direct_Message'),
 					action: directMessageClick,
 				},
 			}),
 			...(canEditOtherUserInfo && {
 				editUser: {
 					icon: 'edit',
 					label: t('Edit'),
+					title: isAFederatedUser ? t('Edit_Federated_User_Not_Allowed') : t('Edit'),
 					action: editUserClick,
+					disabled: isAFederatedUser,
 				},
 			}),
 			...(changeAdminStatusAction && { makeAdmin: changeAdminStatusAction }),
@@ -84,6 +96,7 @@ const AdminUserInfoActions = ({ username, userId, isActive, isAdmin, onChange, o
 			deleteUserAction,
 			resetE2EKeyAction,
 			resetTOTPAction,
+			isAFederatedUser,
 		],
 	);
 
@@ -110,8 +123,8 @@ const AdminUserInfoActions = ({ username, userId, isActive, isAdmin, onChange, o
 
 	// TODO: sanitize Action type to avoid any
 	const actions = useMemo(() => {
-		const mapAction = ([key, { label, icon, action }]: any): ReactElement => (
-			<UserInfo.Action key={key} title={label} label={label} onClick={action} icon={icon} />
+		const mapAction = ([key, { label, icon, action, disabled, title }]: any): ReactElement => (
+			<UserInfo.Action key={key} title={title} label={label} onClick={action} disabled={disabled} icon={icon} />
 		);
 		return [...actionsDefinition.map(mapAction), menu].filter(Boolean);
 	}, [actionsDefinition, menu]);

diff --git a/apps/meteor/client/views/admin/users/AdminUserInfoWithData.tsx b/apps/meteor/client/views/admin/users/AdminUserInfoWithData.tsx
@@ -109,6 +109,7 @@ const AdminUserInfoWithData = ({ uid, onReload }: AdminUserInfoWithDataProps): R
 					isAdmin={data?.user.roles.includes('admin')}
 					userId={data?.user._id}
 					username={user.username}
+					isAFederatedUser={data?.user.federated}
 					onChange={onChange}
 					onReload={onReload}
 				/>

diff --git a/apps/meteor/client/views/admin/users/EditUserWithData.tsx b/apps/meteor/client/views/admin/users/EditUserWithData.tsx
@@ -1,4 +1,4 @@
-import { IUser } from '@rocket.chat/core-typings';
+import { isUserFederated, IUser } from '@rocket.chat/core-typings';
 import { Box, Callout } from '@rocket.chat/fuselage';
 import { useTranslation } from '@rocket.chat/ui-contexts';
 import React, { useMemo, ReactElement } from 'react';
@@ -41,6 +41,14 @@ const EditUserWithData = ({ uid, onReload, ...props }: EditUserWithDataProps): R
 		);
 	}
 
+	if (data?.user && isUserFederated({ federated: data?.user.federated })) {
+		return (
+			<Callout m='x16' type='danger'>
+				{t('Edit_Federated_User_Not_Allowed')}
+			</Callout>
+		);
+	}
+
 	return <EditUser data={data?.user} roles={roleData?.roles} onReload={onReload} {...props} />;
 };
 

diff --git a/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json b/apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json
@@ -1628,6 +1628,7 @@
   "Edit_Canned_Responses": "Edit Canned Responses",
   "Edit_Custom_Field": "Edit Custom Field",
   "Edit_Department": "Edit Department",
+  "Edit_Federated_User_Not_Allowed": "Not possible to edit a federated user",
   "Edit_Invite": "Edit Invite",
   "Edit_previous_message": "`%s` - Edit previous message",
   "Edit_Priority": "Edit Priority",