[FIX] Disabling admin actions for federated users (#27251)

* fix: disabling admin actions + do not count only federated and remote users as seat cap * fix: consider as federated users only remote users * chore: fix lint * fix: make sure everything is being validated on BE as well * fix: removing forgotten log * fix: apply suggestions from review * fix: apply suggestions from review (WIP) * fix: apply suggestions from review
RocketChat · Feb 13, 2023 · ca6190f · ca6190f
1 parent 6027839
commit ca6190f
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 21 deletions.
diff --git a/apps/meteor/app/2fa/server/functions/resetTOTP.ts b/apps/meteor/app/2fa/server/functions/resetTOTP.ts
@@ -1,6 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { TAPi18n } from 'meteor/rocketchat:tap-i18n';
 import type { IUser } from '@rocket.chat/core-typings';
+import { isUserFederated } from '@rocket.chat/core-typings';
 import { Users } from '@rocket.chat/models';
 
 import { settings } from '../../../settings/server';
@@ -60,6 +61,11 @@ export async function resetTOTP(userId: string, notifyUser = false): Promise<boo
 		await sendResetNotification(userId);
 	}
 
+	const user = await Users.findOneById(userId, { projection: { federated: 1, username: 1 } });
+	if (!user || isUserFederated(user)) {
+		throw new Meteor.Error('error-not-allowed', 'Federated Users cant have TOTP', { function: 'resetTOTP' });
+	}
+
 	const result = await Users.resetTOTPById(userId);
 
 	if (result?.modifiedCount === 1) {

diff --git a/apps/meteor/app/lib/server/functions/deleteUser.ts b/apps/meteor/app/lib/server/functions/deleteUser.ts
@@ -1,6 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { TAPi18n } from 'meteor/rocketchat:tap-i18n';
 import type { FileProp } from '@rocket.chat/core-typings';
+import { isUserFederated } from '@rocket.chat/core-typings';
 import { Integrations, FederationServers, LivechatVisitors } from '@rocket.chat/models';
 import { api } from '@rocket.chat/core-services';
 
@@ -15,19 +16,15 @@ import { LivechatUnitMonitors } from '../../../../ee/app/models/server';
 
 export async function deleteUser(userId: string, confirmRelinquish = false): Promise<void> {
 	const user = Users.findOneById(userId, {
-		fields: { username: 1, avatarOrigin: 1, federation: 1, roles: 1 },
+		fields: { username: 1, avatarOrigin: 1, roles: 1, federated: 1 },
 	});
 
 	if (!user) {
 		return;
 	}
 
-	if (user.federation) {
-		const existingSubscriptions = Subscriptions.find({ 'u._id': user._id }).count();
-
-		if (existingSubscriptions > 0) {
-			throw new Meteor.Error('FEDERATION_Error_user_is_federated_on_rooms');
-		}
+	if (isUserFederated(user)) {
+		throw new Meteor.Error('error-user-is-federated', 'Cannot delete federated user');
 	}
 
 	const subscribedRooms = getSubscribedRoomsForUserWithDetails(userId);

diff --git a/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts b/apps/meteor/app/lib/server/functions/setUserActiveStatus.ts
@@ -2,6 +2,7 @@ import { Meteor } from 'meteor/meteor';
 import { check } from 'meteor/check';
 import { Accounts } from 'meteor/accounts-base';
 import type { IUser, IUserEmail, IDirectMessageRoom } from '@rocket.chat/core-typings';
+import { isUserFederated } from '@rocket.chat/core-typings';
 
 import * as Mailer from '../../../mailer';
 import { Users, Subscriptions, Rooms } from '../../../models/server';
@@ -43,6 +44,12 @@ export function setUserActiveStatus(userId: string, active: boolean, confirmReli
 		return false;
 	}
 
+	if (isUserFederated(user)) {
+		throw new Meteor.Error('error-user-is-federated', 'Cannot change federated users status', {
+			method: 'setUserActiveStatus',
+		});
+	}
+
 	// Users without username can't do anything, so there is no need to check for owned rooms
 	if (user.username != null && !active) {
 		const userAdmin = Users.findOneAdmin(userId);

diff --git a/apps/meteor/app/lib/server/methods/setAdminStatus.ts b/apps/meteor/app/lib/server/methods/setAdminStatus.ts
@@ -1,5 +1,6 @@
 import { Meteor } from 'meteor/meteor';
 import { Match, check } from 'meteor/check';
+import { isUserFederated } from '@rocket.chat/core-typings';
 
 import { hasPermission } from '../../../authorization/server';
 
@@ -18,7 +19,10 @@ Meteor.methods({
 			throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'setAdminStatus' });
 		}
 
-		const user = Meteor.users.findOne({ _id: userId }, { fields: { username: 1 } });
+		const user = Meteor.users.findOne({ _id: userId }, { fields: { username: 1, federated: 1 } });
+		if (!user || isUserFederated(user)) {
+			throw new Meteor.Error('error-not-allowed', 'Federated Users cant be admins', { method: 'setAdminStatus' });
+		}
 
 		if (admin) {
 			return Meteor.call('authorization:addUserToRole', 'admin', user?.username);

diff --git a/apps/meteor/client/views/admin/users/AdminUserInfoActions.tsx b/apps/meteor/client/views/admin/users/AdminUserInfoActions.tsx
@@ -15,7 +15,7 @@ import { useResetTOTPAction } from './hooks/useResetTOTPAction';
 type AdminUserInfoActionsProps = {
 	username: IUser['username'];
 	userId: IUser['_id'];
-	isAFederatedUser: IUser['federated'];
+	isFederatedUser: IUser['federated'];
 	isActive: boolean;
 	isAdmin: boolean;
 	onChange: () => void;
@@ -25,7 +25,7 @@ type AdminUserInfoActionsProps = {
 const AdminUserInfoActions = ({
 	username,
 	userId,
-	isAFederatedUser,
+	isFederatedUser,
 	isActive,
 	isAdmin,
 	onChange,
@@ -75,16 +75,16 @@ const AdminUserInfoActions = ({
 				editUser: {
 					icon: 'edit',
 					label: t('Edit'),
-					title: isAFederatedUser ? t('Edit_Federated_User_Not_Allowed') : t('Edit'),
+					title: isFederatedUser ? t('Edit_Federated_User_Not_Allowed') : t('Edit'),
 					action: editUserClick,
-					disabled: isAFederatedUser,
+					disabled: isFederatedUser,
 				},
 			}),
-			...(changeAdminStatusAction && { makeAdmin: changeAdminStatusAction }),
-			...(resetE2EKeyAction && { resetE2EKey: resetE2EKeyAction }),
-			...(resetTOTPAction && { resetTOTP: resetTOTPAction }),
-			...(deleteUserAction && { delete: deleteUserAction }),
-			...(changeUserStatusAction && { changeActiveStatus: changeUserStatusAction }),
+			...(changeAdminStatusAction && !isFederatedUser && { makeAdmin: changeAdminStatusAction }),
+			...(resetE2EKeyAction && !isFederatedUser && { resetE2EKey: resetE2EKeyAction }),
+			...(resetTOTPAction && !isFederatedUser && { resetTOTP: resetTOTPAction }),
+			...(deleteUserAction && !isFederatedUser && { delete: deleteUserAction }),
+			...(changeUserStatusAction && !isFederatedUser && { changeActiveStatus: changeUserStatusAction }),
 		}),
 		[
 			t,
@@ -97,7 +97,7 @@ const AdminUserInfoActions = ({
 			deleteUserAction,
 			resetE2EKeyAction,
 			resetTOTPAction,
-			isAFederatedUser,
+			isFederatedUser,
 		],
 	);
 

diff --git a/apps/meteor/client/views/admin/users/AdminUserInfoWithData.tsx b/apps/meteor/client/views/admin/users/AdminUserInfoWithData.tsx
@@ -1,3 +1,4 @@
+import { isUserFederated } from '@rocket.chat/core-typings';
 import type { IUser } from '@rocket.chat/core-typings';
 import { Callout } from '@rocket.chat/fuselage';
 import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
@@ -118,7 +119,7 @@ const AdminUserInfoWithData = ({ uid, onReload }: AdminUserInfoWithDataProps): R
 					isAdmin={data?.user.roles.includes('admin')}
 					userId={data?.user._id}
 					username={user.username}
-					isAFederatedUser={data?.user.federated}
+					isFederatedUser={isUserFederated(data?.user as unknown as IUser)}
 					onChange={onChange}
 					onReload={onReload}
 				/>

diff --git a/apps/meteor/client/views/admin/users/EditUserWithData.tsx b/apps/meteor/client/views/admin/users/EditUserWithData.tsx
@@ -67,7 +67,7 @@ const EditUserWithData = ({ uid, onReload, ...props }: EditUserWithDataProps): R
 		);
 	}
 
-	if (data?.user && isUserFederated({ federated: data?.user.federated })) {
+	if (data?.user && isUserFederated(data?.user as unknown as IUser)) {
 		return (
 			<Callout m='x16' type='danger'>
 				{t('Edit_Federated_User_Not_Allowed')}

diff --git a/apps/meteor/ee/client/views/admin/users/SeatsCapUsage/SeatsCapUsage.tsx b/apps/meteor/ee/client/views/admin/users/SeatsCapUsage/SeatsCapUsage.tsx
@@ -25,7 +25,7 @@ const SeatsCapUsage = ({ limit, members }: SeatsCapUsageProps): ReactElement =>
 				fontScale='c1'
 				mb='x8'
 			>
-				<div>{t('Seats_Available', { seatsLeft })}</div>
+				<div role='status'>{t('Seats_Available', { seatsLeft })}</div>
 				<Box color={reachedLimit ? 'on-danger' : 'hint'}>{`${members}/${limit}`}</Box>
 			</Box>
 			<ProgressBar percentage={percentage} variant={closeToLimit ? 'danger' : 'success'} />

diff --git a/apps/meteor/server/lib/resetUserE2EKey.ts b/apps/meteor/server/lib/resetUserE2EKey.ts
@@ -1,6 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { TAPi18n } from 'meteor/rocketchat:tap-i18n';
 import type { IUser } from '@rocket.chat/core-typings';
+import { isUserFederated } from '@rocket.chat/core-typings';
 
 import { Users, Subscriptions } from '../../app/models/server';
 import { settings } from '../../app/settings/server';
@@ -61,6 +62,11 @@ export function resetUserE2EEncriptionKey(uid: string, notifyUser: boolean): boo
 		sendResetNotitification(uid);
 	}
 
+	const user = Users.findOneById(uid, { fields: { federated: 1, username: 1 } });
+	if (isUserFederated(user)) {
+		throw new Meteor.Error('error-not-allowed', 'Federated Users cant have e2e encryption', { function: 'resetUserE2EEncriptionKey' });
+	}
+
 	Users.resetE2EKey(uid);
 	Subscriptions.resetUserE2EKey(uid);