perf: Avoid extra round trips when checking permissions

Signed-off-by: Julius Härtl <jus@bitgrid.net>
This commit is contained in:
Julius Härtl
2023-02-16 12:10:13 +01:00
parent 8ec8a91cab
commit 4b319d8d23
2 changed files with 15 additions and 17 deletions

View File

@@ -143,7 +143,7 @@ class PermissionService {
* @return bool
* @throws NoPermissionException
*/
public function checkPermission($mapper, $id, $permission, $userId = null) {
public function checkPermission($mapper, $id, $permission, $userId = null): bool {
$boardId = $id;
if ($mapper instanceof IPermissionMapper && !($mapper instanceof BoardMapper)) {
$boardId = $mapper->findBoardId($id);
@@ -153,23 +153,11 @@ class PermissionService {
throw new NoPermissionException('Permission denied');
}
if ($permission === Acl::PERMISSION_SHARE && $this->shareManager->sharingDisabledForUser($this->userId)) {
throw new NoPermissionException('Permission denied');
}
if ($this->userIsBoardOwner($boardId, $userId)) {
$permissions = $this->getPermissions($boardId);
if ($permissions[$permission] === true) {
return true;
}
try {
$acls = $this->getBoard($boardId)->getAcl() ?? [];
$result = $this->userCan($acls, $permission, $userId);
if ($result) {
return true;
}
} catch (DoesNotExistException | MultipleObjectsReturnedException $e) {
}
// Throw NoPermission to not leak information about existing entries
throw new NoPermissionException('Permission denied');
}

View File

@@ -236,6 +236,11 @@ class PermissionServiceTest extends \Test\TestCase {
$board->setAcl($this->getAcls($boardId));
$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
$this->aclMapper->expects($this->any())
->method('findAll')
->with($boardId)
->willReturn($this->getAcls($boardId));
$this->shareManager->expects($this->any())
->method('sharingDisabledForUser')
->willReturn(false);
@@ -262,12 +267,17 @@ class PermissionServiceTest extends \Test\TestCase {
$this->boardMapper->expects($this->any())->method('find')->willReturn($board);
}
$this->aclMapper->expects($this->any())
->method('findAll')
->with($boardId)
->willReturn($this->getAcls($boardId));
if ($result) {
$actual = $this->service->checkPermission($mapper, 1234, $permission);
$actual = $this->service->checkPermission($mapper, $boardId, $permission);
$this->assertTrue($actual);
} else {
$this->expectException(NoPermissionException::class);
$this->service->checkPermission($mapper, 1234, $permission);
$this->service->checkPermission($mapper, $boardId, $permission);
}
}