From a5f00ae2c3bc6dd6aa27d56b530b75408539b067 Mon Sep 17 00:00:00 2001 From: Hamish Friedlander Date: Thu, 4 Jul 2013 14:54:13 +1200 Subject: [PATCH] FIX Not checking stage in SiteTree#canView SiteTree versions that arent the live version shouldnt be accessed by regular users, but the logic to check that was split off into canViewStage, which wasnt checked by code that isnt specifically SiteTree aware (like RestfulServer) --- code/controllers/ContentController.php | 23 ++++++----- code/model/SiteTree.php | 39 +++++++++++++------ .../ContentControllerPermissionsTest.php | 7 +++- tests/model/SiteTreePermissionsTest.php | 11 ++++++ tests/search/SearchFormTest.php | 20 ++++++---- 5 files changed, 71 insertions(+), 29 deletions(-) diff --git a/code/controllers/ContentController.php b/code/controllers/ContentController.php index 2cd3f418..8f8e8afc 100644 --- a/code/controllers/ContentController.php +++ b/code/controllers/ContentController.php @@ -102,19 +102,24 @@ class ContentController extends Controller { // Check page permissions if($this->dataRecord && $this->URLSegment != 'Security' && !$this->dataRecord->canView()) { - return Security::permissionFailure($this); - } + $permissionMessage = null; - // Draft/Archive security check - only CMS users should be able to look at stage/archived content - if($this->URLSegment != 'Security' && !Session::get('unsecuredDraftSite') && (Versioned::current_archived_date() || (Versioned::current_stage() && Versioned::current_stage() != 'Live'))) { - if(!$this->dataRecord->canViewStage(Versioned::current_stage())) { - $link = $this->Link(); - $message = _t("ContentController.DRAFT_SITE_ACCESS_RESTRICTION", 'You must log in with your CMS password in order to view the draft or archived content. Click here to go back to the published site.'); + // Check if we could view the live version, offer redirect if so + if($this->canViewStage('Live')) { Session::clear('currentStage'); Session::clear('archiveDate'); - - return Security::permissionFailure($this, sprintf($message, Controller::join_links($link, "?stage=Live"))); + + $permissionMessage = sprintf( + _t( + "ContentController.DRAFT_SITE_ACCESS_RESTRICTION", + 'You must log in with your CMS password in order to view the draft or archived content. '. + 'Click here to go back to the published site.' + ), + Controller::join_links($this->Link(), "?stage=Live") + ); } + + return Security::permissionFailure($this, $permissionMessage); } // Use theme from the site config diff --git a/code/model/SiteTree.php b/code/model/SiteTree.php index dc6029e6..54eb6f4b 100644 --- a/code/model/SiteTree.php +++ b/code/model/SiteTree.php @@ -830,6 +830,23 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid // admin override if($member && Permission::checkMember($member, array("ADMIN", "SITETREE_VIEW_ALL"))) return true; + // make sure we were loaded off an allowed stage + + // Were we definitely loaded directly off Live during our query? + $fromLive = true; + + foreach (array('mode' => 'stage', 'stage' => 'live') as $param => $match) { + $fromLive = $fromLive && strtolower((string)$this->getSourceQueryParam("Versioned.$param")) == $match; + } + + if(!$fromLive + && !Session::get('unsecuredDraftSite') + && !Permission::checkMember($member, array('CMS_ACCESS_CMSMain', 'VIEW_DRAFT_CONTENT'))) { + // If we weren't definitely loaded from live, and we can't view non-live content, we need to + // check to make sure this version is the live version and so can be viewed + if (Versioned::get_versionnumber_by_stage($this->class, 'Live', $this->ID) != $this->Version) return false; + } + // Standard mechanism for accepting permission changes from extensions $extended = $this->extendedCan('canView', $member); if($extended !== null) return $extended; @@ -858,27 +875,25 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid return false; } - + /** - * Determines permissions for a specific stage (see {@link Versioned}). + * Determines canView permissions for the latest version of this Page on a specific stage (see {@link Versioned}). * Usually the stage is read from {@link Versioned::current_stage()}. - * Falls back to {@link canView}. - * + * * @todo Implement in CMS UI. * * @param String $stage * @param Member $member * @return boolean */ - public function canViewStage($stage, $member = null) { - if(!$member) $member = Member::currentUser(); + public function canViewStage($stage = 'Live', $member = null) { + $oldMode = Versioned::get_reading_mode(); + Versioned::reading_stage($stage); - if( - strtolower($stage) == 'stage' && - !(Permission::checkMember($member, 'CMS_ACCESS_CMSMain') || Permission::checkMember($member, 'VIEW_DRAFT_CONTENT')) - ) return false; - - return $this->canView($member); + $versionFromStage = DataObject::get($this->class)->byID($this->ID); + + Versioned::set_reading_mode($oldMode); + return $versionFromStage ? $versionFromStage->canView($member) : false; } /** diff --git a/tests/controller/ContentControllerPermissionsTest.php b/tests/controller/ContentControllerPermissionsTest.php index e8ce8eb0..189da4af 100644 --- a/tests/controller/ContentControllerPermissionsTest.php +++ b/tests/controller/ContentControllerPermissionsTest.php @@ -10,11 +10,16 @@ class ContentControllerPermissionsTest extends FunctionalTest { protected $autoFollowRedirection = false; public function testCanViewStage() { + // Create a new page $page = new Page(); $page->URLSegment = 'testpage'; $page->write(); $page->publish('Stage', 'Live'); - + + // Add a stage-only version + $page->Content = "Version2"; + $page->write(); + $response = $this->get('/testpage'); $this->assertEquals($response->getStatusCode(), 200, 'Doesnt require login for implicit live stage'); diff --git a/tests/model/SiteTreePermissionsTest.php b/tests/model/SiteTreePermissionsTest.php index 93731aff..c890ade3 100644 --- a/tests/model/SiteTreePermissionsTest.php +++ b/tests/model/SiteTreePermissionsTest.php @@ -126,7 +126,16 @@ class SiteTreePermissionsTest extends FunctionalTest { } public function testCanViewStage() { + $this->useDraftSite(false); // useDraftSite deliberately disables checking the stage as part of canView + + // Get page & make sure it exists on Live $page = $this->objFromFixture('Page', 'standardpage'); + $page->publish('Stage', 'Live'); + + // Then make sure there's a new version on Stage + $page->Title = 1; + $page->write(); + $editor = $this->objFromFixture('Member', 'editor'); $websiteuser = $this->objFromFixture('Member', 'websiteuser'); @@ -135,6 +144,8 @@ class SiteTreePermissionsTest extends FunctionalTest { $this->assertTrue($page->canViewStage('Live', $editor)); $this->assertTrue($page->canViewStage('Stage', $editor)); + + $this->useDraftSite(); } public function testAccessTabOnlyDisplaysWithGrantAccessPermissions() { diff --git a/tests/search/SearchFormTest.php b/tests/search/SearchFormTest.php index bb59b214..f060842b 100644 --- a/tests/search/SearchFormTest.php +++ b/tests/search/SearchFormTest.php @@ -11,7 +11,7 @@ class ZZZSearchFormTest extends FunctionalTest { protected static $fixture_file = 'SearchFormTest.yml'; - + protected $mockController; public function waitUntilIndexingFinished() { @@ -88,7 +88,6 @@ class ZZZSearchFormTest extends FunctionalTest { ); } - /* public function testUnpublishedPagesNotIncluded() { if(!$this->checkFulltextSupport()) return; @@ -102,14 +101,15 @@ class ZZZSearchFormTest extends FunctionalTest { 'Unpublished pages are not found by searchform' ); } - */ - + public function testPagesRestrictedToLoggedinUsersNotIncluded() { if(!$this->checkFulltextSupport()) return; $sf = new SearchForm($this->mockController, 'SearchForm'); $page = $this->objFromFixture('SiteTree', 'restrictedViewLoggedInUsers'); + $page->publish('Stage', 'Live'); + $results = $sf->getResults(null, array('Search'=>'restrictedViewLoggedInUsers')); $this->assertNotContains( $page->ID, @@ -134,6 +134,8 @@ class ZZZSearchFormTest extends FunctionalTest { $sf = new SearchForm($this->mockController, 'SearchForm'); $page = $this->objFromFixture('SiteTree', 'restrictedViewOnlyWebsiteUsers'); + $page->publish('Stage', 'Live'); + $results = $sf->getResults(null, array('Search'=>'restrictedViewOnlyWebsiteUsers')); $this->assertNotContains( $page->ID, @@ -162,13 +164,17 @@ class ZZZSearchFormTest extends FunctionalTest { $member->logOut(); } - public function testInheritedRestrictedPagesNotInlucded() { + public function testInheritedRestrictedPagesNotIncluded() { if(!$this->checkFulltextSupport()) return; $sf = new SearchForm($this->mockController, 'SearchForm'); - + + $parent = $this->objFromFixture('SiteTree', 'restrictedViewLoggedInUsers'); + $parent->publish('Stage', 'Live'); + $page = $this->objFromFixture('SiteTree', 'inheritRestrictedView'); - + $page->publish('Stage', 'Live'); + $results = $sf->getResults(null, array('Search'=>'inheritRestrictedView')); $this->assertNotContains( $page->ID,