From 873fa37ff86b7c9476e175a11c7addf9d8d1e9da 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 dba10ec3b65..b83c4cbf152 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -517,6 +517,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'); } @@ -1791,6 +1806,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 7f2b6b8e3b3901ac961e02ddbe45644a52aea281 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 be911f50d4c..7ea5ca3ea42 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -250,6 +250,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(); } @@ -272,6 +278,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 7a75ceb4fe017d341982085d931fd17230942f07 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 7ea5ca3ea42..9fde9e96459 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -198,13 +198,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) { @@ -232,6 +226,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 9b40fc60eaaf35b47530d8211c3c671b85aed32e 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 9fde9e96459..93d8f52b884 100644 --- a/lib/Model/AttendeeMapper.php +++ b/lib/Model/AttendeeMapper.php @@ -198,12 +198,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 { @@ -216,6 +216,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 0f65788892b1952b80bb1332f9592d938261b5a7 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 d538a3f685c..443ca03f442 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -190,6 +190,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; }