From bbfb330b921496c24bda1f821c48f97782f90232 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 4 Jan 2021 18:07:39 +0000 Subject: [PATCH] Added check of owner field for manage-permissions-own This permission was still checking based on created-by. Updated testing to specifically check the owner since the tests were passing by the fact of matching creator and owner. Fixes #2445 --- app/Auth/Permissions/PermissionService.php | 3 ++- tests/Permissions/RolesTest.php | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/Auth/Permissions/PermissionService.php b/app/Auth/Permissions/PermissionService.php index d858a7c18..89c8a5fbb 100644 --- a/app/Auth/Permissions/PermissionService.php +++ b/app/Auth/Permissions/PermissionService.php @@ -533,7 +533,8 @@ class PermissionService $allPermission = $this->currentUser() && $this->currentUser()->can($permission . '-all'); $ownPermission = $this->currentUser() && $this->currentUser()->can($permission . '-own'); $this->currentAction = 'view'; - $isOwner = $this->currentUser() && $this->currentUser()->id === $ownable->created_by; + $ownerField = ($ownable instanceof Entity) ? 'owned_by' : 'created_by'; + $isOwner = $this->currentUser() && $this->currentUser()->id === $ownable->$ownerField; return ($allPermission || ($isOwner && $ownPermission)); } diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index 3397ef429..8398d0828 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -216,15 +216,23 @@ class RolesTest extends BrowserKitTest { $otherUsersPage = Page::first(); $content = $this->createEntityChainBelongingToUser($this->user); + + // Set a different creator on the page we're checking to ensure + // that the owner fields are checked + $page = $content['page']; /** @var Page $page */ + $page->created_by = $otherUsersPage->id; + $page->owned_by = $this->user->id; + $page->save(); + // Check can't restrict other's content $this->actingAs($this->user)->visit($otherUsersPage->getUrl()) ->dontSee('Permissions') ->visit($otherUsersPage->getUrl() . '/permissions') ->seePageIs('/'); // Check can't restrict own content - $this->actingAs($this->user)->visit($content['page']->getUrl()) + $this->actingAs($this->user)->visit($page->getUrl()) ->dontSee('Permissions') - ->visit($content['page']->getUrl() . '/permissions') + ->visit($page->getUrl() . '/permissions') ->seePageIs('/'); $this->giveUserPermissions($this->user, ['restrictions-manage-own']); @@ -235,10 +243,10 @@ class RolesTest extends BrowserKitTest ->visit($otherUsersPage->getUrl() . '/permissions') ->seePageIs('/'); // Check can restrict own content - $this->actingAs($this->user)->visit($content['page']->getUrl()) + $this->actingAs($this->user)->visit($page->getUrl()) ->see('Permissions') ->click('Permissions') - ->seePageIs($content['page']->getUrl() . '/permissions'); + ->seePageIs($page->getUrl() . '/permissions'); } /**