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

fix(conversations): Remove call-permissions #13086

Merged
merged 3 commits into from
Aug 22, 2024
Merged
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
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: ['master']
guests-versions: ['master']
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: ['master']
guests-versions: ['master']
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: ['master']
guests-versions: ['master']
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: ['master']
guests-versions: ['master']
Expand Down
6 changes: 3 additions & 3 deletions .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: ['master']
guests-versions: ['master']
Expand Down Expand Up @@ -115,7 +115,7 @@ jobs:
coverage: none
ini-file: development
# Temporary workaround for missing pcntl_* in PHP 8.3: ini-values: apc.enable_cli=on
ini-values: apc.enable_cli=on, disable_functions=
ini-values: apc.enable_cli=on, disable_functions=
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Expand All @@ -136,7 +136,7 @@ jobs:
./occ config:system:set debug --value=true --type=boolean
./occ config:system:set hashing_default_password --value=true --type=boolean
./occ config:system:set memcache.local --value="\\OC\\Memcache\\APCu"
./occ config:system:set memcache.distributed --value="\\OC\\Memcache\\APCu"
./occ config:system:set memcache.distributed --value="\\OC\\Memcache\\APCu"
./occ app:enable --force ${{ env.APP_NAME }}
./occ app:enable --force call_summary_bot
./occ app:enable --force circles
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
Loading