diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 4a652cca8..b08bfbe53 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -1416,6 +1416,7 @@ The below methods have been added or had their functionality updated to `DBDate` #### ORM Removed API +* `DataObject::can*` methods no longer accept a member ID. These must now be passed a Member object or left null * `DataObject::db` removed and replaced with `DataObjectSchema::fieldSpec` and `DataObjectSchema::fieldSpecs` * `DataObject::manyManyComponent` moved to `DataObjectSchema` * `DataObject::belongsToComponent` moved to `DataObjectSchema` diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 1cb80c07c..f8ca55c86 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -2525,8 +2525,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function can($perm, $member = null, $context = array()) { - if (!$member || !($member instanceof Member) || is_numeric($member)) { - $member = Member::currentUserID(); + if (!$member) { + $member = Member::currentUser(); } if ($member && Permission::checkMember($member, "ADMIN")) { diff --git a/src/Security/DefaultPermissionChecker.php b/src/Security/DefaultPermissionChecker.php new file mode 100644 index 000000000..098284643 --- /dev/null +++ b/src/Security/DefaultPermissionChecker.php @@ -0,0 +1,44 @@ + "SilverStripe\\Security\\Group", + "Parent" => Group::class, ); private static $has_many = array( - "Permissions" => "SilverStripe\\Security\\Permission", - "Groups" => "SilverStripe\\Security\\Group" + "Permissions" => Permission::class, + "Groups" => Group::class, ); private static $many_many = array( - "Members" => "SilverStripe\\Security\\Member", - "Roles" => "SilverStripe\\Security\\PermissionRole", + "Members" => Member::class, + "Roles" => PermissionRole::class, ); private static $extensions = array( - "SilverStripe\\ORM\\Hierarchy\\Hierarchy", + Hierarchy::class, ); private static $table_name = "Group"; @@ -132,7 +131,7 @@ class Group extends DataObject $permissionsField = new PermissionCheckboxSetField( 'Permissions', false, - 'SilverStripe\\Security\\Permission', + Permission::class, 'GroupID', $this ) @@ -157,7 +156,7 @@ class Group extends DataObject ->setResultsFormat('$Title ($Email)') ->setSearchFields(array('FirstName', 'Surname', 'Email')); /** @var GridFieldDetailForm $detailForm */ - $detailForm = $config->getComponentByType('SilverStripe\\Forms\\GridField\\GridFieldDetailForm'); + $detailForm = $config->getComponentByType(GridFieldDetailForm::class); $detailForm ->setValidator(Member_Validator::create()) ->setItemEditFormCallback(function ($form, $component) use ($group) { @@ -425,7 +424,7 @@ class Group extends DataObject $inheritedCodes = Permission::get() ->filter('GroupID', $this->Parent()->collateAncestorIDs()) ->column('Code'); - $privilegedCodes = Config::inst()->get('SilverStripe\\Security\\Permission', 'privileged_permissions'); + $privilegedCodes = Permission::config()->get('privileged_permissions'); if (array_intersect($inheritedCodes, $privilegedCodes)) { $result->addError(sprintf( _t( @@ -471,12 +470,12 @@ class Group extends DataObject * Checks for permission-code CMS_ACCESS_SecurityAdmin. * If the group has ADMIN permissions, it requires the user to have ADMIN permissions as well. * - * @param $member Member + * @param Member $member Member * @return boolean */ public function canEdit($member = null) { - if (!$member || !(is_a($member, 'SilverStripe\\Security\\Member')) || is_numeric($member)) { + if (!$member) { $member = Member::currentUser(); } @@ -507,12 +506,12 @@ class Group extends DataObject /** * Checks for permission-code CMS_ACCESS_SecurityAdmin. * - * @param $member Member + * @param Member $member * @return boolean */ public function canView($member = null) { - if (!$member || !(is_a($member, 'SilverStripe\\Security\\Member')) || is_numeric($member)) { + if (!$member) { $member = Member::currentUser(); } @@ -534,7 +533,7 @@ class Group extends DataObject public function canDelete($member = null) { - if (!$member || !(is_a($member, 'SilverStripe\\Security\\Member')) || is_numeric($member)) { + if (!$member) { $member = Member::currentUser(); } @@ -556,7 +555,7 @@ class Group extends DataObject public function AllChildrenIncludingDeleted() { /** @var Hierarchy $extInstance */ - $extInstance = $this->getExtensionInstance('SilverStripe\\ORM\\Hierarchy\\Hierarchy'); + $extInstance = $this->getExtensionInstance(Hierarchy::class); $extInstance->setOwner($this); $children = $extInstance->AllChildrenIncludingDeleted(); $extInstance->clearOwner(); @@ -585,7 +584,7 @@ class Group extends DataObject parent::requireDefaultRecords(); // Add default author group if no other group exists - $allGroups = DataObject::get('SilverStripe\\Security\\Group'); + $allGroups = Group::get(); if (!$allGroups->count()) { $authorGroup = new Group(); $authorGroup->Code = 'content-authors'; diff --git a/src/Security/InheritedPermissions.php b/src/Security/InheritedPermissions.php new file mode 100644 index 000000000..b22e45557 --- /dev/null +++ b/src/Security/InheritedPermissions.php @@ -0,0 +1,613 @@ +baseClass = $baseClass; + return $this; + } + + /** + * @param DefaultPermissionChecker $callback + * @return $this + */ + public function setDefaultPermissions(DefaultPermissionChecker $callback) + { + $this->defaultPermissions = $callback; + return $this; + } + + /** + * Global permissions required to edit + * + * @param array $permissions + * @return $this + */ + public function setGlobalEditPermissions($permissions) + { + $this->globalEditPermissions = $permissions; + return $this; + } + + /** + * @return array + */ + public function getGlobalEditPermissions() + { + return $this->globalEditPermissions; + } + + /** + * Get root permissions handler, or null if no handler + * + * @return DefaultPermissionChecker|null + */ + public function getDefaultPermissions() + { + return $this->defaultPermissions; + } + + /** + * Get base class + * + * @return string + */ + public function getBaseClass() + { + return $this->baseClass; + } + + /** + * Force pre-calculation of a list of permissions for optimisation + * + * @param string $permission + * @param array $ids + */ + public function prePopulatePermissionCache($permission = 'edit', $ids = []) + { + switch ($permission) { + case self::EDIT: + $this->canEditMultiple($ids, Member::currentUser(), false); + break; + case self::VIEW: + $this->canViewMultiple($ids, Member::currentUser(), false); + break; + case self::DELETE: + $this->canDeleteMultiple($ids, Member::currentUser(), false); + break; + default: + throw new InvalidArgumentException("Invalid permission type $permission"); + } + } + + /** + * This method is NOT a full replacement for the individual can*() methods, e.g. {@link canEdit()}. Rather than + * checking (potentially slow) PHP logic, it relies on the database group associations, e.g. the "CanEditType" field + * plus the "SiteTree_EditorGroups" many-many table. By batch checking multiple records, we can combine the queries + * efficiently. + * + * Caches based on $typeField data. To invalidate the cache, use {@link SiteTree::reset()} or set the $useCached + * property to FALSE. + * + * @param string $type Either edit, view, or create + * @param array $ids Array of IDs + * @param Member $member Member + * @param array $globalPermission If the member doesn't have this permission code, don't bother iterating deeper + * @param bool $useCached Enables use of cache. Cache will be populated even if this is false. + * @return array A map of permissions, keys are ID numbers, and values are boolean permission checks + * ID keys to boolean values + */ + protected function batchPermissionCheck( + $type, + $ids, + Member $member = null, + $globalPermission = [], + $useCached = true + ) { + // Validate ids + $ids = array_filter($ids, 'is_numeric'); + if (empty($ids)) { + return []; + } + + // Default result: nothing editable + $result = array_fill_keys($ids, false); + + // Validate member permission + // Only VIEW allows anonymous (Anyone) permissions + $memberID = $member ? (int)$member->ID : 0; + if (!$memberID && $type !== self::VIEW) { + return $result; + } + + // Look in the cache for values + $cacheKey = "{$type}-{$memberID}"; + if ($useCached && isset($this->cachePermissions[$cacheKey])) { + $cachedValues = array_intersect_key($this->cachePermissions[$cacheKey], $result); + + // If we can't find everything in the cache, then look up the remainder separately + $uncachedIDs = array_keys(array_diff_key($result, $this->cachePermissions[$cacheKey])); + if ($uncachedIDs) { + $uncachedValues = $this->batchPermissionCheck($type, $uncachedIDs, $member, $globalPermission, false); + return $cachedValues + $uncachedValues; + } + return $cachedValues; + } + + // If a member doesn't have a certain permission then they can't edit anything + if ($globalPermission && !Permission::checkMember($member, $globalPermission)) { + return $result; + } + + // Get the groups that the given member belongs to + $groupIDsSQLList = '0'; + if ($memberID) { + $groupIDs = $member->Groups()->column("ID"); + $groupIDsSQLList = implode(", ", $groupIDs) ?: '0'; + } + + // Check if record is versioned + if ($this->isVersioned()) { + // Check all records for each stage and merge + $combinedStageResult = []; + foreach ([ Versioned::DRAFT, Versioned::LIVE ] as $stage) { + $stageRecords = Versioned::get_by_stage($this->getBaseClass(), $stage) + ->byIDs($ids); + // Exclude previously calculated records from later stage calculations + if ($combinedStageResult) { + $stageRecords = $stageRecords->exclude('ID', array_keys($combinedStageResult)); + } + $stageResult = $this->batchPermissionCheckForStage( + $type, + $globalPermission, + $stageRecords, + $groupIDsSQLList, + $member + ); + // Note: Draft stage takes precedence over live, but only if draft exists + $combinedStageResult = $combinedStageResult + $stageResult; + } + } else { + // Unstaged result + $stageRecords = DataObject::get($this->getBaseClass())->byIDs($ids); + $combinedStageResult = $this->batchPermissionCheckForStage( + $type, + $globalPermission, + $stageRecords, + $groupIDsSQLList, + $member + ); + } + + // Cache the results + if (empty($this->cachePermissions[$cacheKey])) { + $this->cachePermissions[$cacheKey] = []; + } + if ($combinedStageResult) { + $this->cachePermissions[$cacheKey] = $combinedStageResult + $this->cachePermissions[$cacheKey]; + } + return $combinedStageResult; + } + + /** + * @param string $type + * @param array $globalPermission List of global permissions + * @param DataList $stageRecords List of records to check for this stage + * @param string $groupIDsSQLList Group IDs this member belongs to + * @param Member $member + * @return array + */ + protected function batchPermissionCheckForStage( + $type, + $globalPermission, + DataList $stageRecords, + $groupIDsSQLList, + Member $member = null + ) { + // Initialise all IDs to false + $result = array_fill_keys($stageRecords->column('ID'), false); + + // Get the uninherited permissions + $typeField = $this->getPermissionField($type); + if ($member && $member->ID) { + // Determine if this member matches any of the group or other rules + $groupJoinTable = $this->getJoinTable($type); + $baseTable = DataObject::getSchema()->baseDataTable($this->getBaseClass()); + $uninheritedPermissions = $stageRecords + ->where([ + "(\"$typeField\" IN (?, ?) OR " . + "(\"$typeField\" = ? AND \"$groupJoinTable\".\"{$baseTable}ID\" IS NOT NULL))" + => [ + self::ANYONE, + self::LOGGED_IN_USERS, + self::ONLY_THESE_USERS + ] + ]) + ->leftJoin( + $groupJoinTable, + "\"$groupJoinTable\".\"{$baseTable}ID\" = \"{$baseTable}\".\"ID\" AND " . + "\"$groupJoinTable\".\"GroupID\" IN ($groupIDsSQLList)" + )->column('ID'); + } else { + // Only view pages with ViewType = Anyone if not logged in + $uninheritedPermissions = $stageRecords + ->filter($typeField, self::ANYONE) + ->column('ID'); + } + + if ($uninheritedPermissions) { + // Set all the relevant items in $result to true + $result = array_fill_keys($uninheritedPermissions, true) + $result; + } + + // Group $potentiallyInherited by ParentID; we'll look at the permission of all those parents and + // then see which ones the user has permission on + $groupedByParent = []; + $potentiallyInherited = $stageRecords->filter($typeField, self::INHERIT); + foreach ($potentiallyInherited as $item) { + /** @var DataObject|Hierarchy $item */ + if ($item->ParentID) { + if (!isset($groupedByParent[$item->ParentID])) { + $groupedByParent[$item->ParentID] = []; + } + $groupedByParent[$item->ParentID][] = $item->ID; + } else { + // Fail over to default permission check for Inherit and ParentID = 0 + $result[$item->ID] = $this->checkDefaultPermissions($type, $member); + } + } + + // Copy permissions from parent to child + if ($groupedByParent) { + $actuallyInherited = $this->batchPermissionCheck( + $type, + array_keys($groupedByParent), + $member, + $globalPermission + ); + if ($actuallyInherited) { + $parentIDs = array_keys(array_filter($actuallyInherited)); + foreach ($parentIDs as $parentID) { + // Set all the relevant items in $result to true + $result = array_fill_keys($groupedByParent[$parentID], true) + $result; + } + } + } + return $result; + } + + public function canEditMultiple($ids, Member $member = null, $useCached = true) + { + return $this->batchPermissionCheck( + self::EDIT, + $ids, + $member, + $this->getGlobalEditPermissions(), + $useCached + ); + } + + public function canViewMultiple($ids, Member $member = null, $useCached = true) + { + return $this->batchPermissionCheck(self::VIEW, $ids, $member, [], $useCached); + } + + public function canDeleteMultiple($ids, Member $member = null, $useCached = true) + { + // Validate ids + $ids = array_filter($ids, 'is_numeric'); + if (empty($ids)) { + return []; + } + $result = array_fill_keys($ids, false); + + // Validate member permission + if (!$member || !$member->ID) { + return $result; + } + $deletable = []; + + // Look in the cache for values + $cacheKey = "delete-{$member->ID}"; + if ($useCached && isset($this->cachePermissions[$cacheKey])) { + $cachedValues = array_intersect_key($this->cachePermissions[$cacheKey], $result); + + // If we can't find everything in the cache, then look up the remainder separately + $uncachedIDs = array_keys(array_diff_key($result, $this->cachePermissions[$cacheKey])); + if ($uncachedIDs) { + $uncachedValues = $this->canDeleteMultiple($uncachedIDs, $member, false); + return $cachedValues + $uncachedValues; + } + return $cachedValues; + } + + // You can only delete pages that you can edit + $editableIDs = array_keys(array_filter($this->canEditMultiple($ids, $member))); + if ($editableIDs) { + // You can only delete pages whose children you can delete + $childRecords = DataObject::get($this->baseClass) + ->filter('ParentID', $editableIDs); + + // Find out the children that can be deleted + $children = $childRecords->map("ID", "ParentID"); + $childIDs = $children->keys(); + if ($childIDs) { + $deletableChildren = $this->canDeleteMultiple($childIDs, $member); + + // Get a list of all the parents that have no undeletable children + $deletableParents = array_fill_keys($editableIDs, true); + foreach ($deletableChildren as $id => $canDelete) { + if (!$canDelete) { + unset($deletableParents[$children[$id]]); + } + } + + // Use that to filter the list of deletable parents that have children + $deletableParents = array_keys($deletableParents); + + // Also get the $ids that don't have children + $parents = array_unique($children->values()); + $deletableLeafNodes = array_diff($editableIDs, $parents); + + // Combine the two + $deletable = array_merge($deletableParents, $deletableLeafNodes); + } else { + $deletable = $editableIDs; + } + } + + // Convert the array of deletable IDs into a map of the original IDs with true/false as the value + return array_fill_keys($deletable, true) + array_fill_keys($ids, false); + } + + public function canDelete($id, Member $member = null) + { + // No ID: Check default permission + if (!$id) { + return $this->checkDefaultPermissions(self::DELETE, $member); + } + + // Regular canEdit logic is handled by canEditMultiple + $results = $this->canDeleteMultiple( + [ $id ], + $member + ); + + // Check if in result + return isset($results[$id]) ? $results[$id] : false; + } + + public function canEdit($id, Member $member = null) + { + // No ID: Check default permission + if (!$id) { + return $this->checkDefaultPermissions(self::EDIT, $member); + } + + // Regular canEdit logic is handled by canEditMultiple + $results = $this->canEditMultiple( + [ $id ], + $member + ); + + // Check if in result + return isset($results[$id]) ? $results[$id] : false; + } + + public function canView($id, Member $member = null) + { + // No ID: Check default permission + if (!$id) { + return $this->checkDefaultPermissions(self::VIEW, $member); + } + + // Regular canView logic is handled by canViewMultiple + $results = $this->canViewMultiple( + [ $id ], + $member + ); + + // Check if in result + return isset($results[$id]) ? $results[$id] : false; + } + + /** + * Get field to check for permission type for the given check. + * Defaults to those provided by {@see InheritedPermissionsExtension) + * + * @param string $type + * @return string + */ + protected function getPermissionField($type) + { + switch ($type) { + case self::DELETE: + // Delete uses edit type - Drop through + case self::EDIT: + return 'CanEditType'; + case self::VIEW: + return 'CanViewType'; + default: + throw new InvalidArgumentException("Invalid argument type $type"); + } + } + + /** + * Get join table for type + * Defaults to those provided by {@see InheritedPermissionsExtension) + * + * @param string $type + * @return string + */ + protected function getJoinTable($type) + { + switch ($type) { + case self::DELETE: + // Delete uses edit type - Drop through + case self::EDIT: + return $this->getEditorGroupsTable(); + case self::VIEW: + return $this->getViewerGroupsTable(); + default: + throw new InvalidArgumentException("Invalid argument type $type"); + } + } + + /** + * Determine default permission for a givion check + * + * @param string $type Method to check + * @param Member $member + * @return bool + */ + protected function checkDefaultPermissions($type, Member $member = null) + { + $defaultPermissions = $this->getDefaultPermissions(); + if (!$defaultPermissions) { + return false; + } + switch ($type) { + case self::VIEW: + return $defaultPermissions->canView($member); + case self::EDIT: + return $defaultPermissions->canEdit($member); + case self::DELETE: + return $defaultPermissions->canDelete($member); + default: + return false; + } + } + + /** + * Check if this model has versioning + * + * @return bool + */ + protected function isVersioned() + { + if (!class_exists(Versioned::class)) { + return false; + } + $singleton = DataObject::singleton($this->getBaseClass()); + return $singleton->hasExtension(Versioned::class); + } + + public function clearCache() + { + $this->cachePermissions = []; + return $this; + } + + /** + * Get table to use for editor groups relation + * + * @return string + */ + protected function getEditorGroupsTable() + { + $table = DataObject::getSchema()->tableName($this->baseClass); + return "{$table}_EditorGroups"; + } + + /** + * Get table to use for viewer groups relation + * + * @return string + */ + protected function getViewerGroupsTable() + { + $table = DataObject::getSchema()->tableName($this->baseClass); + return "{$table}_ViewerGroups"; + } +} diff --git a/src/Security/InheritedPermissionsExtension.php b/src/Security/InheritedPermissionsExtension.php new file mode 100644 index 000000000..88f6c93a6 --- /dev/null +++ b/src/Security/InheritedPermissionsExtension.php @@ -0,0 +1,32 @@ + "Enum('Anyone, LoggedInUsers, OnlyTheseUsers, Inherit', 'Inherit')", + 'CanEditType' => "Enum('LoggedInUsers, OnlyTheseUsers, Inherit', 'Inherit')", + ]; + + private static $many_many = [ + 'ViewerGroups' => Group::class, + 'EditorGroups' => Group::class, + ]; + + private static $defaults = [ + 'CanViewType' => InheritedPermissions::INHERIT, + 'CanEditType' => InheritedPermissions::INHERIT, + ]; +} diff --git a/src/Security/Member.php b/src/Security/Member.php index e62ba18e7..6dba32a06 100644 --- a/src/Security/Member.php +++ b/src/Security/Member.php @@ -1638,7 +1638,7 @@ class Member extends DataObject implements TemplateGlobalProvider public function canView($member = null) { //get member - if (!($member instanceof Member)) { + if (!$member) { $member = Member::currentUser(); } //check for extensions, we do this first as they can overrule everything @@ -1669,7 +1669,7 @@ class Member extends DataObject implements TemplateGlobalProvider public function canEdit($member = null) { //get member - if (!($member instanceof Member)) { + if (!$member) { $member = Member::currentUser(); } //check for extensions, we do this first as they can overrule everything @@ -1703,7 +1703,7 @@ class Member extends DataObject implements TemplateGlobalProvider */ public function canDelete($member = null) { - if (!($member instanceof Member)) { + if (!$member) { $member = Member::currentUser(); } //check for extensions, we do this first as they can overrule everything diff --git a/src/Security/Permission.php b/src/Security/Permission.php index 8c1b7f895..8d5a663d9 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -33,7 +33,7 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl ); private static $has_one = array( - "Group" => "SilverStripe\\Security\\Group" + "Group" => Group::class, ); private static $indexes = array( diff --git a/src/Security/PermissionChecker.php b/src/Security/PermissionChecker.php new file mode 100644 index 000000000..0c1e0b999 --- /dev/null +++ b/src/Security/PermissionChecker.php @@ -0,0 +1,68 @@ +fixStepArgument($name); + $property = $this->fixStepArgument($property); + + $context = $this->getMainContext(); + $fieldObj = $context->assertSession()->fieldExists($name); + + // Check property + $hasProperty = $fieldObj->hasAttribute($property); + switch ($cond) { + case 'has': + assert($hasProperty, "Field $name does not have property $property"); + break; + case 'does not have': + assert(!$hasProperty, "Field $name should not have property $property"); + break; + default: + throw new BadMethodCallException("Invalid condition"); + } + } } diff --git a/tests/php/Security/InheritedPermissionsTest.php b/tests/php/Security/InheritedPermissionsTest.php new file mode 100644 index 000000000..0d4cbc7f6 --- /dev/null +++ b/tests/php/Security/InheritedPermissionsTest.php @@ -0,0 +1,269 @@ +setGlobalEditPermissions(['TEST_NODE_ACCESS']) + ->setDefaultPermissions($this->rootPermissions = new TestDefaultPermissionChecker()); + Injector::inst()->registerService( + $permission, + PermissionChecker::class.'.testpermissions' + ); + + // Reset root permission + } + + public function testEditPermissions() + { + $editor = $this->objFromFixture(Member::class, 'editor'); + + $about = $this->objFromFixture(TestPermissionNode::class, 'about'); + $aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff'); + $history = $this->objFromFixture(TestPermissionNode::class, 'history'); + $products = $this->objFromFixture(TestPermissionNode::class, 'products'); + $product1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1'); + $product4 = $this->objFromFixture(TestPermissionNode::class, 'products-product4'); + + // Test logged out users cannot edit + Member::actAs(null, function () use ($aboutStaff) { + $this->assertFalse($aboutStaff->canEdit()); + }); + + // Can't edit a page that is locked to admins + $this->assertFalse($about->canEdit($editor)); + + // Can edit a page that is locked to editors + $this->assertTrue($products->canEdit($editor)); + + // Can edit a child of that page that inherits + $this->assertTrue($product1->canEdit($editor)); + + // Can't edit a child of that page that has its permissions overridden + $this->assertFalse($product4->canEdit($editor)); + + // Test that root node respects root permissions + $this->assertTrue($history->canEdit($editor)); + + TestPermissionNode::getInheritedPermissions()->clearCache(); + $this->rootPermissions->setCanEdit(false); + + // With root edit false, permissions are now denied for CanEditType = Inherit + $this->assertFalse($history->canEdit($editor)); + } + + public function testDeletePermissions() + { + $editor = $this->objFromFixture(Member::class, 'editor'); + + $about = $this->objFromFixture(TestPermissionNode::class, 'about'); + $aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff'); + $history = $this->objFromFixture(TestPermissionNode::class, 'history'); + $products = $this->objFromFixture(TestPermissionNode::class, 'products'); + $product1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1'); + $product4 = $this->objFromFixture(TestPermissionNode::class, 'products-product4'); + + // Test logged out users cannot edit + Member::actAs(null, function () use ($aboutStaff) { + $this->assertFalse($aboutStaff->canDelete()); + }); + + // Can't edit a page that is locked to admins + $this->assertFalse($about->canDelete($editor)); + + // Can't delete a page if a child (product4) is un-deletable + $this->assertFalse($products->canDelete($editor)); + + // Can edit a child of that page that inherits + $this->assertTrue($product1->canDelete($editor)); + + // Can't edit a child of that page that has its permissions overridden + $this->assertFalse($product4->canDelete($editor)); + + // Test that root node respects root permissions + $this->assertTrue($history->canDelete($editor)); + + TestPermissionNode::getInheritedPermissions()->clearCache(); + $this->rootPermissions->setCanEdit(false); + + // With root edit false, permissions are now denied for CanEditType = Inherit + $this->assertFalse($history->canDelete($editor)); + } + + public function testViewPermissions() + { + $history = $this->objFromFixture(TestPermissionNode::class, 'history'); + $contact = $this->objFromFixture(TestPermissionNode::class, 'contact'); + $contactForm = $this->objFromFixture(TestPermissionNode::class, 'contact-form'); + $secret = $this->objFromFixture(TestPermissionNode::class, 'secret'); + $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'); + + // Not logged in user can only access Inherit or Anyone pages + Member::actAs( + null, + function () use ($protectedChild, $secretNested, $protected, $secret, $history, $contact, $contactForm) { + $this->assertTrue($history->canView()); + $this->assertTrue($contact->canView()); + $this->assertTrue($contactForm->canView()); + // Protected + $this->assertFalse($secret->canView()); + $this->assertFalse($secretNested->canView()); + $this->assertFalse($protected->canView()); + $this->assertFalse($protectedChild->canView()); + } + ); + + // Editor can view pages restricted to logged in users + $this->assertTrue($secret->canView($editor)); + $this->assertTrue($secretNested->canView($editor)); + + // Cannot read admin-only pages + $this->assertFalse($protected->canView($editor)); + $this->assertFalse($protectedChild->canView($editor)); + + // Check root permissions + $this->assertTrue($history->canView($editor)); + + TestPermissionNode::getInheritedPermissions()->clearCache(); + $this->rootPermissions->setCanView(false); + + $this->assertFalse($history->canView($editor)); + } + + /** + * Test that draft permissions deny unrestricted live permissions + */ + public function testRestrictedDraftUnrestrictedLive() + { + Versioned::set_stage(Versioned::DRAFT); + + // Should be editable by non-admin editor + /** @var TestPermissionNode $products */ + $products = $this->objFromFixture(TestPermissionNode::class, 'products'); + /** @var TestPermissionNode $products1 */ + $products1 = $this->objFromFixture(TestPermissionNode::class, 'products-product1'); + $editor = $this->objFromFixture(Member::class, 'editor'); + + // Ensure the editor can edit + $this->assertTrue($products->canEdit($editor)); + $this->assertTrue($products1->canEdit($editor)); + + // Write current version to live + $products->writeToStage(Versioned::LIVE); + $products1->writeToStage(Versioned::LIVE); + + // Draft version restrict to admins + $products->EditorGroups()->setByIDList([ + $this->idFromFixture(Group::class, 'admins') + ]); + $products->write(); + + // Ensure editor can no longer edit + TestPermissionNode::getInheritedPermissions()->clearCache(); + $this->assertFalse($products->canEdit($editor)); + $this->assertFalse($products1->canEdit($editor)); + } + + /** + * Test that draft permissions permit access over live permissions + */ + public function testUnrestrictedDraftOverridesLive() + { + Versioned::set_stage(Versioned::DRAFT); + + // Should be editable by non-admin editor + /** @var TestPermissionNode $about */ + $about = $this->objFromFixture(TestPermissionNode::class, 'about'); + /** @var TestPermissionNode $aboutStaff */ + $aboutStaff = $this->objFromFixture(TestPermissionNode::class, 'about-staff'); + $editor = $this->objFromFixture(Member::class, 'editor'); + + // Ensure the editor can't edit + $this->assertFalse($about->canEdit($editor)); + $this->assertFalse($aboutStaff->canEdit($editor)); + + // Write current version to live + $about->writeToStage(Versioned::LIVE); + $aboutStaff->writeToStage(Versioned::LIVE); + + // Unrestrict draft + $about->CanEditType = InheritedPermissions::LOGGED_IN_USERS; + $about->write(); + + // Ensure editor can no longer edit + TestPermissionNode::getInheritedPermissions()->clearCache(); + $this->assertTrue($about->canEdit($editor)); + $this->assertTrue($aboutStaff->canEdit($editor)); + } + + /** + * Ensure that flipping parent / child relationship on live doesn't + * cause infinite loop + */ + public function testMobiusHierarchy() + { + Versioned::set_stage(Versioned::DRAFT); + + /** @var TestPermissionNode $history */ + $history = $this->objFromFixture(TestPermissionNode::class, 'history'); + /** @var TestPermissionNode $historyGallery */ + $historyGallery = $this->objFromFixture(TestPermissionNode::class, 'history-gallery'); + + // Publish current state to live + $history->writeToStage(Versioned::LIVE); + $historyGallery->writeToStage(Versioned::LIVE); + + // Flip relation + $historyGallery->ParentID = 0; + $historyGallery->write(); + $history->ParentID = $historyGallery->ID; + $history->write(); + + // Test viewability (not logged in users) + Member::actAs(null, function () use ($history, $historyGallery) { + $this->assertTrue($history->canView()); + $this->assertTrue($historyGallery->canView()); + }); + + // Change permission on draft root and ensure it affects both + $historyGallery->CanViewType = InheritedPermissions::LOGGED_IN_USERS; + $historyGallery->write(); + TestPermissionNode::getInheritedPermissions()->clearCache(); + + // Test viewability (not logged in users) + Member::actAs(null, function () use ($history, $historyGallery) { + $this->assertFalse($historyGallery->canView()); + $this->assertFalse($history->canView()); + }); + } +} diff --git a/tests/php/Security/InheritedPermissionsTest.yml b/tests/php/Security/InheritedPermissionsTest.yml new file mode 100644 index 000000000..9e8ebb625 --- /dev/null +++ b/tests/php/Security/InheritedPermissionsTest.yml @@ -0,0 +1,102 @@ +SilverStripe\Security\Group: + editors: + Title: Editors + admins: + Title: Administrators + allsections: + Title: All Section Editors + securityadmins: + Title: Security Admins + +SilverStripe\Security\Permission: + admins: + Code: ADMIN + Group: =>SilverStripe\Security\Group.admins + editors: + Code: CMS_ACCESS_CMSMain + Group: =>SilverStripe\Security\Group.editors + testpermission: + Code: TEST_NODE_ACCESS + Group: =>SilverStripe\Security\Group.editors + + +SilverStripe\Security\Member: + editor: + FirstName: Test + Surname: Editor + Groups: =>SilverStripe\Security\Group.editors + admin: + FirstName: Test + Surname: Administrator + Groups: =>SilverStripe\Security\Group.admins + allsections: + Groups: =>SilverStripe\Security\Group.allsections + securityadmin: + Groups: =>SilverStripe\Security\Group.securityadmins + +SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode: + about: + Title: About Us + CanEditType: OnlyTheseUsers + EditorGroups: =>SilverStripe\Security\Group.admins + about-staff: + Title: Staff + CanEditType: Inherit + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.about + about-staff-ceo: + Title: CEO + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.about-staff + about-staffduplicate: + Title: Staff + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.about + products: + Title: Products + CanEditType: OnlyTheseUsers + EditorGroups: =>SilverStripe\Security\Group.editors + products-product1: + Title: 1.1 Test Product + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.products + CanEditType: Inherit + products-product2: + Title: Another Product + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.products + CanEditType: Inherit + products-product3: + Title: Another Product + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.products + CanEditType: Inherit + products-product4: + Title: Another Product + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.products + CanEditType: OnlyTheseUsers + EditorGroups: =>SilverStripe\Security\Group.admins + history: + Title: History + CanViewType: Inherit + CanEditType: Inherit + history-gallery: + Title: Gallery + CanViewType: Inherit + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.history + contact: + Title: Contact Us + CanViewType: Anyone + contact-form: + Title: Send us a message + CanViewType: Inherit + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.contact + secret: + Title: Secret + CanViewType: LoggedInUsers + secret-nested: + Title: Nested + CanViewType: Inherit + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.secret + protected: + Title: Protected + CanViewType: OnlyTheseUsers + ViewerGroups: =>SilverStripe\Security\Group.admins + protected-child: + Title: Child + CanViewType: Inherit + Parent: =>SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode.protected diff --git a/tests/php/Security/InheritedPermissionsTest/TestDefaultPermissionChecker.php b/tests/php/Security/InheritedPermissionsTest/TestDefaultPermissionChecker.php new file mode 100644 index 000000000..d7c13747d --- /dev/null +++ b/tests/php/Security/InheritedPermissionsTest/TestDefaultPermissionChecker.php @@ -0,0 +1,69 @@ +canEdit; + } + + /** + * Can root be viewed? + * + * @param Member $member + * @return bool + */ + public function canView(Member $member = null) + { + return $this->canView; + } + + /** + * Can root be deleted? + * + * @param Member $member + * @return bool + */ + public function canDelete(Member $member = null) + { + return $this->canEdit; + } + + /** + * Can root objects be created? + * + * @param Member $member + * @return bool + */ + public function canCreate(Member $member = null) + { + return $this->canEdit; + } + + public function setCanEdit($canEdit) + { + $this->canEdit = $canEdit; + return $this; + } + + public function setCanView($canView) + { + $this->canView = $canView; + return $this; + } +} diff --git a/tests/php/Security/InheritedPermissionsTest/TestPermissionNode.php b/tests/php/Security/InheritedPermissionsTest/TestPermissionNode.php new file mode 100644 index 000000000..8a311836b --- /dev/null +++ b/tests/php/Security/InheritedPermissionsTest/TestPermissionNode.php @@ -0,0 +1,68 @@ + "Varchar(255)", + ]; + + private static $has_one = [ + "Parent" => self::class, + ]; + + private static $table_name = 'InheritedPermissionsTest_TestPermissionNode'; + + private static $extensions = [ + Versioned::class, + InheritedPermissionsExtension::class, + ]; + + /** + * @return InheritedPermissions + */ + public static function getInheritedPermissions() + { + /** @var InheritedPermissions $permissions */ + return Injector::inst()->get(PermissionChecker::class.'.testpermissions'); + } + + public function canEdit($member = null) + { + if (!$member) { + $member = Member::currentUser(); + } + return static::getInheritedPermissions()->canEdit($this->ID, $member); + } + + public function canView($member = null) + { + if (!$member) { + $member = Member::currentUser(); + } + return static::getInheritedPermissions()->canView($this->ID, $member); + } + + public function canDelete($member = null) + { + if (!$member) { + $member = Member::currentUser(); + } + return static::getInheritedPermissions()->canDelete($this->ID, $member); + } +}