Skip to content

Commit

Permalink
Regression: Do not allow non-owners edit rooms or add/remove users on…
Browse files Browse the repository at this point in the history
… federated rooms (#26263)

* fix: do not allow non-owners edit or add users on federated rooms

* fix: do not allow non-owners to remove users from federated rooms

* fix: apply suggestion from review
  • Loading branch information
MarcosSpessatto authored and ggazzo committed Jul 18, 2022
1 parent 3ba18af commit cac9e18
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 14 deletions.
9 changes: 8 additions & 1 deletion apps/meteor/app/federation-v2/client/Federation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { IRoom, IUser, ValueOf } from '@rocket.chat/core-typings';
import { RoomType } from '@rocket.chat/apps-engine/definition/rooms';
import { IRoom, ValueOf } from '@rocket.chat/core-typings';

import { RoomMemberActions } from '../../../definition/IRoomTypeConfig';

Expand All @@ -15,3 +15,10 @@ export const actionAllowed = (room: Partial<IRoom>, action: ValueOf<typeof RoomM
? false
: allowedActionsInFederatedRooms.includes(action);
};

export const isEditableByTheUser = (user: IUser | undefined, room: IRoom | undefined): boolean => {
if (!user || !room) {
return false;
}
return user._id === room.u?._id;
};
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { isRoomFederated } from '@rocket.chat/core-typings';
import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
import {
useSetModal,
Expand All @@ -9,9 +10,11 @@ import {
useEndpoint,
useMethod,
useTranslation,
useUser,
} from '@rocket.chat/ui-contexts';
import React from 'react';

import * as Federation from '../../../../../../app/federation-v2/client/Federation';
import { RoomManager } from '../../../../../../app/ui-utils/client/lib/RoomManager';
import { UiTextContext } from '../../../../../../definition/IRoomTypeConfig';
import GenericModal from '../../../../../components/GenericModal';
Expand All @@ -37,6 +40,7 @@ const retentionPolicyAppliesTo = {
const RoomInfoWithData = ({ rid, openEditing, onClickBack, onEnterRoom, resetState }) => {
const onClickClose = useTabBarClose();
const t = useTranslation();
const user = useUser();

const room = useUserRoom(rid);
room.type = room.t;
Expand Down Expand Up @@ -68,9 +72,13 @@ const RoomInfoWithData = ({ rid, openEditing, onClickBack, onEnterRoom, resetSta
t('Success'),
);

const canDelete = usePermission(type === 'c' ? 'delete-c' : 'delete-p', rid);
const canEdit = usePermission('edit-room', rid);
const canConvertRoomToTeam = usePermission('create-team');
const isFederated = isRoomFederated(room);
const hasPermissionToDelete = usePermission(type === 'c' ? 'delete-c' : 'delete-p', rid);
const hasPermissionToEdit = usePermission('edit-room', rid);
const hasPermissionToConvertRoomToTeam = usePermission('create-team');
const canDelete = isFederated ? Federation.isEditableByTheUser(user, room) && hasPermissionToDelete : hasPermissionToDelete;
const canEdit = isFederated ? Federation.isEditableByTheUser(user, room) && hasPermissionToEdit : hasPermissionToEdit;
const canConvertRoomToTeam = isFederated ? false : hasPermissionToConvertRoomToTeam;
const canLeave = usePermission(type === 'c' ? 'leave-c' : 'leave-p') && room.cl !== false && joined;

const handleDelete = useMutableCallback(() => {
Expand Down Expand Up @@ -198,7 +206,7 @@ const RoomInfoWithData = ({ rid, openEditing, onClickBack, onEnterRoom, resetSta
onClickDelete={canDelete && handleDelete}
onClickLeave={canLeave && handleLeave}
onClickHide={joined && handleHide}
onClickMoveToTeam={!room.teamId && !prid && canEdit && onMoveToTeam}
onClickMoveToTeam={!isRoomFederated(room) && !room.teamId && !prid && canEdit && onMoveToTeam}
onClickConvertToTeam={allowConvertToTeam && onConvertToTeam}
onClickEnterRoom={onEnterRoom && onClickEnterRoom}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { isRoomFederated } from '@rocket.chat/core-typings';
import { useMutableCallback, useDebouncedValue, useLocalStorage } from '@rocket.chat/fuselage-hooks';
import { useUserRoom, useAtLeastOnePermission } from '@rocket.chat/ui-contexts';
import { useUserRoom, useAtLeastOnePermission, useUser } from '@rocket.chat/ui-contexts';
import React, { useCallback, useMemo, useState } from 'react';

import * as Federation from '../../../../../../app/federation-v2/client/Federation';
import { useRecordList } from '../../../../../hooks/lists/useRecordList';
import { AsyncStatePhase } from '../../../../../hooks/useAsyncState';
import { useMembersList } from '../../../../hooks/useMembersList';
Expand All @@ -17,6 +19,7 @@ const RoomMembersWithData = ({ rid }) => {
const room = useUserRoom(rid);
const isTeam = room.teamMain;
const isDirect = room.t === 'd';
const user = useUser();

room.type = room.t;
room.rid = rid;
Expand All @@ -32,10 +35,13 @@ const RoomMembersWithData = ({ rid }) => {

const { phase, items, itemCount: total } = useRecordList(membersList);

const canAddUsers = useAtLeastOnePermission(
const hasPermissionToAddUsers = useAtLeastOnePermission(
useMemo(() => [room.t === 'p' ? 'add-user-to-any-p-room' : 'add-user-to-any-c-room', 'add-user-to-joined-room'], [room.t]),
rid,
);
const canAddUsers = isRoomFederated(room)
? Federation.isEditableByTheUser(user, room) && hasPermissionToAddUsers
: hasPermissionToAddUsers;

const handleTextChange = useCallback((event) => {
setText(event.currentTarget.value);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { IRoom, IUser } from '@rocket.chat/core-typings';
import { IRoom, isRoomFederated, IUser } from '@rocket.chat/core-typings';
import { Box, Icon } from '@rocket.chat/fuselage';
import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
import { escapeHTML } from '@rocket.chat/string-helpers';
import { usePermission, useSetModal, useTranslation, useUserRoom } from '@rocket.chat/ui-contexts';
import { usePermission, useSetModal, useTranslation, useUser, useUserRoom } from '@rocket.chat/ui-contexts';
import React, { useMemo } from 'react';

import * as Federation from '../../../../../../app/federation-v2/client/Federation';
import GenericModal from '../../../../../components/GenericModal';
import { useEndpointActionExperimental } from '../../../../../hooks/useEndpointActionExperimental';
import { roomCoordinator } from '../../../../../lib/rooms/roomCoordinator';
Expand All @@ -16,17 +17,21 @@ import { getRoomDirectives } from '../../../lib/getRoomDirectives';
export const useRemoveUserAction = (user: Pick<IUser, '_id' | 'username'>, rid: IRoom['_id'], reload?: () => void): Action | undefined => {
const t = useTranslation();
const room = useUserRoom(rid);
const currentUser = useUser();
const { _id: uid } = user;

const userCanRemove = usePermission('remove-user', rid);
const setModal = useSetModal();
const closeModal = useMutableCallback(() => setModal(null));
const roomName = room?.t && escapeHTML(roomCoordinator.getRoomName(room.t, room));

if (!room) {
throw Error('Room not provided');
}

const hasPermissionToRemove = usePermission('remove-user', rid);
const userCanRemove = isRoomFederated(room)
? Federation.isEditableByTheUser(currentUser || undefined, room) && hasPermissionToRemove
: hasPermissionToRemove;
const setModal = useSetModal();
const closeModal = useMutableCallback(() => setModal(null));
const roomName = room?.t && escapeHTML(roomCoordinator.getRoomName(room.t, room));

const endpointPrefix = room.t === 'p' ? '/v1/groups' : '/v1/channels';
const { roomCanRemove } = getRoomDirectives(room);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,18 @@ describe('Federation[Client] - Federation', () => {
expect(Federation.actionAllowed({ t: 'd' } as any, RoomMemberActions.INVITE)).to.be.true;
});
});

describe('#isEditableByTheUser()', () => {
it('should return false if the user is null', () => {
expect(Federation.isEditableByTheUser(undefined, { u: { _id: 'id' } } as any)).to.be.false;
});

it('should return true if current user is the room owner', () => {
expect(Federation.isEditableByTheUser({ _id: 'id' } as any, { u: { _id: 'id' } } as any)).to.be.true;
});

it('should return false if current user is NOT the room owner', () => {
expect(Federation.isEditableByTheUser({ _id: 'differentId' } as any, { u: { _id: 'id' } } as any)).to.be.false;
});
});
});

0 comments on commit cac9e18

Please sign in to comment.