From 883998e2443d754732cb445e62f389ee74d26786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 15 Aug 2024 07:26:06 +0200 Subject: [PATCH 1/4] fix: Propagate permission changes of federated users to their servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend gets the permission for the current participant from the room data (as guest users can not get it from the participant list). However, permission changes were not propagated to federated servers, so the frontend of federated users always assumed that the user had the default permissions. Signed-off-by: Daniel Calviño Sánchez --- lib/AppInfo/Application.php | 2 + lib/Federation/BackendNotifier.php | 34 ++++++++++ .../CloudFederationProviderTalk.php | 38 +++++++++++ lib/Federation/FederationManager.php | 1 + .../Notifier/ParticipantModifiedListener.php | 68 +++++++++++++++++++ .../features/bootstrap/FeatureContext.php | 9 +++ .../features/federation/permissions.feature | 23 +++++++ 7 files changed, 175 insertions(+) create mode 100644 lib/Federation/Proxy/TalkV1/Notifier/ParticipantModifiedListener.php create mode 100644 tests/integration/features/federation/permissions.feature diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index f145a74053b..e91901cd963 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -70,6 +70,7 @@ use OCA\Talk\Federation\Proxy\TalkV1\Notifier\BeforeRoomDeletedListener as TalkV1BeforeRoomDeletedListener; use OCA\Talk\Federation\Proxy\TalkV1\Notifier\CancelRetryOCMListener as TalkV1CancelRetryOCMListener; use OCA\Talk\Federation\Proxy\TalkV1\Notifier\MessageSentListener as TalkV1MessageSentListener; +use OCA\Talk\Federation\Proxy\TalkV1\Notifier\ParticipantModifiedListener as TalkV1ParticipantModifiedListener; use OCA\Talk\Federation\Proxy\TalkV1\Notifier\RoomModifiedListener as TalkV1RoomModifiedListener; use OCA\Talk\Files\Listener as FilesListener; use OCA\Talk\Files\TemplateLoader as FilesTemplateLoader; @@ -266,6 +267,7 @@ public function register(IRegistrationContext $context): void { // Federation listeners $context->registerEventListener(BeforeRoomDeletedEvent::class, TalkV1BeforeRoomDeletedListener::class); + $context->registerEventListener(ParticipantModifiedEvent::class, TalkV1ParticipantModifiedListener::class); $context->registerEventListener(CallEndedEvent::class, TalkV1RoomModifiedListener::class); $context->registerEventListener(CallEndedForEveryoneEvent::class, TalkV1RoomModifiedListener::class); $context->registerEventListener(CallStartedEvent::class, TalkV1RoomModifiedListener::class); diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php index 6aff638db4b..b95ce041dd5 100644 --- a/lib/Federation/BackendNotifier.php +++ b/lib/Federation/BackendNotifier.php @@ -251,6 +251,40 @@ public function sendRoomModifiedUpdate( return $this->sendUpdateToRemote($remote, $notification); } + /** + * Send information to remote participants that the participant meta info updated + * Sent from Host server to Remote participant server + */ + public function sendParticipantModifiedUpdate( + string $remoteServer, + int $localAttendeeId, + #[SensitiveParameter] + string $accessToken, + string $localToken, + string $changedProperty, + string|int $newValue, + string|int|null $oldValue, + ): ?bool { + $remote = $this->prepareRemoteUrl($remoteServer); + + $notification = $this->cloudFederationFactory->getCloudFederationNotification(); + $notification->setMessage( + FederationManager::NOTIFICATION_PARTICIPANT_MODIFIED, + FederationManager::TALK_ROOM_RESOURCE, + (string) $localAttendeeId, + [ + 'remoteServerUrl' => $this->getServerRemoteUrl(), + 'sharedSecret' => $accessToken, + 'remoteToken' => $localToken, + 'changedProperty' => $changedProperty, + 'newValue' => $newValue, + 'oldValue' => $oldValue, + ], + ); + + return $this->sendUpdateToRemote($remote, $notification); + } + /** * Send information to remote participants that "active since" was updated * Sent from Host server to Remote participant server diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index aa845ec9759..e0b89e8bdbe 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -197,6 +197,8 @@ public function notificationReceived($notificationType, $providerId, array $noti return $this->shareDeclined((int) $providerId, $notification); case FederationManager::NOTIFICATION_SHARE_UNSHARED: return $this->shareUnshared((int) $providerId, $notification); + case FederationManager::NOTIFICATION_PARTICIPANT_MODIFIED: + return $this->participantModified((int) $providerId, $notification); case FederationManager::NOTIFICATION_ROOM_MODIFIED: return $this->roomModified((int) $providerId, $notification); case FederationManager::NOTIFICATION_MESSAGE_POSTED: @@ -292,6 +294,42 @@ private function shareUnshared(int $remoteAttendeeId, array $notification): arra return []; } + /** + * @param int $remoteAttendeeId + * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, changedProperty: string, newValue: string|int, oldValue: string|int|null} $notification + * @return array + * @throws ActionNotSupportedException + * @throws AuthenticationFailedException + * @throws ShareNotFound + */ + private function participantModified(int $remoteAttendeeId, array $notification): array { + $invite = $this->getByRemoteAttendeeAndValidate($notification['remoteServerUrl'], $remoteAttendeeId, $notification['sharedSecret']); + try { + $room = $this->manager->getRoomById($invite->getLocalRoomId()); + } catch (RoomNotFoundException) { + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); + } + + // Sanity check to make sure the room is a remote room + if (!$room->isFederatedConversation()) { + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); + } + + try { + $participant = $this->participantService->getParticipant($room, $invite->getUserId()); + } catch (ParticipantNotFoundException $e) { + throw new ShareNotFound(FederationManager::OCM_RESOURCE_NOT_FOUND); + } + + if ($notification['changedProperty'] === AParticipantModifiedEvent::PROPERTY_PERMISSIONS) { + $this->participantService->updatePermissions($room, $participant, Attendee::PERMISSIONS_MODIFY_SET, $notification['newValue']); + } else { + $this->logger->debug('Update of participant property "' . $notification['changedProperty'] . '" is not handled and should not be send via federation'); + } + + return []; + } + /** * @param int $remoteAttendeeId * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, changedProperty: string, newValue: string|int|bool|null, oldValue: string|int|bool|null, callFlag?: int, dateTime?: string, timerReached?: bool, details?: array} $notification diff --git a/lib/Federation/FederationManager.php b/lib/Federation/FederationManager.php index 9e022a9a66c..60208082891 100644 --- a/lib/Federation/FederationManager.php +++ b/lib/Federation/FederationManager.php @@ -42,6 +42,7 @@ class FederationManager { public const NOTIFICATION_SHARE_ACCEPTED = 'SHARE_ACCEPTED'; public const NOTIFICATION_SHARE_DECLINED = 'SHARE_DECLINED'; public const NOTIFICATION_SHARE_UNSHARED = 'SHARE_UNSHARED'; + public const NOTIFICATION_PARTICIPANT_MODIFIED = 'PARTICIPANT_MODIFIED'; public const NOTIFICATION_ROOM_MODIFIED = 'ROOM_MODIFIED'; public const NOTIFICATION_MESSAGE_POSTED = 'MESSAGE_POSTED'; public const TOKEN_LENGTH = 64; diff --git a/lib/Federation/Proxy/TalkV1/Notifier/ParticipantModifiedListener.php b/lib/Federation/Proxy/TalkV1/Notifier/ParticipantModifiedListener.php new file mode 100644 index 00000000000..4e4e60a1d28 --- /dev/null +++ b/lib/Federation/Proxy/TalkV1/Notifier/ParticipantModifiedListener.php @@ -0,0 +1,68 @@ + + */ +class ParticipantModifiedListener implements IEventListener { + public function __construct( + protected BackendNotifier $backendNotifier, + protected ParticipantService $participantService, + protected ICloudIdManager $cloudIdManager, + ) { + } + + public function handle(Event $event): void { + if (!$event instanceof ParticipantModifiedEvent) { + return; + } + + if (!in_array($event->getProperty(), [ + AParticipantModifiedEvent::PROPERTY_PERMISSIONS, + ], true)) { + return; + } + + $participants = $this->participantService->getParticipantsByActorType($event->getRoom(), Attendee::ACTOR_FEDERATED_USERS); + foreach ($participants as $participant) { + $cloudId = $this->cloudIdManager->resolveCloudId($participant->getAttendee()->getActorId()); + + $success = $this->notifyParticipantModified($cloudId, $participant, $event); + + if ($success === null) { + $this->participantService->removeAttendee($event->getRoom(), $participant, AAttendeeRemovedEvent::REASON_LEFT); + } + } + } + + private function notifyParticipantModified(ICloudId $cloudId, Participant $participant, AParticipantModifiedEvent $event) { + return $this->backendNotifier->sendParticipantModifiedUpdate( + $cloudId->getRemote(), + $participant->getAttendee()->getId(), + $participant->getAttendee()->getAccessToken(), + $event->getRoom()->getToken(), + $event->getProperty(), + $event->getNewValue(), + $event->getOldValue(), + ); + } +} diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index c1a8e8ea916..31f66250852 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -559,6 +559,9 @@ 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['participants'])) { throw new \Exception('participants key needs to be checked via participants endpoint'); } @@ -2004,6 +2007,12 @@ public function userSetsPermissionsForInRoomTo(string $user, string $participant } elseif (strpos($participant, 'guest') === 0) { $sessionId = self::$userToSessionId[$participant]; $attendeeId = $this->getAttendeeId('guests', sha1($sessionId), $identifier, $statusCode === 200 ? $user : null); + } elseif (str_ends_with($participant, '@{$LOCAL_REMOTE_URL}') || + str_ends_with($participant, '@{$REMOTE_URL}')) { + $participant = str_replace('{$LOCAL_REMOTE_URL}', rtrim($this->localRemoteServerUrl, '/'), $participant); + $participant = str_replace('{$REMOTE_URL}', rtrim($this->remoteServerUrl, '/'), $participant); + + $attendeeId = $this->getAttendeeId('federated_users', $participant, $identifier, $statusCode === 200 ? $user : null); } else { $attendeeId = $this->getAttendeeId('users', $participant, $identifier, $statusCode === 200 ? $user : null); } diff --git a/tests/integration/features/federation/permissions.feature b/tests/integration/features/federation/permissions.feature new file mode 100644 index 00000000000..efa3efdf2e5 --- /dev/null +++ b/tests/integration/features/federation/permissions.feature @@ -0,0 +1,23 @@ +Feature: federation/permissions + + Background: + Given user "participant1" exists + And user "participant2" exists + And the following "spreed" app config is set + | federation_enabled | yes | + + Scenario: set participant permissions + 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" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room name | 2 | LOCAL | room | + When user "participant1" sets permissions for "participant2@{$LOCAL_REMOTE_URL}" in room "room" to "S" with 200 (v4) + Then user "participant2" is participant of room "LOCAL::room" (v4) + | permissions | + | CS | From 12290eac0565c6bc0dba3e2453ed39a72c05bc2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 15 Aug 2024 07:27:18 +0200 Subject: [PATCH 2/4] refactor: Include additional context in room permission change events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- docs/events.md | 6 +++ lib/Events/APermissionsModifiedEvent.php | 42 +++++++++++++++++++ lib/Events/BeforePermissionsModifiedEvent.php | 12 ++++++ lib/Events/PermissionsModifiedEvent.php | 12 ++++++ lib/Service/RoomService.php | 6 ++- 5 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 lib/Events/APermissionsModifiedEvent.php create mode 100644 lib/Events/BeforePermissionsModifiedEvent.php create mode 100644 lib/Events/PermissionsModifiedEvent.php diff --git a/docs/events.md b/docs/events.md index e09e956b1ea..06a820e58bc 100644 --- a/docs/events.md +++ b/docs/events.md @@ -34,6 +34,12 @@ See the general [Nextcloud Developers - Events](https://docs.nextcloud.com/serve * After event: `OCA\Talk\Events\LobbyModifiedEvent` * Since: 18.0.0 +### Permissions modified + +* Before event: `OCA\Talk\Events\BeforePermissionsModifiedEvent` +* After event: `OCA\Talk\Events\PermissionsModifiedEvent` +* Since: 20.0.0 + ### Call started * Before event: `OCA\Talk\Events\BeforeCallStartedEvent` diff --git a/lib/Events/APermissionsModifiedEvent.php b/lib/Events/APermissionsModifiedEvent.php new file mode 100644 index 00000000000..3362292f9fe --- /dev/null +++ b/lib/Events/APermissionsModifiedEvent.php @@ -0,0 +1,42 @@ +method; + } + + public function getPermissions(): int { + return $this->permissions; + } + + public function resetCustomPermissions(): bool { + return $this->resetCustomPermissions; + } +} diff --git a/lib/Events/BeforePermissionsModifiedEvent.php b/lib/Events/BeforePermissionsModifiedEvent.php new file mode 100644 index 00000000000..2ca417278f0 --- /dev/null +++ b/lib/Events/BeforePermissionsModifiedEvent.php @@ -0,0 +1,12 @@ +dispatcher->dispatchTyped($event); if ($resetCustomPermissions) { @@ -222,7 +224,7 @@ public function setPermissions(Room $room, string $level, string $method, int $p $room->setCallPermissions($newPermissions); } - $event = new RoomModifiedEvent($room, $property, $newPermissions, $oldPermissions); + $event = new PermissionsModifiedEvent($room, $property, $newPermissions, $oldPermissions, $method, $permissions, $resetCustomPermissions); $this->dispatcher->dispatchTyped($event); return true; From ceb9aefb740f625123095b4f9d81c48592a4b2b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 15 Aug 2024 08:37:27 +0200 Subject: [PATCH 3/4] fix: Propagate permission changes of conversations to federated servers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a participant does not have custom permissions the permissions are got from the call or conversation, so they need to be propagated as well to the federated servers to correctly calculate the participant permissions. Signed-off-by: Daniel Calviño Sánchez --- lib/AppInfo/Application.php | 2 + lib/Federation/BackendNotifier.php | 40 ++++++++ .../CloudFederationProviderTalk.php | 6 +- .../TalkV1/Notifier/RoomModifiedListener.php | 22 +++++ .../features/bootstrap/FeatureContext.php | 35 +++++++ .../features/federation/permissions.feature | 96 +++++++++++++++++++ 6 files changed, 200 insertions(+), 1 deletion(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index e91901cd963..cebd55379c7 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -58,6 +58,7 @@ use OCA\Talk\Events\LobbyModifiedEvent; use OCA\Talk\Events\MessageParseEvent; use OCA\Talk\Events\ParticipantModifiedEvent; +use OCA\Talk\Events\PermissionsModifiedEvent; use OCA\Talk\Events\RoomCreatedEvent; use OCA\Talk\Events\RoomDeletedEvent; use OCA\Talk\Events\RoomModifiedEvent; @@ -272,6 +273,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(CallEndedForEveryoneEvent::class, TalkV1RoomModifiedListener::class); $context->registerEventListener(CallStartedEvent::class, TalkV1RoomModifiedListener::class); $context->registerEventListener(LobbyModifiedEvent::class, TalkV1RoomModifiedListener::class); + $context->registerEventListener(PermissionsModifiedEvent::class, TalkV1RoomModifiedListener::class); $context->registerEventListener(RoomModifiedEvent::class, TalkV1RoomModifiedListener::class); $context->registerEventListener(ChatMessageSentEvent::class, TalkV1MessageSentListener::class); $context->registerEventListener(SystemMessageSentEvent::class, TalkV1MessageSentListener::class); diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php index b95ce041dd5..1bfc6d115e2 100644 --- a/lib/Federation/BackendNotifier.php +++ b/lib/Federation/BackendNotifier.php @@ -401,6 +401,46 @@ public function sendRoomModifiedLobbyUpdate( return $this->sendUpdateToRemote($remote, $notification); } + /** + * Send information to remote participants that the room permissions were updated + * Sent from Host server to Remote participant server + */ + public function sendRoomModifiedPermissionsUpdate( + string $remoteServer, + int $localAttendeeId, + #[SensitiveParameter] + string $accessToken, + string $localToken, + string $changedProperty, + int $newValue, + int $oldValue, + string $method, + int $permissions, + bool $resetCustomPermissions, + ): ?bool { + $remote = $this->prepareRemoteUrl($remoteServer); + + $notification = $this->cloudFederationFactory->getCloudFederationNotification(); + $notification->setMessage( + FederationManager::NOTIFICATION_ROOM_MODIFIED, + FederationManager::TALK_ROOM_RESOURCE, + (string) $localAttendeeId, + [ + 'remoteServerUrl' => $this->getServerRemoteUrl(), + 'sharedSecret' => $accessToken, + 'remoteToken' => $localToken, + 'changedProperty' => $changedProperty, + 'newValue' => $newValue, + 'oldValue' => $oldValue, + 'method' => $method, + 'permissions' => $permissions, + 'resetCustomPermissions' => $resetCustomPermissions, + ], + ); + + return $this->sendUpdateToRemote($remote, $notification); + } + /** * Send information to remote participants that a message was posted * Sent from Host server to Remote participant server diff --git a/lib/Federation/CloudFederationProviderTalk.php b/lib/Federation/CloudFederationProviderTalk.php index e0b89e8bdbe..656334d95b9 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -332,7 +332,7 @@ private function participantModified(int $remoteAttendeeId, array $notification) /** * @param int $remoteAttendeeId - * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, changedProperty: string, newValue: string|int|bool|null, oldValue: string|int|bool|null, callFlag?: int, dateTime?: string, timerReached?: bool, details?: array} $notification + * @param array{remoteServerUrl: string, sharedSecret: string, remoteToken: string, changedProperty: string, newValue: string|int|bool|null, oldValue: string|int|bool|null, callFlag?: int, dateTime?: string, timerReached?: bool, method?: string, permissions?: int, resetCustomPermissions?: bool, details?: array} $notification * @return array * @throws ActionNotSupportedException * @throws AuthenticationFailedException @@ -360,6 +360,10 @@ private function roomModified(int $remoteAttendeeId, array $notification): array } } elseif ($notification['changedProperty'] === ARoomModifiedEvent::PROPERTY_AVATAR) { $this->roomService->setAvatar($room, $notification['newValue']); + } elseif ($notification['changedProperty'] === ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS) { + $this->roomService->setPermissions($room, 'call', $notification['method'], $notification['permissions'], $notification['resetCustomPermissions']); + } elseif ($notification['changedProperty'] === ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS) { + $this->roomService->setPermissions($room, 'default', $notification['method'], $notification['permissions'], $notification['resetCustomPermissions']); } elseif ($notification['changedProperty'] === ARoomModifiedEvent::PROPERTY_DESCRIPTION) { $this->roomService->setDescription($room, $notification['newValue']); } elseif ($notification['changedProperty'] === ARoomModifiedEvent::PROPERTY_IN_CALL) { diff --git a/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php b/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php index 90d7dc5024f..b7f0b0aad44 100644 --- a/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php +++ b/lib/Federation/Proxy/TalkV1/Notifier/RoomModifiedListener.php @@ -11,11 +11,13 @@ use OCA\Talk\Events\AAttendeeRemovedEvent; use OCA\Talk\Events\ALobbyModifiedEvent; use OCA\Talk\Events\AParticipantModifiedEvent; +use OCA\Talk\Events\APermissionsModifiedEvent; use OCA\Talk\Events\ARoomModifiedEvent; use OCA\Talk\Events\CallEndedEvent; use OCA\Talk\Events\CallEndedForEveryoneEvent; use OCA\Talk\Events\CallStartedEvent; use OCA\Talk\Events\LobbyModifiedEvent; +use OCA\Talk\Events\PermissionsModifiedEvent; use OCA\Talk\Events\RoomModifiedEvent; use OCA\Talk\Federation\BackendNotifier; use OCA\Talk\Model\Attendee; @@ -42,6 +44,7 @@ public function handle(Event $event): void { && !$event instanceof CallEndedEvent && !$event instanceof CallEndedForEveryoneEvent && !$event instanceof LobbyModifiedEvent + && !$event instanceof PermissionsModifiedEvent && !$event instanceof RoomModifiedEvent) { return; } @@ -49,6 +52,8 @@ public function handle(Event $event): void { if (!in_array($event->getProperty(), [ ARoomModifiedEvent::PROPERTY_ACTIVE_SINCE, ARoomModifiedEvent::PROPERTY_AVATAR, + ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS, + ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS, ARoomModifiedEvent::PROPERTY_DESCRIPTION, ARoomModifiedEvent::PROPERTY_IN_CALL, ARoomModifiedEvent::PROPERTY_LOBBY, @@ -70,6 +75,8 @@ public function handle(Event $event): void { $success = $this->notifyCallEnded($cloudId, $participant, $event); } elseif ($event instanceof ALobbyModifiedEvent) { $success = $this->notifyLobbyModified($cloudId, $participant, $event); + } elseif ($event instanceof APermissionsModifiedEvent) { + $success = $this->notifyPermissionsModified($cloudId, $participant, $event); } else { $success = $this->notifyRoomModified($cloudId, $participant, $event); } @@ -130,6 +137,21 @@ private function notifyLobbyModified(ICloudId $cloudId, Participant $participant ); } + private function notifyPermissionsModified(ICloudId $cloudId, Participant $participant, APermissionsModifiedEvent $event) { + return $this->backendNotifier->sendRoomModifiedPermissionsUpdate( + $cloudId->getRemote(), + $participant->getAttendee()->getId(), + $participant->getAttendee()->getAccessToken(), + $event->getRoom()->getToken(), + $event->getProperty(), + $event->getNewValue(), + $event->getOldValue(), + $event->getMethod(), + $event->getPermissions(), + $event->resetCustomPermissions(), + ); + } + private function notifyRoomModified(ICloudId $cloudId, Participant $participant, ARoomModifiedEvent $event) { return $this->backendNotifier->sendRoomModifiedUpdate( $cloudId->getRemote(), diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 31f66250852..63406d99c6c 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -2033,6 +2033,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/federation/permissions.feature b/tests/integration/features/federation/permissions.feature index efa3efdf2e5..61a925ebbe3 100644 --- a/tests/integration/features/federation/permissions.feature +++ b/tests/integration/features/federation/permissions.feature @@ -21,3 +21,99 @@ Feature: federation/permissions Then user "participant2" is participant of room "LOCAL::room" (v4) | permissions | | CS | + + Scenario: change permissions for all attendees + 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" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room name | 2 | LOCAL | room | + When user "participant1" sets permissions for all attendees in room "room" to "SM" with 200 (v4) + And user "participant1" removes permissions for all attendees in room "room" to "S" with 200 (v4) + And user "participant1" adds permissions for all attendees in room "room" to "L" with 200 (v4) + Then user "participant2" is participant of room "LOCAL::room" (v4) + | permissions | + | CLM | + + Scenario: change permissions for all attendees before federated user accepts invitation + 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 | + When user "participant1" sets permissions for all attendees in room "room" to "SM" with 200 (v4) + And user "participant1" removes permissions for all attendees in room "room" to "S" with 200 (v4) + And user "participant1" adds permissions for all attendees in room "room" to "L" with 200 (v4) + 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) + | permissions | + | CLM | + + Scenario: set conversation permissions + 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" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room name | 2 | LOCAL | room | + When user "participant1" sets default permissions for room "room" to "M" with 200 (v4) + Then user "participant2" is participant of room "LOCAL::room" (v4) + | permissions | + | CM | + + Scenario: set conversation permissions before federated user accepts invitation + 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 | + When user "participant1" sets default permissions for room "room" to "M" with 200 (v4) + 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) + | permissions | + | 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) + | 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" accepts invite to room "room" of server "LOCAL" with 200 (v1) + | id | name | type | remoteServer | remoteToken | + | LOCAL::room | room name | 2 | LOCAL | room | + And user "participant1" sets default permissions for room "room" to "AVP" with 200 (v4) + And user "participant1" sets permissions for "participant2@{$LOCAL_REMOTE_URL}" in room "room" to "S" with 200 (v4) + When user "participant1" adds federated_user "participant3" to room "room" with 200 (v4) + And user "participant3" has the following invitations (v1) + | remoteServerUrl | remoteToken | state | inviterCloudId | inviterDisplayName | + | LOCAL | room | 0 | participant1@http://localhost:8080 | participant1-displayname | + And user "participant3" 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) + | permissions | + | CS | + And user "participant3" is participant of room "LOCAL::room" (v4) + | permissions | + | CAVP | From f0d80368eef0789e9d56b1aa95c8938309da8322 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 4/4] 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 | 4 ++ .../CloudFederationProviderTalk.php | 4 +- lib/Federation/FederationManager.php | 12 ++++ .../features/federation/permissions.feature | 55 +++++++++++++++++++ tests/php/Federation/FederationTest.php | 6 +- 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php index 1bfc6d115e2..ef84f31de63 100644 --- a/lib/Federation/BackendNotifier.php +++ b/lib/Federation/BackendNotifier.php @@ -69,6 +69,8 @@ public function sendRemoteShare( $roomName = $room->getName(); $roomType = $room->getType(); $roomToken = $room->getToken(); + $roomCallPermissions = $room->getCallPermissions(); + $roomDefaultPermissions = $room->getDefaultPermissions(); try { $this->restrictionValidator->isAllowedToInvite($sharedBy, $invitedCloudId); @@ -101,6 +103,8 @@ public function sendRemoteShare( $protocol['invitedCloudId'] = $invitedCloudId->getId(); $protocol['roomName'] = $roomName; $protocol['roomType'] = $roomType; + $protocol['roomCallPermissions'] = $roomCallPermissions; + $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 656334d95b9..a170f6e87d5 100644 --- a/lib/Federation/CloudFederationProviderTalk.php +++ b/lib/Federation/CloudFederationProviderTalk.php @@ -125,6 +125,8 @@ public function shareReceived(ICloudFederationShare $share): string { $remoteId = $share->getProviderId(); $roomToken = $share->getResourceName(); $roomName = $share->getProtocol()['roomName']; + $roomCallPermissions = $share->getProtocol()['roomCallPermissions']; + $roomDefaultPermissions = $share->getProtocol()['roomDefaultPermissions']; if (isset($share->getProtocol()['invitedCloudId'])) { $localCloudId = $share->getProtocol()['invitedCloudId']; } else { @@ -173,7 +175,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, $roomCallPermissions, $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..4e04cba129d 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,8 @@ public function addRemoteRoom( int $remoteAttendeeId, int $roomType, string $roomName, + int $roomCallPermissions, + int $roomDefaultPermissions, string $remoteToken, string $remoteServerUrl, #[SensitiveParameter] @@ -91,6 +95,14 @@ 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 (empty($participant = $this->participantService->getParticipantsForRoom($room))) { + $this->roomService->setPermissions($room, 'call', Attendee::PERMISSIONS_MODIFY_SET, $roomCallPermissions, true); + $this->roomService->setPermissions($room, 'default', Attendee::PERMISSIONS_MODIFY_SET, $roomDefaultPermissions, true); + } + 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 61a925ebbe3..127a1615938 100644 --- a/tests/integration/features/federation/permissions.feature +++ b/tests/integration/features/federation/permissions.feature @@ -40,6 +40,24 @@ Feature: federation/permissions | permissions | | CLM | + Scenario: change permissions for all attendees before inviting federated user + Given user "participant1" creates room "room" (v4) + | roomType | 2 | + | roomName | room name | + When user "participant1" sets permissions for all attendees in room "room" to "SM" with 200 (v4) + And user "participant1" removes permissions for all attendees in room "room" to "S" with 200 (v4) + And user "participant1" adds permissions for all attendees in room "room" to "L" 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) + | permissions | + | CLM | + Scenario: change permissions for all attendees before federated user accepts invitation Given user "participant1" creates room "room" (v4) | roomType | 2 | @@ -74,6 +92,43 @@ Feature: federation/permissions | permissions | | CM | + Scenario: set conversation 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) + | permissions | + | CM | + + Scenario: set conversation 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) + | permissions | + | CM | + Scenario: set conversation permissions before federated user accepts invitation Given user "participant1" creates room "room" (v4) | roomType | 2 | diff --git a/tests/php/Federation/FederationTest.php b/tests/php/Federation/FederationTest.php index 936f19c3b6f..0efc6126e52 100644 --- a/tests/php/Federation/FederationTest.php +++ b/tests/php/Federation/FederationTest.php @@ -256,6 +256,8 @@ public function testReceiveRemoteShare(): void { $shareType = 'user'; $roomType = Room::TYPE_GROUP; $roomName = 'Room name'; + $roomCallPermissions = Attendee::PERMISSIONS_CUSTOM | Attendee::PERMISSIONS_PUBLISH_AUDIO; + $roomDefaultPermissions = Attendee::PERMISSIONS_CUSTOM | Attendee::PERMISSIONS_CHAT; $shareWithUser = $this->createMock(IUser::class); $shareWithUserID = '10'; @@ -277,6 +279,8 @@ public function testReceiveRemoteShare(): void { 'name' => 'nctalk', 'roomType' => $roomType, 'roomName' => $roomName, + 'roomCallPermissions' => $roomCallPermissions, + 'roomDefaultPermissions' => $roomDefaultPermissions, 'options' => [ 'sharedSecret' => $token, ], @@ -288,7 +292,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, $roomCallPermissions, $roomDefaultPermissions, $name, $remote, $token) ->willReturn($invite); $this->config->method('isFederationEnabled')