From c6f743d7ad9f08baba5147effa45e0a50cd4c24c 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 2487dd83785..68bc9cdaf16 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -476,6 +476,21 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa 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'); } @@ -1716,6 +1731,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 87fe6e9015df0a2eb8bee41cfa3f7ce50875d491 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 b048030b067..c25b959051d 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 4887d285b7aa279395e63692bbc97bd5f1ecd573 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 c25b959051d..a1c7cc19268 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 e0551c07503fea5db190678512dfa17eee23a692 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 a1c7cc19268..beefe42e699 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 f2b3f29727e471d6dc52947a098ccff04bccc8cb 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 b0315fe6e04..8f1af5baf34 100644 --- a/lib/Service/RoomService.php +++ b/lib/Service/RoomService.php @@ -194,6 +194,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; }