Merge pull request #792 from hafriedlander/fix/stageaccess

FIX Not checking stage in SiteTree#canView

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
+			// Check if we could view the live version, offer redirect if so
-		if($this->URLSegment != 'Security' && !Session::get('unsecuredDraftSite') && (Versioned::current_archived_date() || (Versioned::current_stage() && Versioned::current_stage() != 'Live'))) {
+			if($this->canViewStage('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.  <a href="%s">Click here to go back to the published site.</a>');
 				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. '.
+						'<a href="%s">Click here to go back to the published site.</a>'
+					),
+					Controller::join_links($this->Link(), "?stage=Live")
+				);
 			}
+
+			return Security::permissionFailure($this, $permissionMessage);
 		}
 		
 		// Use theme from the site config

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;
@@ -860,9 +877,8 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 	}
 
 	/**
-	 * 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.
 	 * 
@@ -870,15 +886,14 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 	 * @param Member $member
 	 * @return boolean
 	 */
-	public function canViewStage($stage, $member = null) {
+	public function canViewStage($stage = 'Live', $member = null) {
-		if(!$member) $member = Member::currentUser();
+		$oldMode = Versioned::get_reading_mode();
+		Versioned::reading_stage($stage);
 
-		if(
+		$versionFromStage = DataObject::get($this->class)->byID($this->ID);
-			strtolower($stage) == 'stage' && 
-			!(Permission::checkMember($member, 'CMS_ACCESS_CMSMain') || Permission::checkMember($member, 'VIEW_DRAFT_CONTENT'))
-		) return false;
 
-		return $this->canView($member);
+		Versioned::set_reading_mode($oldMode);
+		return $versionFromStage ? $versionFromStage->canView($member) : false;
 	}
 
 	/**

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');
 		

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() {

tests/search/SearchFormTest.php

@@ -88,7 +88,6 @@ class ZZZSearchFormTest extends FunctionalTest {
 		);
 	}
 	
-	/*
 	public function testUnpublishedPagesNotIncluded() {
 		if(!$this->checkFulltextSupport()) return;
 
@@ -102,7 +101,6 @@ class ZZZSearchFormTest extends FunctionalTest {
 			'Unpublished pages are not found by searchform'
 		);
 	}
-	*/
 
 	public function testPagesRestrictedToLoggedinUsersNotIncluded() {
 		if(!$this->checkFulltextSupport()) return;
@@ -110,6 +108,8 @@ class ZZZSearchFormTest extends FunctionalTest {
 		$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,12 +164,16 @@ 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(