From 34019426dd82f3c2eace241e7cef9152d8d8653e Mon Sep 17 00:00:00 2001 From: Andrew Paxley Date: Wed, 14 Jun 2023 13:48:46 +1200 Subject: [PATCH] NEW add OnlyTheseMembers Inherited Permission type --- src/Security/InheritedPermissions.php | 89 ++++++++++++++++--- .../InheritedPermissionsExtension.php | 10 ++- .../php/Security/InheritedPermissionsTest.php | 41 +++++++-- .../php/Security/InheritedPermissionsTest.yml | 16 ++++ 4 files changed, 139 insertions(+), 17 deletions(-) diff --git a/src/Security/InheritedPermissions.php b/src/Security/InheritedPermissions.php index a327cbf3b..d961d11cd 100644 --- a/src/Security/InheritedPermissions.php +++ b/src/Security/InheritedPermissions.php @@ -25,37 +25,43 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher /** * Delete permission */ - const DELETE = 'delete'; + public const DELETE = 'delete'; /** * View permission */ - const VIEW = 'view'; + public const VIEW = 'view'; /** * Edit permission */ - const EDIT = 'edit'; + public const EDIT = 'edit'; /** * Anyone canView permission */ - const ANYONE = 'Anyone'; + public const ANYONE = 'Anyone'; /** * Restrict to logged in users */ - const LOGGED_IN_USERS = 'LoggedInUsers'; + public const LOGGED_IN_USERS = 'LoggedInUsers'; /** + * @TODO - rename this to ONLY_THESE_GROUPS * Restrict to specific groups */ - const ONLY_THESE_USERS = 'OnlyTheseUsers'; + public const ONLY_THESE_USERS = 'OnlyTheseUsers'; + + /** + * Restrict to specific members + */ + public const ONLY_THESE_MEMBERS = 'OnlyTheseMembers'; /** * Inherit from parent */ - const INHERIT = 'Inherit'; + public const INHERIT = 'Inherit'; /** * Class name @@ -359,19 +365,27 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher if ($member && $member->ID) { if (!Permission::checkMember($member, 'ADMIN')) { // Determine if this member matches any of the group or other rules - $groupJoinTable = $this->getJoinTable($type); + $groupJoinTable = $this->getGroupJoinTable($type); + $memberJoinTable = $this->getMemberJoinTable($type); $uninheritedPermissions = $stageRecords ->where([ - "(\"$typeField\" IN (?, ?) OR " . "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))" + "(\"$typeField\" IN (?, ?)" + . " OR (\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL)" + . " OR (\"$typeField\" = ? AND \"$memberJoinTable\".\"{$baseTable}ID\" IS NOT NULL)" + . ")" => [ self::ANYONE, self::LOGGED_IN_USERS, - self::ONLY_THESE_USERS + self::ONLY_THESE_USERS, + self::ONLY_THESE_MEMBERS, ] ]) ->leftJoin( $groupJoinTable, "\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)" + )->leftJoin( + $memberJoinTable, + "\"$memberJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . "\"$memberJoinTable\".\"MemberID\" = {$member->ID}" )->column('ID'); } else { $uninheritedPermissions = $stageRecords->column('ID'); @@ -632,6 +646,18 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher * @return string */ protected function getJoinTable($type) + { + return $this->getGroupJoinTable($type); + } + + /** + * Get group join table for type + * Defaults to those provided by {@see InheritedPermissionsExtension) + * + * @param string $type + * @return string + */ + protected function getGroupJoinTable($type) { switch ($type) { case self::DELETE: @@ -645,6 +671,27 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher } } + /** + * Get member join table for type + * Defaults to those provided by {@see InheritedPermissionsExtension) + * + * @param string $type + * @return string + */ + protected function getMemberJoinTable($type) + { + switch ($type) { + case self::DELETE: + // Delete uses edit type - Drop through + case self::EDIT: + return $this->getEditorMembersTable(); + case self::VIEW: + return $this->getViewerMembersTable(); + default: + throw new InvalidArgumentException("Invalid argument type $type"); + } + } + /** * Determine default permission for a givion check * @@ -716,6 +763,28 @@ class InheritedPermissions implements PermissionChecker, MemberCacheFlusher return "{$table}_ViewerGroups"; } + /** + * Get table to use for editor members relation + * + * @return string + */ + protected function getEditorMembersTable() + { + $table = DataObject::getSchema()->tableName($this->baseClass); + return "{$table}_EditorMembers"; + } + + /** + * Get table to use for viewer members relation + * + * @return string + */ + protected function getViewerMembersTable() + { + $table = DataObject::getSchema()->tableName($this->baseClass); + return "{$table}_ViewerMembers"; + } + /** * Gets the permission from cache * diff --git a/src/Security/InheritedPermissionsExtension.php b/src/Security/InheritedPermissionsExtension.php index df069b5c6..291e84bcd 100644 --- a/src/Security/InheritedPermissionsExtension.php +++ b/src/Security/InheritedPermissionsExtension.php @@ -12,17 +12,21 @@ use SilverStripe\ORM\ManyManyList; * @property string $CanEditType * @method ManyManyList ViewerGroups() * @method ManyManyList EditorGroups() + * @method ManyManyList ViewerMembers() + * @method ManyManyList EditorMembers() */ class InheritedPermissionsExtension extends DataExtension { private static array $db = [ - 'CanViewType' => "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, Inherit', 'Inherit')", - 'CanEditType' => "Enum('LoggedInUsers, OnlyTheseUsers, Inherit', 'Inherit')", + 'CanViewType' => "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers, Inherit', 'Inherit')", + 'CanEditType' => "Enum('LoggedInUsers, OnlyTheseUsers, OnlyTheseMembers, Inherit', 'Inherit')", ]; private static array $many_many = [ 'ViewerGroups' => Group::class, 'EditorGroups' => Group::class, + 'ViewerMembers' => Member::class, + 'EditorMembers' => Member::class, ]; private static array $defaults = [ @@ -33,5 +37,7 @@ class InheritedPermissionsExtension extends DataExtension private static array $cascade_duplicates = [ 'ViewerGroups', 'EditorGroups', + 'ViewerMembers', + 'EditorMembers', ]; } diff --git a/tests/php/Security/InheritedPermissionsTest.php b/tests/php/Security/InheritedPermissionsTest.php index 696351030..4eefa5986 100644 --- a/tests/php/Security/InheritedPermissionsTest.php +++ b/tests/php/Security/InheritedPermissionsTest.php @@ -68,6 +68,7 @@ class InheritedPermissionsTest extends SapphireTest public function testEditPermissions() { $editor = $this->objFromFixture(Member::class, 'editor'); + $freddie = $this->objFromFixture(Member::class, 'oneFileFreddie'); $about = $this->objFromFixture(TestPermissionNode::class, 'about'); $aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff'); @@ -75,10 +76,12 @@ class InheritedPermissionsTest extends SapphireTest $products = $this->objFromFixture(TestPermissionNode::class, 'products'); $product1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1'); $product4 = $this->objFromFixture(TestPermissionNode::class, 'products-product4'); + $freddiesFile = $this->objFromFixture(TestPermissionNode::class, 'freddies-file'); // Test logged out users cannot edit - Member::actAs(null, function () use ($aboutStaff) { + Member::actAs(null, function () use ($aboutStaff, $freddiesFile) { $this->assertFalse($aboutStaff->canEdit()); + $this->assertFalse($freddiesFile->canEdit()); }); // Can't edit a page that is locked to admins @@ -96,6 +99,10 @@ class InheritedPermissionsTest extends SapphireTest // Test that root node respects root permissions $this->assertTrue($history->canEdit($editor)); + // Test that only Freddie can edit Freddie's file + $this->assertFalse($freddiesFile->canEdit($editor)); + $this->assertTrue($freddiesFile->canEdit($freddie)); + TestPermissionNode::getInheritedPermissions()->clearCache(); $this->rootPermissions->setCanEdit(false); @@ -106,6 +113,7 @@ class InheritedPermissionsTest extends SapphireTest public function testDeletePermissions() { $editor = $this->objFromFixture(Member::class, 'editor'); + $freddie = $this->objFromFixture(Member::class, 'oneFileFreddie'); $about = $this->objFromFixture(TestPermissionNode::class, 'about'); $aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff'); @@ -113,10 +121,12 @@ class InheritedPermissionsTest extends SapphireTest $products = $this->objFromFixture(TestPermissionNode::class, 'products'); $product1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1'); $product4 = $this->objFromFixture(TestPermissionNode::class, 'products-product4'); + $freddiesFile = $this->objFromFixture(TestPermissionNode::class, 'freddies-file'); // Test logged out users cannot edit - Member::actAs(null, function () use ($aboutStaff) { + Member::actAs(null, function () use ($aboutStaff, $freddiesFile) { $this->assertFalse($aboutStaff->canDelete()); + $this->assertFalse($freddiesFile->canDelete()); }); // Can't edit a page that is locked to admins @@ -134,6 +144,10 @@ class InheritedPermissionsTest extends SapphireTest // Test that root node respects root permissions $this->assertTrue($history->canDelete($editor)); + // Test that only Freddie can delete Freddie's file + $this->assertFalse($freddiesFile->canDelete($editor)); + $this->assertTrue($freddiesFile->canDelete($freddie)); + TestPermissionNode::getInheritedPermissions()->clearCache(); $this->rootPermissions->setCanEdit(false); @@ -150,14 +164,17 @@ class InheritedPermissionsTest extends SapphireTest $secretNested = $this->objFromFixture(TestPermissionNode::class, 'secret-nested'); $protected = $this->objFromFixture(TestPermissionNode::class, 'protected'); $protectedChild = $this->objFromFixture(TestPermissionNode::class, 'protected-child'); - $editor = $this->objFromFixture(Member::class, 'editor'); $restricted = $this->objFromFixture(TestPermissionNode::class, 'restricted-page'); + $freddiesFile = $this->objFromFixture(TestPermissionNode::class, 'freddies-file'); + + $editor = $this->objFromFixture(Member::class, 'editor'); $admin = $this->objFromFixture(Member::class, 'admin'); + $freddie = $this->objFromFixture(Member::class, 'oneFileFreddie'); // Not logged in user can only access Inherit or Anyone pages Member::actAs( null, - function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm) { + function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm, $freddiesFile) { $this->assertTrue($history->canView()); $this->assertTrue($contact->canView()); $this->assertTrue($contactForm->canView()); @@ -166,6 +183,7 @@ class InheritedPermissionsTest extends SapphireTest $this->assertFalse($secretNested->canView()); $this->assertFalse($protected->canView()); $this->assertFalse($protectedChild->canView()); + $this->assertFalse($freddiesFile->canView()); } ); @@ -180,6 +198,10 @@ class InheritedPermissionsTest extends SapphireTest // Check root permissions $this->assertTrue($history->canView($editor)); + // Test that only Freddie can view Freddie's file + $this->assertFalse($freddiesFile->canView($editor)); + $this->assertTrue($freddiesFile->canView($freddie)); + TestPermissionNode::getInheritedPermissions()->clearCache(); $this->rootPermissions->setCanView(false); @@ -198,12 +220,16 @@ class InheritedPermissionsTest extends SapphireTest $secretNested = $this->objFromFixture(UnstagedNode::class, 'secret-nested'); $protected = $this->objFromFixture(UnstagedNode::class, 'protected'); $protectedChild = $this->objFromFixture(UnstagedNode::class, 'protected-child'); + $freddiesFile = $this->objFromFixture(UnstagedNode::class, 'freddies-file'); + $editor = $this->objFromFixture(Member::class, 'editor'); + $freddie = $this->objFromFixture(Member::class, 'oneFileFreddie'); + // Not logged in user can only access Inherit or Anyone pages Member::actAs( null, - function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm) { + function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm, $freddiesFile) { $this->assertTrue($history->canView()); $this->assertTrue($contact->canView()); $this->assertTrue($contactForm->canView()); @@ -212,6 +238,7 @@ class InheritedPermissionsTest extends SapphireTest $this->assertFalse($secretNested->canView()); $this->assertFalse($protected->canView()); $this->assertFalse($protectedChild->canView()); + $this->assertFalse($freddiesFile->canView()); } ); @@ -226,6 +253,10 @@ class InheritedPermissionsTest extends SapphireTest // Check root permissions $this->assertTrue($history->canView($editor)); + // Test that only Freddie can view Freddie's file + $this->assertFalse($freddiesFile->canView($editor)); + $this->assertTrue($freddiesFile->canView($freddie)); + UnstagedNode::getInheritedPermissions()->clearCache(); $this->rootPermissions->setCanView(false); diff --git a/tests/php/Security/InheritedPermissionsTest.yml b/tests/php/Security/InheritedPermissionsTest.yml index 92141c361..558abe02b 100644 --- a/tests/php/Security/InheritedPermissionsTest.yml +++ b/tests/php/Security/InheritedPermissionsTest.yml @@ -33,6 +33,10 @@ SilverStripe\Security\Member: Groups: =>SilverStripe\Security\Group.allsections securityadmin: Groups: =>SilverStripe\Security\Group.securityadmins + oneFileFreddie: + FirstName: Freddie + Surname: Fantastic + Groups: =>SilverStripe\Security\Group.editors SilverStripe\Security\Tests\InheritedPermissionsTest\TestPermissionNode: about: @@ -104,6 +108,12 @@ SilverStripe\Security\Tests\InheritedPermissionsTest\TestPermissionNode: Title: Restricted Page CanViewType: OnlyTheseUsers ViewerGroups: =>SilverStripe\Security\Group.allsections + freddies-file: + Title: Freddies File + CanViewType: OnlyTheseMembers + CanEditType: OnlyTheseMembers + ViewerMembers: =>SilverStripe\Security\Member.oneFileFreddie + EditorMembers: =>SilverStripe\Security\Member.oneFileFreddie SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode: about: @@ -175,3 +185,9 @@ SilverStripe\Security\Tests\InheritedPermissionsTest\UnstagedNode: Title: Restricted Page CanViewType: OnlyTheseUsers ViewerGroups: =>SilverStripe\Security\Group.allsections + freddies-file: + Title: Freddies File + CanViewType: OnlyTheseMembers + CanEditType: OnlyTheseMembers + ViewerMembers: =>SilverStripe\Security\Member.oneFileFreddie + EditorMembers: =>SilverStripe\Security\Member.oneFileFreddie