Skip to content

Commit

Permalink
Merge pull request #13105 from nextcloud/backport/13086/stable30
Browse files Browse the repository at this point in the history
[stable30]  fix(conversations): Remove call-permissions
  • Loading branch information
nickvergessen authored Aug 22, 2024
2 parents 923c3a3 + 0a0be11 commit 7b9b367
Show file tree
Hide file tree
Showing 40 changed files with 122 additions and 250 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/integration-mariadb.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test-suite: ['callapi', 'chat-1', 'chat-2', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
test-suite: ['callapi', 'chat-1', 'chat-2', 'chat-3', 'chat-4', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
php-versions: ['8.2']
server-versions: ['stable30']
guests-versions: ['stable30']
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-mysql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test-suite: ['callapi', 'chat-1', 'chat-2', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
test-suite: ['callapi', 'chat-1', 'chat-2', 'chat-3', 'chat-4', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
php-versions: ['8.2']
server-versions: ['stable30']
guests-versions: ['stable30']
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-oci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test-suite: ['callapi', 'chat-1', 'chat-2', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
test-suite: ['callapi', 'chat-1', 'chat-2', 'chat-3', 'chat-4', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
php-versions: ['8.2']
server-versions: ['stable30']
guests-versions: ['stable30']
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-pgsql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test-suite: ['callapi', 'chat-1', 'chat-2', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
test-suite: ['callapi', 'chat-1', 'chat-2', 'chat-3', 'chat-4', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
php-versions: ['8.3']
server-versions: ['stable30']
guests-versions: ['stable30']
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-sqlite.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
strategy:
fail-fast: false
matrix:
test-suite: ['callapi', 'chat-1', 'chat-2', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
test-suite: ['callapi', 'chat-1', 'chat-2', 'chat-3', 'chat-4', 'command', 'conversation-1', 'conversation-2', 'conversation-3', 'conversation-4', 'conversation-5', 'federation', 'integration', 'sharing-1', 'sharing-2', 'sharing-3', 'sharing-4']
php-versions: ['8.2']
server-versions: ['stable30']
guests-versions: ['stable30']
Expand Down
2 changes: 1 addition & 1 deletion docs/conversation.md
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ Get all (for moderators and in case of "free selection") or the assigned breakou

| field | type | Description |
|---------------|--------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `mode` | string | `default` or `call`, in case of call the permissions will be reset to `0` (default) after the end of a call. |
| `mode` | string | `default` or `call`, in case of call the permissions will be reset to `0` (default) after the end of a call. (🏁 `call` is no-op since Talk 20) |
| `permissions` | int | New permissions for the attendees, see [constants list](constants.md#attendee-permissions). If permissions are not `0` (default), the `1` (custom) permission will always be added. Note that this will reset all custom permissions that have been given to attendees so far. |

* Response:
Expand Down
1 change: 1 addition & 0 deletions docs/participant.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ Setting custom permissions for a self-joined user will also make them a permanen

## Set permissions for all attendees

* Deprecated: 🏁 The API is no-op since Talk 20
* Method: `PUT`
* Endpoint: `/room/{token}/attendees/permissions/all`
* Data:
Expand Down
8 changes: 1 addition & 7 deletions lib/Controller/CallController.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,6 @@ public function getPeersForCall(): DataResponse {
*
* @param int<0, 15>|null $flags In-Call flags
* @psalm-param int-mask-of<Participant::FLAG_*>|null $flags
* @param int<0, 255>|null $forcePermissions In-call permissions
* @psalm-param int-mask-of<Attendee::PERMISSIONS_*>|null $forcePermissions
* @param bool $silent Join the call silently
* @param bool $recordingConsent When the user ticked a checkbox and agreed with being recorded
* (Only needed when the `config => call => recording-consent` capability is set to {@see RecordingService::CONSENT_REQUIRED_YES}
Expand All @@ -135,7 +133,7 @@ public function getPeersForCall(): DataResponse {
#[RequireModeratorOrNoLobby]
#[RequireParticipant]
#[RequireReadWriteConversation]
public function joinCall(?int $flags = null, ?int $forcePermissions = null, bool $silent = false, bool $recordingConsent = false): DataResponse {
public function joinCall(?int $flags = null, bool $silent = false, bool $recordingConsent = false): DataResponse {
try {
$this->validateRecordingConsent($recordingConsent);
} catch (\InvalidArgumentException) {
Expand Down Expand Up @@ -166,10 +164,6 @@ public function joinCall(?int $flags = null, ?int $forcePermissions = null, bool
return $response;
}

if ($forcePermissions !== null && $this->participant->hasModeratorPermissions()) {
$this->roomService->setPermissions($this->room, 'call', Attendee::PERMISSIONS_MODIFY_SET, $forcePermissions, true);
}

try {
$this->participantService->changeInCall($this->room, $this->participant, $flags, silent: $silent);
$this->roomService->setActiveSince($this->room, $this->participant, $this->timeFactory->getDateTime(), $flags, silent: $silent);
Expand Down
17 changes: 13 additions & 4 deletions lib/Controller/RoomController.php
Original file line number Diff line number Diff line change
Expand Up @@ -2108,19 +2108,27 @@ protected function changeParticipantType(int $attendeeId, bool $promote): DataRe
/**
* Update the permissions of a room
*
* @param 'call'|'default' $mode Level of the permissions ('call', 'default')
* @param 'call'|'default' $mode Level of the permissions ('call' (removed in Talk 20), 'default')
* @param int<0, 255> $permissions New permissions
* @psalm-param int-mask-of<Attendee::PERMISSIONS_*> $permissions
* @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array<empty>, array{}>
* @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array{error: 'breakout-room'|'mode'|'type'}, array{}>
*
* 200: Permissions updated successfully
* 400: Updating permissions is not possible
*/
#[PublicPage]
#[RequireModeratorParticipant]
public function setPermissions(string $mode, int $permissions): DataResponse {
if (!$this->roomService->setPermissions($this->room, $mode, Attendee::PERMISSIONS_MODIFY_SET, $permissions, true)) {
return new DataResponse([], Http::STATUS_BAD_REQUEST);
if ($mode !== 'default') {
return new DataResponse(['error' => 'mode'], Http::STATUS_BAD_REQUEST);
}

try {
$this->roomService->setDefaultPermissions($this->room, $permissions);
} catch (\InvalidArgumentException $e) {
/** @var 'breakout-room'|'type' $error */
$error = $e->getMessage();
return new DataResponse(['error' => $error], Http::STATUS_BAD_REQUEST);
}

return new DataResponse($this->formatRoom($this->room, $this->participant));
Expand Down Expand Up @@ -2171,6 +2179,7 @@ public function setAttendeePermissions(int $attendeeId, string $method, int $per
* @param int<0, 255> $permissions New permissions
* @psalm-param int-mask-of<Attendee::PERMISSIONS_*> $permissions
* @return DataResponse<Http::STATUS_OK, TalkRoom, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, array<empty>, array{}>
* @deprecated Call permissions have been removed
*
* 200: Permissions updated successfully
* 400: Updating permissions is not possible
Expand Down
1 change: 1 addition & 0 deletions lib/Events/ARoomModifiedEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ abstract class ARoomModifiedEvent extends ARoomEvent {
public const PROPERTY_AVATAR = 'avatar';
public const PROPERTY_BREAKOUT_ROOM_MODE = 'breakoutRoomMode';
public const PROPERTY_BREAKOUT_ROOM_STATUS = 'breakoutRoomStatus';
/** @deprecated */
public const PROPERTY_CALL_PERMISSIONS = 'callPermissions';
public const PROPERTY_CALL_RECORDING = 'callRecording';
public const PROPERTY_DEFAULT_PERMISSIONS = 'defaultPermissions';
Expand Down
5 changes: 0 additions & 5 deletions lib/Participant.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,6 @@ protected function getPermissionsFromFallbackChain(): int {
return $this->getAttendee()->getPermissions();
}

if ($this->room->getCallPermissions() !== Attendee::PERMISSIONS_DEFAULT) {
// The currently ongoing call is in a special mode
return $this->room->getCallPermissions();
}

if ($this->room->getDefaultPermissions() !== Attendee::PERMISSIONS_DEFAULT) {
// The conversation has some permissions set
return $this->room->getDefaultPermissions();
Expand Down
8 changes: 7 additions & 1 deletion lib/Room.php
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,16 @@ public function setDefaultPermissions(int $defaultPermissions): void {
$this->defaultPermissions = $defaultPermissions;
}

/**
* @deprecated
*/
public function getCallPermissions(): int {
return $this->callPermissions;
return Attendee::PERMISSIONS_DEFAULT;
}

/**
* @deprecated
*/
public function setCallPermissions(int $callPermissions): void {
$this->callPermissions = $callPermissions;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/RoomFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ public function formatRoomV4(
'attendeeId' => $attendee->getId(),
'permissions' => $currentParticipant->getPermissions(),
'attendeePermissions' => $attendee->getPermissions(),
'callPermissions' => $room->getCallPermissions(),
'callPermissions' => Attendee::PERMISSIONS_DEFAULT,
'defaultPermissions' => $room->getDefaultPermissions(),
'description' => $room->getDescription(),
'listable' => $room->getListable(),
Expand Down
91 changes: 36 additions & 55 deletions lib/Service/RoomService.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,78 +169,60 @@ public function prepareConversationName(string $objectName): string {
return rtrim(mb_substr(ltrim($objectName), 0, 64));
}

/**
* @deprecated
*/
public function setPermissions(Room $room, string $level, string $method, int $permissions, bool $resetCustomPermissions): bool {
if ($room->getType() === Room::TYPE_ONE_TO_ONE || $room->getType() === Room::TYPE_ONE_TO_ONE_FORMER) {
return false;
if ($level === 'default' && $method === 'set') {
try {
$this->setDefaultPermissions($room, $permissions);
return true;
} catch (InvalidArgumentException) {
return false;

}
}
return false;
}

if ($room->getType() === Room::TYPE_NOTE_TO_SELF) {
return false;
/**
* @throws InvalidArgumentException
*/
public function setDefaultPermissions(Room $room, int $permissions): void {
if ($room->getType() === Room::TYPE_ONE_TO_ONE
|| $room->getType() === Room::TYPE_ONE_TO_ONE_FORMER
|| $room->getType() === Room::TYPE_NOTE_TO_SELF) {
throw new \InvalidArgumentException('type');
}

if ($room->getObjectType() === BreakoutRoom::PARENT_OBJECT_TYPE) {
// Do not allow manual changing the permissions in breakout rooms
return false;
}

if ($level === 'default') {
$property = ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS;
$oldPermissions = $room->getDefaultPermissions();
} 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;
throw new InvalidArgumentException('breakout-room');
}

$oldPermissions = $room->getDefaultPermissions();
$newPermissions = $permissions;
if ($method === Attendee::PERMISSIONS_MODIFY_SET) {
if ($newPermissions !== Attendee::PERMISSIONS_DEFAULT) {
// Make sure the custom flag is set when not setting to default permissions
$newPermissions |= Attendee::PERMISSIONS_CUSTOM;
}
// If we are setting a fixed set of permissions and apply that to users,
// we can also simplify it and reset to default.
$resetCustomPermissions = true;
} elseif ($method === Attendee::PERMISSIONS_MODIFY_ADD) {
$newPermissions = $oldPermissions | $newPermissions;
} elseif ($method === Attendee::PERMISSIONS_MODIFY_REMOVE) {
$newPermissions = $oldPermissions & ~$newPermissions;
} else {
return false;
if ($newPermissions !== Attendee::PERMISSIONS_DEFAULT) {
// Make sure the custom flag is set when not setting to default permissions
$newPermissions |= Attendee::PERMISSIONS_CUSTOM;
}

$event = new BeforeRoomModifiedEvent($room, $property, $newPermissions, $oldPermissions);
$event = new BeforeRoomModifiedEvent($room, ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS, $newPermissions, $oldPermissions);
$this->dispatcher->dispatchTyped($event);

if ($resetCustomPermissions) {
$this->participantService->updateAllPermissions($room, Attendee::PERMISSIONS_MODIFY_SET, Attendee::PERMISSIONS_DEFAULT);
} else {
$this->participantService->updateAllPermissions($room, $method, $permissions);
}
// Reset custom user permissions to default
$this->participantService->updateAllPermissions($room, Attendee::PERMISSIONS_MODIFY_SET, Attendee::PERMISSIONS_DEFAULT);

$update = $this->db->getQueryBuilder();
$update->update('talk_rooms')
->set($level . '_permissions', $update->createNamedParameter($newPermissions, IQueryBuilder::PARAM_INT))
->set('default_permissions', $update->createNamedParameter($newPermissions, IQueryBuilder::PARAM_INT))
->where($update->expr()->eq('id', $update->createNamedParameter($room->getId(), IQueryBuilder::PARAM_INT)));
$update->executeStatement();

if ($level === 'default') {
$room->setDefaultPermissions($newPermissions);
} else {
$room->setCallPermissions($newPermissions);
}
$room->setDefaultPermissions($newPermissions);

$event = new RoomModifiedEvent($room, $property, $newPermissions, $oldPermissions);
$event = new RoomModifiedEvent($room, ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS, $newPermissions, $oldPermissions);
$this->dispatcher->dispatchTyped($event);

return true;
}

public function setSIPEnabled(Room $room, int $newSipEnabled): bool {
Expand Down Expand Up @@ -875,7 +857,6 @@ public function resetActiveSinceInDatabaseOnly(Room $room): bool {
*/
public function resetActiveSinceInModelOnly(Room $room): void {
$room->resetActiveSince();
$room->setCallPermissions(Attendee::PERMISSIONS_DEFAULT);
}

public function resetActiveSince(Room $room, ?Participant $participant): void {
Expand Down Expand Up @@ -1055,11 +1036,11 @@ public function syncPropertiesFromHostRoom(Room $local, array $host): void {
}
}
if (isset($host['defaultPermissions']) && $host['defaultPermissions'] !== $local->getDefaultPermissions()) {
$success = $this->setPermissions($local, 'default', Attendee::PERMISSIONS_MODIFY_SET, $host['defaultPermissions'], false);
if (!$success) {
$this->logger->error('An error occurred while trying to sync defaultPermissions of ' . $local->getId() . ' to ' . $host['defaultPermissions']);
} else {
try {
$this->setDefaultPermissions($local, $host['defaultPermissions']);
$changed[] = ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS;
} catch (\InvalidArgumentException $e) {
$this->logger->error('An error (' . $e->getMessage() . ') occurred while trying to sync defaultPermissions of ' . $local->getId() . ' to ' . $host['defaultPermissions'], ['exception' => $e]);
}
}
if (isset($host['avatarVersion']) && $host['avatarVersion'] !== $local->getAvatar()) {
Expand Down
8 changes: 2 additions & 6 deletions lib/Signaling/Listener.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class Listener implements IEventListener {
ARoomModifiedEvent::PROPERTY_BREAKOUT_ROOM_MODE,
ARoomModifiedEvent::PROPERTY_BREAKOUT_ROOM_STATUS,
ARoomModifiedEvent::PROPERTY_CALL_RECORDING,
ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS,
ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS,
ARoomModifiedEvent::PROPERTY_DESCRIPTION,
ARoomModifiedEvent::PROPERTY_LISTABLE,
Expand Down Expand Up @@ -114,7 +113,6 @@ protected function refreshParticipantListParticipantModified(ParticipantModified

protected function refreshParticipantListRoomModified(RoomModifiedEvent $event): void {
if (!in_array($event->getProperty(), [
ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS,
ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS,
], true)) {
return;
Expand Down Expand Up @@ -153,8 +151,7 @@ protected function notifyRoomModified(ARoomModifiedEvent $event): void {
return;
}

if ($event->getProperty() === ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS
|| $event->getProperty() === ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS) {
if ($event->getProperty() === ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS) {
$this->notifyRoomPermissionsModified($event);
// The room permission itself does not need a signaling message anymore
return;
Expand All @@ -175,8 +172,7 @@ protected function notifyRoomSynced(RoomSyncedEvent $event): void {
return;
}

if (in_array(ARoomModifiedEvent::PROPERTY_CALL_PERMISSIONS, $event->getProperties(), true)
|| in_array(ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS, $event->getProperties(), true)) {
if (in_array(ARoomModifiedEvent::PROPERTY_DEFAULT_PERMISSIONS, $event->getProperties(), true)) {
$this->notifyRoomPermissionsModified($event);
}

Expand Down
Loading

0 comments on commit 7b9b367

Please sign in to comment.