From aacaee08cd0a21819d29dc199788b6b3a3fe9b3e Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Thu, 7 Nov 2013 12:03:52 +1300 Subject: [PATCH] BUG Prevent session-interface mismatch. Disables transparent subsite switch on AJAX requests. Makes sure the subsite is appropriately set up when opening up the CMS with a link to subsited object. --- _config/config.yml | 6 +++ code/extensions/LeftAndMainSubsites.php | 50 ++++++++++++++++--------- tests/LeftAndMainSubsitesTest.php | 19 ++++++++++ tests/SubsiteAdminFunctionalTest.php | 28 ++++++++++++++ 4 files changed, 86 insertions(+), 17 deletions(-) create mode 100644 _config/config.yml diff --git a/_config/config.yml b/_config/config.yml new file mode 100644 index 0000000..b3b2e21 --- /dev/null +++ b/_config/config.yml @@ -0,0 +1,6 @@ +--- +Name: mysiteconfig +After: 'framework/*','cms/*' +--- +AssetAdmin: + treats_subsite_0_as_global: true diff --git a/code/extensions/LeftAndMainSubsites.php b/code/extensions/LeftAndMainSubsites.php index acf503f..47c6574 100644 --- a/code/extensions/LeftAndMainSubsites.php +++ b/code/extensions/LeftAndMainSubsites.php @@ -8,18 +8,17 @@ class LeftAndMainSubsites extends Extension { private static $allowed_actions = array('CopyToSubsite'); + /** + * Normally SubsiteID=0 on a DataObject means it is only accessible from the special "main site". + * However in some situations SubsiteID=0 will be understood as a "globally accessible" object in which + * case this property is set to true (i.e. in AssetAdmin). + */ + private static $treats_subsite_0_as_global = false; + function init() { Requirements::css('subsites/css/LeftAndMain_Subsites.css'); Requirements::javascript('subsites/javascript/LeftAndMain_Subsites.js'); Requirements::javascript('subsites/javascript/VirtualPage_Subsites.js'); - - // Set subsite ID based on currently shown record - $req = $this->owner->getRequest(); - $id = $req->param('ID'); - if($id && is_numeric($id)) { - $record = DataObject::get_by_id($this->owner->stat('tree_class'), $id); - if($record) Session::set('SubsiteID', $record->SubsiteID); - } } /** @@ -151,6 +150,15 @@ class LeftAndMainSubsites extends Extension { return Permission::check("ADMIN", "any", null, "all"); } + /** + * Helper for testing if the subsite should be adjusted. + */ + public function shouldChangeSubsite($adminClass, $recordSubsiteID, $currentSubsiteID) { + if (Config::inst()->get($adminClass, 'treats_subsite_0_as_global') && $recordSubsiteID==0) return false; + if ($recordSubsiteID!=$currentSubsiteID) return true; + return false; + } + /** * Do some pre-flight checks if a subsite switch is needed. * We redirect the user to something accessible if the current section/subsite is forbidden. @@ -159,11 +167,26 @@ class LeftAndMainSubsites extends Extension { // We are accessing the CMS, so we need to let Subsites know we will be using the session. Subsite::$use_session_subsiteid = true; - // Do not try to be smart for AJAX requests. + // Do not attempt to redirect for AJAX calls. The proper security checks are done on specific objects + // (by Subsite augmentations of Member and Group). Also the only good time to change the subsite in the CMS + // is upon initial load - otherwise we risk the internal state becoming mismatched with the interface. if ($this->owner->request->isAjax()) { return; } + // FIRST, check if we need to change subsites due to the URL. + + // Automatically redirect the session to appropriate subsite when requesting a record. + // This is needed to properly initialise the session in situations where someone opens the CMS via a link. + $record = $this->owner->currentPage(); + if($record && isset($record->SubsiteID) && is_numeric($record->SubsiteID)) { + + if ($this->shouldChangeSubsite($this->owner->class, $record->SubsiteID, Subsite::currentSubsiteID())) { + Subsite::changeSubsite($record->SubsiteID); + } + + } + // Catch forced subsite changes that need to cause CMS reloads. if(isset($_GET['SubsiteID'])) { // Clear current page when subsite changes (or is set for the first time) @@ -178,14 +201,7 @@ class LeftAndMainSubsites extends Extension { return $this->owner->redirect('admin/'); } - $className = $this->owner->class; - - // Transparently switch to the subsite of the current page. - if ($this->owner->class == 'CMSMain' && $currentPage = $this->owner->currentPage()) { - if (Subsite::currentSubsiteID() != $currentPage->SubsiteID) { - Subsite::changeSubsite($currentPage->SubsiteID); - } - } + // SECOND, check if we need to change subsites due to lack of permissions. // If we can view current URL there is nothing to do. if ($this->owner->canView()) { diff --git a/tests/LeftAndMainSubsitesTest.php b/tests/LeftAndMainSubsitesTest.php index 5b7345a..0b2c6df 100644 --- a/tests/LeftAndMainSubsitesTest.php +++ b/tests/LeftAndMainSubsitesTest.php @@ -66,4 +66,23 @@ class LeftAndMainSubsitesTest extends FunctionalTest { } + function testShouldChangeSubsite() { + $l = new LeftAndMain(); + Config::inst()->nest(); + + Config::inst()->update('CMSPageEditController', 'treats_subsite_0_as_global', false); + $this->assertTrue($l->shouldChangeSubsite('CMSPageEditController', 0, 5)); + $this->assertFalse($l->shouldChangeSubsite('CMSPageEditController', 0, 0)); + $this->assertTrue($l->shouldChangeSubsite('CMSPageEditController', 1, 5)); + $this->assertFalse($l->shouldChangeSubsite('CMSPageEditController', 1, 1)); + + Config::inst()->update('CMSPageEditController', 'treats_subsite_0_as_global', true); + $this->assertFalse($l->shouldChangeSubsite('CMSPageEditController', 0, 5)); + $this->assertFalse($l->shouldChangeSubsite('CMSPageEditController', 0, 0)); + $this->assertTrue($l->shouldChangeSubsite('CMSPageEditController', 1, 5)); + $this->assertFalse($l->shouldChangeSubsite('CMSPageEditController', 1, 1)); + + Config::inst()->unnest(); + } + } diff --git a/tests/SubsiteAdminFunctionalTest.php b/tests/SubsiteAdminFunctionalTest.php index f5ea663..b0e57dd 100644 --- a/tests/SubsiteAdminFunctionalTest.php +++ b/tests/SubsiteAdminFunctionalTest.php @@ -56,6 +56,34 @@ class SubsiteAdminFunctionalTest extends FunctionalTest { 'SubsiteXHRController is reachable'); } + function testAdminIsRedirectedToObjectsSubsite() { + $member = $this->objFromFixture('Member', 'admin'); + Session::set("loggedInAs", $member->ID); + + $mainSubsitePage = $this->objFromFixture('Page', 'mainSubsitePage'); + $subsite1Home = $this->objFromFixture('Page', 'subsite1_home'); + + Config::inst()->nest(); + + Config::inst()->update('CMSPageEditController', 'treats_subsite_0_as_global', false); + Subsite::changeSubsite(0); + $this->getAndFollowAll("admin/pages/edit/show/$subsite1Home->ID"); + $this->assertEquals(Subsite::currentSubsiteID(), $subsite1Home->SubsiteID, 'Loading an object switches the subsite'); + $this->assertRegExp("#^admin/pages/edit/show/$subsite1Home->ID.*#", $this->mainSession->lastUrl(), 'Lands on the correct section'); + + Config::inst()->update('CMSPageEditController', 'treats_subsite_0_as_global', true); + Subsite::changeSubsite(0); + $this->getAndFollowAll("admin/pages/edit/show/$subsite1Home->ID"); + $this->assertEquals(Subsite::currentSubsiteID(), $subsite1Home->SubsiteID, 'Loading a non-main-site object still switches the subsite if configured with treats_subsite_0_as_global'); + $this->assertRegExp("#^admin/pages/edit/show/$subsite1Home->ID.*#", $this->mainSession->lastUrl(), 'Lands on the correct section'); + + $this->getAndFollowAll("admin/pages/edit/show/$mainSubsitePage->ID"); + $this->assertNotEquals(Subsite::currentSubsiteID(), $mainSubsitePage->SubsiteID, 'Loading a main-site object does not change the subsite if configured with treats_subsite_0_as_global'); + $this->assertRegExp("#^admin/pages/edit/show/$mainSubsitePage->ID.*#", $this->mainSession->lastUrl(), 'Lands on the correct section'); + + Config::inst()->unnest(); + } + /** * User which has AccessAllSubsites set to 1 should be able to access all subsites and main site, * even though he does not have the ADMIN permission.