Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate permission changes to federated servers #12979

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
4 changes: 4 additions & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -70,6 +71,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;
Expand Down Expand Up @@ -266,10 +268,12 @@ 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);
$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);
Expand Down
42 changes: 42 additions & 0 deletions lib/Events/APermissionsModifiedEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Talk\Events;

use OCA\Talk\Room;

abstract class APermissionsModifiedEvent extends ARoomModifiedEvent {
public function __construct(
Room $room,
string $property,
string|int $newValue,
string|int|null $oldValue,
protected string $method,
protected int $permissions,
protected bool $resetCustomPermissions,
) {
parent::__construct(
$room,
$property,
$newValue,
$oldValue,
);
}

public function getMethod(): string {
return $this->method;
}

public function getPermissions(): int {
return $this->permissions;
}

public function resetCustomPermissions(): bool {
return $this->resetCustomPermissions;
}
}
12 changes: 12 additions & 0 deletions lib/Events/BeforePermissionsModifiedEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Talk\Events;

class BeforePermissionsModifiedEvent extends APermissionsModifiedEvent {
danxuliu marked this conversation as resolved.
Show resolved Hide resolved
}
12 changes: 12 additions & 0 deletions lib/Events/PermissionsModifiedEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Talk\Events;

class PermissionsModifiedEvent extends APermissionsModifiedEvent {
}
78 changes: 78 additions & 0 deletions lib/Federation/BackendNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ public function sendRemoteShare(
$roomName = $room->getName();
$roomType = $room->getType();
$roomToken = $room->getToken();
$roomCallPermissions = $room->getCallPermissions();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$roomCallPermissions = $room->getCallPermissions();

$roomDefaultPermissions = $room->getDefaultPermissions();

try {
$this->restrictionValidator->isAllowedToInvite($sharedBy, $invitedCloudId);
Expand Down Expand Up @@ -101,6 +103,8 @@ public function sendRemoteShare(
$protocol['invitedCloudId'] = $invitedCloudId->getId();
$protocol['roomName'] = $roomName;
$protocol['roomType'] = $roomType;
$protocol['roomCallPermissions'] = $roomCallPermissions;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$protocol['roomCallPermissions'] = $roomCallPermissions;

$protocol['roomDefaultPermissions'] = $roomDefaultPermissions;
Comment on lines +106 to +107
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And description, and call state, and, …

No this doesn't scale.
And as said, we remove call permissions, please don't add it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And description, and call state, and, …

No this doesn't scale.

Well, then some other approach is needed. But the initial state needs to be propagated somehow.

And as said, we remove call permissions, please don't add it

This code was written before it was decided that call permissions were going to be removed. Anyway, I will redo it and rebase once call permissions are removed in main and stable30.

$protocol['name'] = FederationManager::TALK_PROTOCOL_NAME;
$share->setProtocol($protocol);

Expand Down Expand Up @@ -251,6 +255,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
Expand Down Expand Up @@ -367,6 +405,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
Expand Down
48 changes: 46 additions & 2 deletions lib/Federation/CloudFederationProviderTalk.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ public function shareReceived(ICloudFederationShare $share): string {
$remoteId = $share->getProviderId();
$roomToken = $share->getResourceName();
$roomName = $share->getProtocol()['roomName'];
$roomCallPermissions = $share->getProtocol()['roomCallPermissions'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$roomCallPermissions = $share->getProtocol()['roomCallPermissions'];

$roomDefaultPermissions = $share->getProtocol()['roomDefaultPermissions'];
if (isset($share->getProtocol()['invitedCloudId'])) {
$localCloudId = $share->getProtocol()['invitedCloudId'];
} else {
Expand Down Expand Up @@ -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();
Expand All @@ -197,6 +199,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:
Expand Down Expand Up @@ -294,7 +298,43 @@ private function shareUnshared(int $remoteAttendeeId, array $notification): arra

/**
* @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<AParticipantModifiedEvent::DETAIL_*, bool>} $notification
* @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']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this always updates the permissions of the recipient, independent of which users permissions were actually modified? :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event includes the final permissions, no matter if they were set, added or removed, so the effect should be the same, no? 🤔

} 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, method?: string, permissions?: int, resetCustomPermissions?: bool, details?: array<AParticipantModifiedEvent::DETAIL_*, bool>} $notification
* @return array
* @throws ActionNotSupportedException
* @throws AuthenticationFailedException
Expand Down Expand Up @@ -322,6 +362,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']);
nickvergessen marked this conversation as resolved.
Show resolved Hide resolved
} 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) {
Expand Down
13 changes: 13 additions & 0 deletions lib/Federation/FederationManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,13 +43,15 @@ 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;

public function __construct(
private Manager $manager,
private ParticipantService $participantService,
private RoomService $roomService,
private InvitationMapper $invitationMapper,
private BackendNotifier $backendNotifier,
private IManager $notificationManager,
Expand All @@ -74,6 +77,8 @@ public function addRemoteRoom(
int $remoteAttendeeId,
int $roomType,
string $roomName,
int $roomCallPermissions,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int $roomCallPermissions,

int $roomDefaultPermissions,
string $remoteToken,
string $remoteServerUrl,
#[SensitiveParameter]
Expand All @@ -90,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))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. No inline assignments
  2. Use getNumberOfActors if you only need a count

$this->roomService->setPermissions($room, 'call', Attendee::PERMISSIONS_MODIFY_SET, $roomCallPermissions, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$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);
Expand Down
Loading
Loading