From cac9e182617c3c6aea97be3318368589b4a08608 Mon Sep 17 00:00:00 2001 From: Marcos Spessatto Defendi Date: Mon, 18 Jul 2022 08:58:28 -0300 Subject: [PATCH] Regression: Do not allow non-owners edit rooms or add/remove users on 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 --- .../app/federation-v2/client/Federation.ts | 9 ++++++++- .../Info/RoomInfo/RoomInfoWithData.js | 16 ++++++++++++---- .../RoomMembers/List/RoomMembersWithData.js | 10 ++++++++-- .../actions/useRemoveUserAction.tsx | 19 ++++++++++++------- .../client/unit/Federation.spec.ts | 14 ++++++++++++++ 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/apps/meteor/app/federation-v2/client/Federation.ts b/apps/meteor/app/federation-v2/client/Federation.ts index 727916391268e..552c3a47623bb 100644 --- a/apps/meteor/app/federation-v2/client/Federation.ts +++ b/apps/meteor/app/federation-v2/client/Federation.ts @@ -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'; @@ -15,3 +15,10 @@ export const actionAllowed = (room: Partial, action: ValueOf { + if (!user || !room) { + return false; + } + return user._id === room.u?._id; +}; diff --git a/apps/meteor/client/views/room/contextualBar/Info/RoomInfo/RoomInfoWithData.js b/apps/meteor/client/views/room/contextualBar/Info/RoomInfo/RoomInfoWithData.js index 55ba60e259cf8..54fb8815d8d7f 100644 --- a/apps/meteor/client/views/room/contextualBar/Info/RoomInfo/RoomInfoWithData.js +++ b/apps/meteor/client/views/room/contextualBar/Info/RoomInfo/RoomInfoWithData.js @@ -1,3 +1,4 @@ +import { isRoomFederated } from '@rocket.chat/core-typings'; import { useMutableCallback } from '@rocket.chat/fuselage-hooks'; import { useSetModal, @@ -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'; @@ -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; @@ -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(() => { @@ -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} /> diff --git a/apps/meteor/client/views/room/contextualBar/RoomMembers/List/RoomMembersWithData.js b/apps/meteor/client/views/room/contextualBar/RoomMembers/List/RoomMembersWithData.js index 572482ce584a2..631ed693693ee 100644 --- a/apps/meteor/client/views/room/contextualBar/RoomMembers/List/RoomMembersWithData.js +++ b/apps/meteor/client/views/room/contextualBar/RoomMembers/List/RoomMembersWithData.js @@ -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'; @@ -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; @@ -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); diff --git a/apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx b/apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx index 6bc41e267db03..3f3d7033739b2 100644 --- a/apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx +++ b/apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx @@ -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'; @@ -16,17 +17,21 @@ import { getRoomDirectives } from '../../../lib/getRoomDirectives'; export const useRemoveUserAction = (user: Pick, 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); diff --git a/apps/meteor/tests/unit/app/federation-v2/client/unit/Federation.spec.ts b/apps/meteor/tests/unit/app/federation-v2/client/unit/Federation.spec.ts index 99467f6ffecd4..88ebc8abae8dc 100644 --- a/apps/meteor/tests/unit/app/federation-v2/client/unit/Federation.spec.ts +++ b/apps/meteor/tests/unit/app/federation-v2/client/unit/Federation.spec.ts @@ -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; + }); + }); });