diff --git a/src/Security/Permission.php b/src/Security/Permission.php index 42e9fbe55cd..804091ae66a 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -392,9 +392,16 @@ public static function groupList($memberID = null) */ public static function grant($groupID, $code, $arg = "any") { - $perm = new Permission(); - $perm->GroupID = $groupID; - $perm->Code = $code; + $permissions = Permission::get()->filter(['GroupID' => $groupID, 'Code' => $code]); + + if ($permissions && $permissions->count() > 0) { + $perm = $permissions->last(); + } else { + $perm = new Permission(); + $perm->GroupID = $groupID; + $perm->Code = $code; + } + $perm->Type = self::GRANT_PERMISSION; // Arg component @@ -427,9 +434,16 @@ public static function grant($groupID, $code, $arg = "any") */ public static function deny($groupID, $code, $arg = "any") { - $perm = new Permission(); - $perm->GroupID = $groupID; - $perm->Code = $code; + $permissions = Permission::get()->filter(['GroupID' => $groupID, 'Code' => $code]); + + if ($permissions && $permissions->count() > 0) { + $perm = $permissions->last(); + } else { + $perm = new Permission(); + $perm->GroupID = $groupID; + $perm->Code = $code; + } + $perm->Type = self::DENY_PERMISSION; // Arg component diff --git a/tests/php/Security/PermissionTest.php b/tests/php/Security/PermissionTest.php index 8ef2ef76dfa..265e71ad889 100644 --- a/tests/php/Security/PermissionTest.php +++ b/tests/php/Security/PermissionTest.php @@ -3,6 +3,7 @@ namespace SilverStripe\Security\Tests; use SilverStripe\Security\Permission; +use SilverStripe\Security\Group; use SilverStripe\Security\Member; use SilverStripe\Security\PermissionCheckboxSetField; use SilverStripe\Core\Config\Config; @@ -163,4 +164,124 @@ public function testEmptyMemberFails() $this->assertFalse(Permission::checkMember($member, 'ADMIN')); $this->assertFalse(Permission::checkMember($member, 'CMS_ACCESS_LeftAndMain')); } + + public function testGrantPermission() + { + $group = $this->objFromFixture(Group::class, 'testpermissiongroup'); + $id = $group->ID; + + Permission::grant($id, 'CMS_ACCESS_CMSMain'); + Permission::grant($id, 'CMS_ACCESS_AssetAdmin'); + Permission::grant($id, 'CMS_ACCESS_ReportAdmin'); + + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(3, $groupPermission->count()); + $this->assertEquals(0, $groupPermission->first()->Arg); + $this->assertEquals(1, $groupPermission->first()->Type); + + + Permission::grant($id, 'CMS_ACCESS_CMSMain', 'all'); + Permission::grant($id, 'CMS_ACCESS_AssetAdmin', 'all'); + Permission::grant($id, 'CMS_ACCESS_ReportAdmin', 'all'); + + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(3, $groupPermission->count()); + $this->assertEquals(-1, $groupPermission->first()->Arg); + $this->assertEquals(1, $groupPermission->first()->Type); + + Permission::grant($id, 'CMS_ACCESS_CMSMain', 'any'); + Permission::grant($id, 'CMS_ACCESS_AssetAdmin', 'any'); + Permission::grant($id, 'CMS_ACCESS_ReportAdmin', 'any'); + + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(3, $groupPermission->count()); + $this->assertEquals(-1, $groupPermission->first()->Arg); + $this->assertEquals(1, $groupPermission->first()->Type); + } + + public function testDenyPermission() + { + $group = $this->objFromFixture(Group::class, 'testpermissiongroup'); + $id = $group->ID; + + Permission::deny($id, 'CMS_ACCESS_CMSMain'); + Permission::deny($id, 'CMS_ACCESS_AssetAdmin'); + Permission::deny($id, 'CMS_ACCESS_ReportAdmin'); + + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(3, $groupPermission->count()); + $this->assertEquals(0, $groupPermission->first()->Arg); + $this->assertEquals(-1, $groupPermission->first()->Type); + + Permission::deny($id, 'CMS_ACCESS_CMSMain', 'all'); + Permission::deny($id, 'CMS_ACCESS_AssetAdmin', 'all'); + Permission::deny($id, 'CMS_ACCESS_ReportAdmin', 'all'); + + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(3, $groupPermission->count()); + $this->assertEquals(-1, $groupPermission->first()->Arg); + $this->assertEquals(-1, $groupPermission->first()->Type); + + Permission::deny($id, 'CMS_ACCESS_CMSMain', 'any'); + Permission::deny($id, 'CMS_ACCESS_AssetAdmin', 'any'); + Permission::deny($id, 'CMS_ACCESS_ReportAdmin', 'any'); + + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(3, $groupPermission->count()); + $this->assertEquals(-1, $groupPermission->first()->Arg); + $this->assertEquals(-1, $groupPermission->first()->Type); + } + + public function testDenyThenGrantPermission() + { + $member = $this->objFromFixture(Member::class, 'testcmseditormember'); + $group = $this->objFromFixture(Group::class, 'testcmseditorgroup'); + $id = $group->ID; + + $this->logInAs($member); + + Permission::grant($id, 'TEST_CMS_EDITOR'); + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(1, $groupPermission->count()); + $this->assertEquals(1, $groupPermission->first()->Type); + $this->assertTrue(Permission::check('TEST_CMS_EDITOR')); + + Permission::deny($id, 'TEST_CMS_EDITOR'); + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(1, $groupPermission->count()); + $this->assertEquals(-1, $groupPermission->last()->Type); + $this->assertFalse(Permission::check('TEST_CMS_EDITOR')); + + Permission::grant($id, 'TEST_CMS_EDITOR'); + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + + $this->assertEquals(1, $groupPermission->count()); + $this->assertEquals(1, $groupPermission->first()->Type); + $this->assertTrue(Permission::check('TEST_CMS_EDITOR')); + + Permission::grant($id, 'CMS_ACCESS_AssetAdmin'); + $groupPermission = Permission::get()->filter(['GroupID' => $id]); + $this->assertEquals(2, $groupPermission->count()); + + $groupPermissionAssetAdmin = Permission::get()->filter( + [ + 'GroupID' => $id, + 'Code' => 'CMS_ACCESS_AssetAdmin', + ] + ); + $this->assertEquals(1, $groupPermissionAssetAdmin->count()); + $this->assertEquals(1, $groupPermissionAssetAdmin->first()->Type); + + $this->assertTrue(Permission::check('CMS_ACCESS_AssetAdmin')); + + $this->logOut(); + } } diff --git a/tests/php/Security/PermissionTest.yml b/tests/php/Security/PermissionTest.yml index ea80ce56bc3..e90fba2d296 100644 --- a/tests/php/Security/PermissionTest.yml +++ b/tests/php/Security/PermissionTest.yml @@ -33,6 +33,10 @@ FirstName: Left Surname: AndMain Email: leftandmain@example.com + testcmseditormember: + FirstName: CMS + Surname: Editor + Email: testcmseditor@example.com 'SilverStripe\Security\Group': author: @@ -50,6 +54,14 @@ leftandmain: Title: LeftAndMain Members: '=>SilverStripe\Security\Member.leftandmain' + cmsmaingroup: + Title: CMSMain + Members: '=>SilverStripe\Security\Member.testcmseditormember' + testpermissiongroup: + Title: TestPermissionGroup + testcmseditorgroup: + Title: TestCMSEditor + Members: '=>SilverStripe\Security\Member.testcmseditormember' 'SilverStripe\Security\Permission': extra1: @@ -61,3 +73,9 @@ leftandmain: Code: CMS_ACCESS_LeftAndMain Group: '=>SilverStripe\Security\Group.leftandmain' + cmsmain: + Code: CMS_ACCESS_CMSMain + Group: '=>SilverStripe\Security\Group.cmsmaingroup' + testcmseditor: + Code: TEST_CMS_EDITOR + Group: '=>SilverStripe\Security\Group.testcmseditorgroup'