Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Disabling admin actions for federated users #27251

Merged
merged 17 commits into from Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
16297ce
fix: disabling admin actions + do not count only federated and remote…
MarcosSpessatto Nov 11, 2022
d49a9c4
fix: consider as federated users only remote users
MarcosSpessatto Nov 11, 2022
db3dc72
chore: fix lint
MarcosSpessatto Nov 11, 2022
d8ccfbc
fix: make sure everything is being validated on BE as well
MarcosSpessatto Nov 14, 2022
cb57210
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Nov 30, 2022
5386f4c
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Dec 14, 2022
1a42dac
fix: removing forgotten log
MarcosSpessatto Dec 15, 2022
27e96c6
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Jan 4, 2023
09f1232
fix: apply suggestions from review
MarcosSpessatto Jan 4, 2023
3a7ab24
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Jan 5, 2023
c96e5d6
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Jan 16, 2023
8bd2a43
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Jan 25, 2023
380e309
fix: apply suggestions from review (WIP)
MarcosSpessatto Jan 25, 2023
f6a6e2e
fix: apply suggestions from review
MarcosSpessatto Jan 25, 2023
9248f9a
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Feb 5, 2023
f9b42de
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Feb 6, 2023
3a6a733
Merge branch 'feat/federation-feat-2' into fix/federation-users-admin
MarcosSpessatto Feb 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions 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';
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 4 additions & 7 deletions 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';

Expand All @@ -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');
MarcosSpessatto marked this conversation as resolved.
Show resolved Hide resolved
}

const subscribedRooms = getSubscribedRoomsForUserWithDetails(userId);
Expand Down
7 changes: 7 additions & 0 deletions apps/meteor/app/lib/server/functions/setUserActiveStatus.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion 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';

Expand All @@ -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);
Expand Down
20 changes: 10 additions & 10 deletions apps/meteor/client/views/admin/users/AdminUserInfoActions.tsx
Expand Up @@ -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;
Expand All @@ -25,7 +25,7 @@ type AdminUserInfoActionsProps = {
const AdminUserInfoActions = ({
username,
userId,
isAFederatedUser,
isFederatedUser,
isActive,
isAdmin,
onChange,
Expand Down Expand Up @@ -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,
Expand All @@ -97,7 +97,7 @@ const AdminUserInfoActions = ({
deleteUserAction,
resetE2EKeyAction,
resetTOTPAction,
isAFederatedUser,
isFederatedUser,
],
);

Expand Down
@@ -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';
Expand Down Expand Up @@ -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}
/>
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/views/admin/users/EditUserWithData.tsx
Expand Up @@ -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')}
Expand Down
Expand Up @@ -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'} />
Expand Down
6 changes: 6 additions & 0 deletions 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';
Expand Down Expand Up @@ -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);

Expand Down