From fded42f8ad597107793e805ea6258490703929a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 17 Aug 2024 19:43:26 +0200 Subject: [PATCH] fix: Propagate permissions to new federated conversations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Besides propagating the permissions to federated servers when modified the existing permissions need to be set when creating the federated conversation (or if a federated user is added again to the conversation when all the previous federated users left it already). Signed-off-by: Daniel Calviño Sánchez --- lib/Federation/BackendNotifier.php | 2 + .../CloudFederationProviderTalk.php | 3 +- lib/Federation/FederationManager.php | 10 +++++ .../features/federation/permissions.feature | 37 +++++++++++++++++++ tests/php/Federation/FederationTest.php | 4 +- 5 files changed, 54 insertions(+), 2 deletions(-) diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php index cb2bd081531..5900855b6de 100644 --- a/lib/Federation/BackendNotifier.php +++ b/lib/Federation/BackendNotifier.php @@ -69,6 +69,7 @@ public function sendRemoteShare( $roomName = $room->getName(); $roomType = $room->getType(); $roomToken = $room->getToken(); + $roomDefaultPermissions = $room->getDefaultPermissions(); try { $this->restrictionValidator->isAllowedToInvite($sharedBy, $invitedCloudId); @@ -101,6 +102,7 @@ public function sendRemoteShare( $protocol['invitedCloudId'] = $invitedCloudId->getId(); $protocol['roomName'] = $roomName; $protocol['roomType'] = $roomType; + $protocol['roomDefaultPermissions'] = $roomDefaultPermissions; $protocol['name'] = FederationManager::TALK_PROTOCOL_NAME; $share->setProtocol($protocol); diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index 72c0811316f..5fd5b6b50c0 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -125,6 +125,7 @@ public function shareReceived(ICloudFederationShare $share): string { $remoteId = $share->getProviderId(); $roomToken = $share->getResourceName(); $roomName = $share->getProtocol()['roomName']; + $roomDefaultPermissions = $share->getProtocol()['roomDefaultPermissions'] ?? Attendee::PERMISSIONS_DEFAULT; if (isset($share->getProtocol()['invitedCloudId'])) { $localCloudId = $share->getProtocol()['invitedCloudId']; } else { @@ -173,7 +174,7 @@ public function shareReceived(ICloudFederationShare $share): string { throw new ProviderCouldNotAddShareException('User does not exist', '', Http::STATUS_BAD_REQUEST); } - $invite = $this->federationManager->addRemoteRoom($shareWithUser, (int) $remoteId, $roomType, $roomName, $roomToken, $remote, $shareSecret, $sharedByFederatedId, $sharedByDisplayName, $localCloudId); + $invite = $this->federationManager->addRemoteRoom($shareWithUser, (int) $remoteId, $roomType, $roomName, $roomDefaultPermissions, $roomToken, $remote, $shareSecret, $sharedByFederatedId, $sharedByDisplayName, $localCloudId); $this->notifyAboutNewShare($shareWithUser, (string) $invite->getId(), $sharedByFederatedId, $sharedByDisplayName, $roomName, $roomToken, $remote); return (string) $invite->getId(); diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php index 60208082891..e350897498f 100644 --- a/lib/Federation/FederationManager.php +++ b/lib/Federation/FederationManager.php @@ -19,6 +19,7 @@ use OCA\Talk\Participant; use OCA\Talk\Room; use OCA\Talk\Service\ParticipantService; +use OCA\Talk\Service\RoomService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\Federation\Exceptions\ProviderCouldNotAddShareException; @@ -50,6 +51,7 @@ class FederationManager { public function __construct( private Manager $manager, private ParticipantService $participantService, + private RoomService $roomService, private InvitationMapper $invitationMapper, private BackendNotifier $backendNotifier, private IManager $notificationManager, @@ -75,6 +77,7 @@ public function addRemoteRoom( int $remoteAttendeeId, int $roomType, string $roomName, + int $roomDefaultPermissions, string $remoteToken, string $remoteServerUrl, #[SensitiveParameter] @@ -91,6 +94,13 @@ public function addRemoteRoom( $room = $this->manager->createRemoteRoom($roomType, $roomName, $remoteToken, $remoteServerUrl); } + // Only update the room permissions if there are no participants in the + // remote room. Otherwise, the room permissions would be up to date + // already due to the notifications about room permission changes. + if (!$this->participantService->getNumberOfActors($room)) { + $this->roomService->setDefaultPermissions($room, $roomDefaultPermissions); + } + if ($couldHaveInviteWithOtherCasing) { try { $this->invitationMapper->getInvitationForUserByLocalRoom($room, $user->getUID(), true); diff --git a/tests/integration/features/federation/permissions.feature b/tests/integration/features/federation/permissions.feature index 5ee60fdce5e..521c074b295 100644 --- a/tests/integration/features/federation/permissions.feature +++ b/tests/integration/features/federation/permissions.feature @@ -65,6 +65,43 @@ Feature: federation/permissions | defaultPermissions | attendeePermissions | permissions | | CLM | D | CLM | + Scenario: set default permissions before inviting federated user + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room name | + When user "participant1" sets default permissions for room "room" to "M" with 200 (v4) + And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4) + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room name | 2 | LOCAL | room | + Then user "participant2" is participant of room "LOCAL::room" (v4) + | defaultPermissions | attendeePermissions | permissions | + | CM | D | CM | + + Scenario: set default permissions before inviting federated user again + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room name | + And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4) + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" declines invite to room "room" of server "LOCAL" with 200 (v1) + When user "participant1" sets default permissions for room "room" to "M" with 200 (v4) + And user "participant1" adds federated_user "participant2" to room "room" with 200 (v4) + And user "participant2" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant2" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room name | 2 | LOCAL | room | + Then user "participant2" is participant of room "LOCAL::room" (v4) + | defaultPermissions | attendeePermissions | permissions | + | CM | D | CM | + Scenario: set participant permissions after setting conversation permissions and then invite another federated user Given user "participant3" exists And user "participant1" creates room "room" (v4) diff --git a/tests/php/Federation/FederationTest.php b/tests/php/Federation/FederationTest.php index 936f19c3b6f..94c465503e2 100644 --- a/tests/php/Federation/FederationTest.php +++ b/tests/php/Federation/FederationTest.php @@ -256,6 +256,7 @@ public function testReceiveRemoteShare(): void { $shareType = 'user'; $roomType = Room::TYPE_GROUP; $roomName = 'Room name'; + $roomDefaultPermissions = Attendee::PERMISSIONS_CUSTOM | Attendee::PERMISSIONS_CHAT; $shareWithUser = $this->createMock(IUser::class); $shareWithUserID = '10'; @@ -277,6 +278,7 @@ public function testReceiveRemoteShare(): void { 'name' => 'nctalk', 'roomType' => $roomType, 'roomName' => $roomName, + 'roomDefaultPermissions' => $roomDefaultPermissions, 'options' => [ 'sharedSecret' => $token, ], @@ -288,7 +290,7 @@ public function testReceiveRemoteShare(): void { // Test receiving federation expectations $this->federationManager->expects($this->once()) ->method('addRemoteRoom') - ->with($shareWithUser, $providerId, $roomType, $roomName, $name, $remote, $token) + ->with($shareWithUser, $providerId, $roomType, $roomName, $roomDefaultPermissions, $name, $remote, $token) ->willReturn($invite); $this->config->method('isFederationEnabled')