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

@@ -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;
@@ -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) {
+	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'))
+		Versioned::set_reading_mode($oldMode);
-		) return false;
+		return $versionFromStage ? $versionFromStage->canView($member) : false;
-		
-		return $this->canView($member);
 	}
 
 	/**

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

@@ -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,