From f1164dbe7db51a3d3e61508b57079b82486e4804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 18 Aug 2024 01:45:19 +0200 Subject: [PATCH 1/5] test: Add integration tests for adding and removing permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/FeatureContext.php | 50 +++++++++++++++++++ .../conversation-5/set-permissions.feature | 36 +++++++++++++ 2 files changed, 86 insertions(+) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index c1a8e8ea916..a2cd34047e9 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -559,6 +559,21 @@ private function assertRooms(array $rooms, TableNode $formData, bool $shouldOrde if (isset($expectedRoom['recordingConsent'])) { $data['recordingConsent'] = (int) $room['recordingConsent']; } + if (isset($expectedRoom['permissions'])) { + $data['permissions'] = $this->mapPermissionsAPIOutput($room['permissions']); + } + if (isset($expectedRoom['permissions'])) { + $data['permissions'] = $this->mapPermissionsAPIOutput($room['permissions']); + } + if (isset($expectedRoom['attendeePermissions'])) { + $data['attendeePermissions'] = $this->mapPermissionsAPIOutput($room['attendeePermissions']); + } + if (isset($expectedRoom['callPermissions'])) { + $data['callPermissions'] = $this->mapPermissionsAPIOutput($room['callPermissions']); + } + if (isset($expectedRoom['defaultPermissions'])) { + $data['defaultPermissions'] = $this->mapPermissionsAPIOutput($room['defaultPermissions']); + } if (isset($expectedRoom['participants'])) { throw new \Exception('participants key needs to be checked via participants endpoint'); } @@ -2024,6 +2039,41 @@ public function userSetsPermissionsForInRoomTo(string $user, string $participant $this->assertStatusCode($this->response, $statusCode); } + /** + * @When /^user "([^"]*)" (sets|removes|adds) permissions for all attendees in room "([^"]*)" to "([^"]*)" with (\d+) \((v4)\)$/ + * + * @param string $user + * @param string $mode + * @param string $identifier + * @param string $permissionsString + * @param int $statusCode + * @param string $apiVersion + */ + public function userSetsRemovesAddsPermissionsForAllAttendeesInRoomTo(string $user, string $method, string $identifier, string $permissionsString, int $statusCode, string $apiVersion): void { + $permissions = $this->mapPermissionsTestInput($permissionsString); + + // Convert method from step syntax to what the API expects + if ($method === 'sets') { + $method = 'set'; + } elseif ($method === 'removes') { + $method = 'remove'; + } else { + $method = 'add'; + } + + $requestParameters = [ + ['method', $method], + ['permissions', $permissions], + ]; + + $this->setCurrentUser($user); + $this->sendRequest( + 'PUT', '/apps/spreed/api/' . $apiVersion . '/room/' . self::$identifierToToken[$identifier] . '/attendees/permissions/all', + new TableNode($requestParameters) + ); + $this->assertStatusCode($this->response, $statusCode); + } + /** * @When /^user "([^"]*)" sets (call|default) permissions for room "([^"]*)" to "([^"]*)" with (\d+) \((v4)\)$/ * diff --git a/tests/integration/features/conversation-5/set-permissions.feature b/tests/integration/features/conversation-5/set-permissions.feature index 1005cda7ad2..31ac6733248 100644 --- a/tests/integration/features/conversation-5/set-permissions.feature +++ b/tests/integration/features/conversation-5/set-permissions.feature @@ -149,3 +149,39 @@ Feature: conversation-2/set-publishing-permissions | actorType | actorId | permissions | attendeePermissions | participantType | | users | owner | SJLAVPM | D | 1 | | users | invited user | CLAVPM | CLAVPM | 3 | + + Scenario: adding and removing permissions to all attendees do not change default attendee permissions + Given user "invited user2" exists + And user "owner" creates room "group room" (v4) + | roomType | 2 | + | roomName | room | + And user "owner" adds user "invited user" to room "group room" with 200 (v4) + And user "owner" adds user "invited user2" to room "group room" with 200 (v4) + And user "owner" sets call permissions for room "group room" to "LPM" with 200 (v4) + And user "owner" sets permissions for "invited user2" in room "group room" to "LVP" with 200 (v4) + When user "owner" adds permissions for all attendees in room "group room" to "JA" with 200 (v4) + And user "owner" removes permissions for all attendees in room "group room" to "SLP" with 200 (v4) + Then user "owner" sees the following attendees in room "group room" with 200 (v4) + | actorType | actorId | permissions | attendeePermissions | + | users | owner | SJLAVPM | D | + | users | invited user | CJAM | D | + | users | invited user2 | CJAV | CJAV | + And user "owner" is participant of room "group room" (v4) + | callPermissions | defaultPermissions | + | CJAM | D | + + Scenario: adding and removing permissions to all attendees customize room permissions if call permissions are not set + Given user "owner" creates room "group room" (v4) + | roomType | 2 | + | roomName | room | + And user "owner" adds user "invited user" to room "group room" with 200 (v4) + And user "owner" sets default permissions for room "group room" to "LPM" with 200 (v4) + When user "owner" adds permissions for all attendees in room "group room" to "JA" with 200 (v4) + And user "owner" removes permissions for all attendees in room "group room" to "SLP" with 200 (v4) + Then user "owner" sees the following attendees in room "group room" with 200 (v4) + | actorType | actorId | permissions | attendeePermissions | + | users | owner | SJLAVPM | D | + | users | invited user | CJAM | D | + And user "owner" is participant of room "group room" (v4) + | callPermissions | defaultPermissions | + | CJAM | CLPM | From b1fce9a4f7fba58dd74ef79541ede627093f4fc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 18 Aug 2024 01:47:09 +0200 Subject: [PATCH 2/5] fix: Fix adding permissions when attendees have default permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When adding permissions to all attendees the permissions are added to the call/room and to the attendees. Attendees with default permissions will get the effective permissions from the call/room permissions, so the permissions should be added only to those attendees that already have custom permissions. Signed-off-by: Daniel Calviño Sánchez --- lib/Model/AttendeeMapper.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/Model/AttendeeMapper.php b/lib/Model/AttendeeMapper.php index e7a44970848..5d536e623fa 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -255,6 +255,12 @@ protected function addSinglePermission(IQueryBuilder $query, int $permission): v $query->createNamedParameter($permission, IQueryBuilder::PARAM_INT) ) ); + $query->andWhere( + $query->expr()->neq( + 'permissions', + $query->createNamedParameter(Attendee::PERMISSIONS_DEFAULT, IQueryBuilder::PARAM_INT) + ) + ); $query->executeStatement(); } @@ -277,6 +283,9 @@ protected function removeSinglePermission(IQueryBuilder $query, int $permission) $query->createNamedParameter($permission, IQueryBuilder::PARAM_INT) ) ); + // Removing permissions does not need to be explicitly prevented when + // the attendee has default permissions, as in that case it will not be + // possible to remove the permissions anyway. $query->executeStatement(); } From a974742ec69cde24fbe64f129d5387421e1007bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 18 Aug 2024 01:48:17 +0200 Subject: [PATCH 3/5] refactor: Extract method to get the base query for setting permissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- lib/Model/AttendeeMapper.php | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/Model/AttendeeMapper.php b/lib/Model/AttendeeMapper.php index 5d536e623fa..9e8aeae2a28 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -203,13 +203,7 @@ public function deleteByIds(array $ids): int { } public function modifyPermissions(int $roomId, string $mode, int $newState): void { - $query = $this->db->getQueryBuilder(); - $query->update($this->getTableName()) - ->where($query->expr()->eq('room_id', $query->createNamedParameter($roomId, IQueryBuilder::PARAM_INT))) - ->andWhere($query->expr()->notIn('actor_type', $query->createNamedParameter([ - Attendee::ACTOR_CIRCLES, - Attendee::ACTOR_GROUPS, - ], IQueryBuilder::PARAM_STR_ARRAY))); + $query = $this->getModifyPermissionsBaseQuery($roomId); if ($mode === Attendee::PERMISSIONS_MODIFY_SET) { if ($newState !== Attendee::PERMISSIONS_DEFAULT) { @@ -237,6 +231,18 @@ public function modifyPermissions(int $roomId, string $mode, int $newState): voi } } + protected function getModifyPermissionsBaseQuery(int $roomId): IQueryBuilder { + $query = $this->db->getQueryBuilder(); + $query->update($this->getTableName()) + ->where($query->expr()->eq('room_id', $query->createNamedParameter($roomId, IQueryBuilder::PARAM_INT))) + ->andWhere($query->expr()->notIn('actor_type', $query->createNamedParameter([ + Attendee::ACTOR_CIRCLES, + Attendee::ACTOR_GROUPS, + ], IQueryBuilder::PARAM_STR_ARRAY))); + + return $query; + } + protected function addSinglePermission(IQueryBuilder $query, int $permission): void { $query->set('permissions', $query->func()->add( 'permissions', From 1111bd3dfa2ef829e0124ecf791908a0b5cc4482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 18 Aug 2024 01:48:48 +0200 Subject: [PATCH 4/5] fix: Fix adding and removing several attendee permissions at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When adding or removing attendee permissions the permissions are decomposed in each separate permission and added or removed individually. However, the same query object was reused, so once it was executed for the first permission found then it no longer worked as expected for the following permissions. Signed-off-by: Daniel Calviño Sánchez --- lib/Model/AttendeeMapper.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Model/AttendeeMapper.php b/lib/Model/AttendeeMapper.php index 9e8aeae2a28..8e85de7da9c 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -203,12 +203,12 @@ public function deleteByIds(array $ids): int { } public function modifyPermissions(int $roomId, string $mode, int $newState): void { - $query = $this->getModifyPermissionsBaseQuery($roomId); - if ($mode === Attendee::PERMISSIONS_MODIFY_SET) { if ($newState !== Attendee::PERMISSIONS_DEFAULT) { $newState |= Attendee::PERMISSIONS_CUSTOM; } + + $query = $this->getModifyPermissionsBaseQuery($roomId); $query->set('permissions', $query->createNamedParameter($newState, IQueryBuilder::PARAM_INT)); $query->executeStatement(); } else { @@ -221,6 +221,8 @@ public function modifyPermissions(int $roomId, string $mode, int $newState): voi Attendee::PERMISSIONS_LOBBY_IGNORE, ] as $permission) { if ($permission & $newState) { + $query = $this->getModifyPermissionsBaseQuery($roomId); + if ($mode === Attendee::PERMISSIONS_MODIFY_ADD) { $this->addSinglePermission($query, $permission); } elseif ($mode === Attendee::PERMISSIONS_MODIFY_REMOVE) { From 7d8ea6505fdd363be0c886c882c82f78c53c86c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 18 Aug 2024 01:49:05 +0200 Subject: [PATCH 5/5] fix: Fix adding or removing permissions without call permissions set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When adding or removing call permissions the default room permissions should be taken as a base if no call permission is set. Otherwise adding will cause the permissions to be set instead of added, while removing them will have no effect. Signed-off-by: Daniel Calviño Sánchez --- lib/Service/RoomService.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/Service/RoomService.php b/lib/Service/RoomService.php index 68389011049..e97b55052db 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -180,6 +180,12 @@ public function setPermissions(Room $room, string $level, string $method, int $p } elseif ($level === 'call') { $property = ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS; $oldPermissions = $room->getCallPermissions(); + + if ($oldPermissions === Attendee::PERMISSIONS_DEFAULT + && ($method === Attendee::PERMISSIONS_MODIFY_ADD + || $method === Attendee::PERMISSIONS_MODIFY_REMOVE)) { + $oldPermissions = $room->getDefaultPermissions(); + } } else { return false; }