From b10f8a908d381daddcdbf766f0a67a3981eda156 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Tue, 12 Jan 2010 23:35:33 +0000 Subject: [PATCH] BUGFIX: Fixed notice-level errors when checking permissions of pages that don't exist anywhere (from r93166) git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@96755 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- core/model/SiteTree.php | 10 ++++- forms/HtmlEditorField.php | 2 + tests/SiteTreePermissionsTest.php | 69 ++++++++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/core/model/SiteTree.php b/core/model/SiteTree.php index 7c7602ceb..02ad8ffd6 100755 --- a/core/model/SiteTree.php +++ b/core/model/SiteTree.php @@ -788,7 +788,10 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid // Regular canEdit logic is handled by can_edit_multiple $results = self::can_delete_multiple(array($this->ID), $memberID); - return $results[$this->ID]; + + // If this page no longer exists in stage/live results won't contain the page. + // Fail-over to false + return isset($results[$this->ID]) ? $results[$this->ID] : false; } /** @@ -863,7 +866,9 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid // Regular canEdit logic is handled by can_edit_multiple $results = self::can_edit_multiple(array($this->ID), $memberID); - return $results[$this->ID]; + // If this page no longer exists in stage/live results won't contain the page. + // Fail-over to false + return isset($results[$this->ID]) ? $results[$this->ID] : false; // Default for unsaved pages } else { @@ -980,6 +985,7 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid foreach(array('Stage', 'Live') as $stage) { // Start by filling the array with the pages that actually exist $table = ($stage=='Stage') ? "SiteTree" : "SiteTree_$stage"; + $result = array_fill_keys(DB::query("SELECT \"ID\" FROM \"$table\" WHERE \"ID\" IN (".implode(", ", $ids).")")->column(), false); diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php index bf5788cde..a835fd126 100755 --- a/forms/HtmlEditorField.php +++ b/forms/HtmlEditorField.php @@ -341,6 +341,8 @@ class HtmlEditorField_Toolbar extends RequestHandler { new FormAction("insertflash", _t('HtmlEditorField.BUTTONINSERTFLASH', 'Insert Flash')) ) ); + + $form->disableSecurityToken(); $form->loadDataFrom($this); $form->disableSecurityToken(); return $form; diff --git a/tests/SiteTreePermissionsTest.php b/tests/SiteTreePermissionsTest.php index 5fee6a154..18e9f41d6 100755 --- a/tests/SiteTreePermissionsTest.php +++ b/tests/SiteTreePermissionsTest.php @@ -29,7 +29,74 @@ class SiteTreePermissionsTest extends FunctionalTest { // we're testing HTTP status codes before being redirected to login forms $this->autoFollowRedirection = false; } + + function testPermissionCheckingWorksOnDeletedPages() { + // Set up fixture - a published page deleted from draft + $this->logInWithPermssion("ADMIN"); + $page = $this->objFromFixture('Page','restrictedEditOnlySubadminGroup'); + $pageID = $page->ID; + $this->assertTrue($page->doPublish()); + $page->delete(); + + // Re-fetch the page from the live site + $page = Versioned::get_one_by_stage('SiteTree', 'Live', "\"SiteTree\".\"ID\" = $pageID"); + + // subadmin has edit rights on that page + $member = $this->objFromFixture('Member','subadmin'); + $member->logIn(); + + // Test can_edit_multiple + $this->assertEquals( + array($pageID => true), + SiteTree::can_edit_multiple(array($pageID), $member->ID) + ); + + // Test canEdit + $member->logIn(); + $this->assertTrue($page->canEdit()); + } + function testPermissionCheckingWorksOnUnpublishedPages() { + // Set up fixture - an unpublished page + $this->logInWithPermssion("ADMIN"); + $page = $this->objFromFixture('Page','restrictedEditOnlySubadminGroup'); + $pageID = $page->ID; + $page->doUnpublish(); + + // subadmin has edit rights on that page + $member = $this->objFromFixture('Member','subadmin'); + $member->logIn(); + + // Test can_edit_multiple + $this->assertEquals( + array($pageID => true), + SiteTree::can_edit_multiple(array($pageID), $member->ID) + ); + + // Test canEdit + $member->logIn(); + $this->assertTrue($page->canEdit()); + } + + function testCanEditOnPageDeletedFromStageAndLiveReturnsFalse() { + // Find a page that exists and delete it from both stage and published + $this->logInWithPermssion("ADMIN"); + $page = $this->objFromFixture('Page','restrictedEditOnlySubadminGroup'); + $pageID = $page->ID; + $page->doUnpublish(); + $page->delete(); + + // We'll need to resurrect the page from the version cache to test this case + $page = Versioned::get_latest_version('SiteTree', $pageID); + + // subadmin had edit rights on that page, but now it's gone + $member = $this->objFromFixture('Member','subadmin'); + $member->logIn(); + + $this->assertFalse($page->canEdit()); + } + +/* function testAccessTabOnlyDisplaysWithGrantAccessPermissions() { $page = $this->objFromFixture('Page', 'standardpage'); @@ -324,7 +391,7 @@ 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'); } - +*/ function testInheritCanViewFromSiteConfig() { $page = $this->objFromFixture('Page', 'inheritWithNoParent'); $siteconfig = $this->objFromFixture('SiteConfig', 'default');