FIX Issue where CMS SiteTree can result in infinite recursion if parent and child relation is swapped

This commit is contained in:
Daniel Hensby 2017-05-30 16:34:39 +01:00
parent 14a855ed97
commit 5116476875
No known key found for this signature in database
GPG Key ID: B00D1E9767F0B06E
2 changed files with 48 additions and 5 deletions

View File

@ -1158,10 +1158,11 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
* @param string $siteConfigMethod Method to call on {@link SiteConfig} for toplevel items, e.g. "canEdit"
* @param string $globalPermission If the member doesn't have this permission code, don't bother iterating deeper
* @param bool $useCached
* @param array $stages Which stages to check permissions against, defaults to both Stage and Live
* @return array An map of {@link SiteTree} ID keys to boolean values
*/
public static function batch_permission_check($ids, $memberID, $typeField, $groupJoinTable, $siteConfigMethod,
$globalPermission = null, $useCached = true) {
$globalPermission = null, $useCached = true, $stages = array('Stage', 'Live')) {
if($globalPermission === NULL) $globalPermission = array('CMS_ACCESS_LeftAndMain', 'CMS_ACCESS_CMSMain');
// Sanitise the IDs
@ -1169,7 +1170,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
// This is the name used on the permission cache
// converts something like 'CanEditType' to 'edit'.
$cacheKey = strtolower(substr($typeField, 3, -4)) . "-$memberID";
$cacheKey = strtolower(substr($typeField, 3, -4)) . "-$memberID" . implode('-', $stages);
// Default result: nothing editable
$result = array_fill_keys($ids, false);
@ -1206,7 +1207,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
$combinedStageResult = array();
foreach(array('Stage', 'Live') as $stage) {
foreach($stages as $stage) {
// Start by filling the array with the pages that actually exist
$table = ($stage=='Stage') ? "SiteTree" : "SiteTree_$stage";
@ -1257,7 +1258,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
}
if($groupedByParent) {
$actuallyInherited = self::batch_permission_check(array_keys($groupedByParent), $memberID, $typeField, $groupJoinTable, $siteConfigMethod);
$actuallyInherited = self::batch_permission_check(array_keys($groupedByParent), $memberID, $typeField, $groupJoinTable, $siteConfigMethod, $globalPermission, $useCached, array($stage));
if($actuallyInherited) {
$parentIDs = array_keys(array_filter($actuallyInherited));
foreach($parentIDs as $parentID) {

View File

@ -445,5 +445,47 @@ class SiteTreePermissionsTest extends FunctionalTest {
$this->session()->inst_set('loggedInAs', $user->ID);
$this->assertFalse($page->canEdit($user), 'Website user can\'t edit a page when set to inherit from the SiteConfig, and SiteConfig has canEdit set to OnlyTheseUsers');
}
/**
* Ensure that flipping parent / child relationship on live doesn't
* cause infinite loop
*/
public function testMobiusHierarchy()
{
// login as admin to be able to publish
$adminID = $this->logInWithPermission('ADMIN');
$parentPage = new Page();
$parentPage->Title = 'Parent Page';
$parentPage->doPublish();
$childPage = new Page();
$childPage->Title = 'Child Page';
$childPage->ParentID = $parentPage->ID;
$childPage->doPublish();
//flip parent/child relation
$childPage->ParentID = 0;
$childPage->write();
$parentPage->ParentID = $childPage->ID;
$parentPage->write();
$this->assertTrue($childPage->isPublished());
$this->assertTrue($parentPage->isPublished());
$result = SiteTree::batch_permission_check(array(
$parentPage->ID,
$childPage->ID,
), $adminID, 'CanEditType', 'SiteTree_EditorGroups', 'canEditPages');
$this->assertArrayHasKey($childPage->ID, $result);
$this->assertArrayHasKey($parentPage->ID, $result);
// mr potato head can't edit
$this->assertTrue($result[$parentPage->ID]);
$this->assertTrue($result[$childPage->ID]);
}
}