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.

Sign up for GitHub

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

Jump to bottom

[stable30] Fix adding and removing permissions #13082

Merged
merged 5 commits into from
Aug 22, 2024
Merged

[stable30] Fix adding and removing permissions #13082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7f81182
test: Add integration tests for adding and removing permissions
danxuliu Aug 17, 2024
90fae7e
fix: Fix adding permissions when attendees have default permissions
danxuliu Aug 17, 2024
6b21f33
refactor: Extract method to get the base query for setting permissions
danxuliu Aug 17, 2024
44c586d
fix: Fix adding and removing several attendee permissions at once
danxuliu Aug 17, 2024
94b09aa
fix: Fix adding or removing permissions without call permissions set
danxuliu Aug 17, 2024
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
33 changes: 25 additions & 8 deletions lib/Model/AttendeeMapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,18 +203,12 @@ public function deleteByIds(array $ids): int {
}

public function modifyPermissions(int $roomId, string $mode, int $newState): void {
$query = $this->db->getQueryBuilder();
$query->update($this->getTableName())
->where($query->expr()->eq('room_id', $query->createNamedParameter($roomId, IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->notIn('actor_type', $query->createNamedParameter([
Attendee::ACTOR_CIRCLES,
Attendee::ACTOR_GROUPS,
], IQueryBuilder::PARAM_STR_ARRAY)));

if ($mode === Attendee::PERMISSIONS_MODIFY_SET) {
if ($newState !== Attendee::PERMISSIONS_DEFAULT) {
$newState |= Attendee::PERMISSIONS_CUSTOM;
}

$query = $this->getModifyPermissionsBaseQuery($roomId);
$query->set('permissions', $query->createNamedParameter($newState, IQueryBuilder::PARAM_INT));
$query->executeStatement();
} else {
Expand All @@ -227,6 +221,8 @@ public function modifyPermissions(int $roomId, string $mode, int $newState): voi
Attendee::PERMISSIONS_LOBBY_IGNORE,
] as $permission) {
if ($permission & $newState) {
$query = $this->getModifyPermissionsBaseQuery($roomId);

if ($mode === Attendee::PERMISSIONS_MODIFY_ADD) {
$this->addSinglePermission($query, $permission);
} elseif ($mode === Attendee::PERMISSIONS_MODIFY_REMOVE) {
Expand All @@ -237,6 +233,18 @@ public function modifyPermissions(int $roomId, string $mode, int $newState): voi
}
}

protected function getModifyPermissionsBaseQuery(int $roomId): IQueryBuilder {
$query = $this->db->getQueryBuilder();
$query->update($this->getTableName())
->where($query->expr()->eq('room_id', $query->createNamedParameter($roomId, IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->notIn('actor_type', $query->createNamedParameter([
Attendee::ACTOR_CIRCLES,
Attendee::ACTOR_GROUPS,
], IQueryBuilder::PARAM_STR_ARRAY)));

return $query;
}

protected function addSinglePermission(IQueryBuilder $query, int $permission): void {
$query->set('permissions', $query->func()->add(
'permissions',
Expand All @@ -255,6 +263,12 @@ protected function addSinglePermission(IQueryBuilder $query, int $permission): v
$query->createNamedParameter($permission, IQueryBuilder::PARAM_INT)
)
);
$query->andWhere(
$query->expr()->neq(
'permissions',
$query->createNamedParameter(Attendee::PERMISSIONS_DEFAULT, IQueryBuilder::PARAM_INT)
)
);

$query->executeStatement();
}
Expand All @@ -277,6 +291,9 @@ protected function removeSinglePermission(IQueryBuilder $query, int $permission)
$query->createNamedParameter($permission, IQueryBuilder::PARAM_INT)
)
);
// Removing permissions does not need to be explicitly prevented when
// the attendee has default permissions, as in that case it will not be
// possible to remove the permissions anyway.

$query->executeStatement();
}
Expand Down
6 changes: 6 additions & 0 deletions lib/Service/RoomService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,12 @@ public function setPermissions(Room $room, string $level, string $method, int $p
} 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;
}
Expand Down
50 changes: 50 additions & 0 deletions tests/integration/features/bootstrap/FeatureContext.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,21 @@ 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['permissions'])) {
$data['permissions'] = $this->mapPermissionsAPIOutput($room['permissions']);
}
if (isset($expectedRoom['attendeePermissions'])) {
$data['attendeePermissions'] = $this->mapPermissionsAPIOutput($room['attendeePermissions']);
}
if (isset($expectedRoom['callPermissions'])) {
$data['callPermissions'] = $this->mapPermissionsAPIOutput($room['callPermissions']);
}
if (isset($expectedRoom['defaultPermissions'])) {
$data['defaultPermissions'] = $this->mapPermissionsAPIOutput($room['defaultPermissions']);
}
if (isset($expectedRoom['participants'])) {
throw new \Exception('participants key needs to be checked via participants endpoint');
}
Expand Down Expand Up @@ -2024,6 +2039,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)\)$/
*
Expand Down
36 changes: 36 additions & 0 deletions tests/integration/features/conversation-5/set-permissions.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,39 @@ Feature: conversation-2/set-publishing-permissions
| actorType | actorId | permissions | attendeePermissions | participantType |
| users | owner | SJLAVPM | D | 1 |
| users | invited user | CLAVPM | CLAVPM | 3 |

Scenario: adding and removing permissions to all attendees do not change default attendee permissions
Given user "invited user2" exists
And user "owner" creates room "group room" (v4)
| roomType | 2 |
| roomName | room |
And user "owner" adds user "invited user" to room "group room" with 200 (v4)
And user "owner" adds user "invited user2" to room "group room" with 200 (v4)
And user "owner" sets call permissions for room "group room" to "LPM" with 200 (v4)
And user "owner" sets permissions for "invited user2" in room "group room" to "LVP" with 200 (v4)
When user "owner" adds permissions for all attendees in room "group room" to "JA" with 200 (v4)
And user "owner" removes permissions for all attendees in room "group room" to "SLP" with 200 (v4)
Then user "owner" sees the following attendees in room "group room" with 200 (v4)
| actorType | actorId | permissions | attendeePermissions |
| users | owner | SJLAVPM | D |
| users | invited user | CJAM | D |
| users | invited user2 | CJAV | CJAV |
And user "owner" is participant of room "group room" (v4)
| callPermissions | defaultPermissions |
| CJAM | D |

Scenario: adding and removing permissions to all attendees customize room permissions if call permissions are not set
Given user "owner" creates room "group room" (v4)
| roomType | 2 |
| roomName | room |
And user "owner" adds user "invited user" to room "group room" with 200 (v4)
And user "owner" sets default permissions for room "group room" to "LPM" with 200 (v4)
When user "owner" adds permissions for all attendees in room "group room" to "JA" with 200 (v4)
And user "owner" removes permissions for all attendees in room "group room" to "SLP" with 200 (v4)
Then user "owner" sees the following attendees in room "group room" with 200 (v4)
| actorType | actorId | permissions | attendeePermissions |
| users | owner | SJLAVPM | D |
| users | invited user | CJAM | D |
And user "owner" is participant of room "group room" (v4)
| callPermissions | defaultPermissions |
| CJAM | CLPM |
Loading