From 91cca0c64d1bcc345a1c546174b6ae6bc8793376 Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Wed, 16 Oct 2013 13:20:54 +1300 Subject: [PATCH 1/3] BUG Move the SubsiteList PJAX request to a dedicated Controller. Currently the request cannot be made if one doesn't have access to the SubsiteAdmin section, which often happens with subsite-specific admins. --- code/SubsiteAdmin.php | 13 ---------- code/SubsiteXHRController.php | 38 ++++++++++++++++++++++++++++++ javascript/LeftAndMain_Subsites.js | 6 ++--- 3 files changed, 41 insertions(+), 16 deletions(-) create mode 100644 code/SubsiteXHRController.php diff --git a/code/SubsiteAdmin.php b/code/SubsiteAdmin.php index 7ed122c..abea9a1 100644 --- a/code/SubsiteAdmin.php +++ b/code/SubsiteAdmin.php @@ -30,17 +30,4 @@ class SubsiteAdmin extends ModelAdmin { return $form; } - public function getResponseNegotiator() { - $negotiator = parent::getResponseNegotiator(); - $self = $this; - // Register a new callback - $negotiator->setCallback('SubsiteList', function() use(&$self) { - return $self->SubsiteList(); - }); - return $negotiator; - } - - public function SubsiteList() { - return $this->renderWith('SubsiteList'); - } } diff --git a/code/SubsiteXHRController.php b/code/SubsiteXHRController.php new file mode 100644 index 0000000..944bbd7 --- /dev/null +++ b/code/SubsiteXHRController.php @@ -0,0 +1,38 @@ +count()>0) return true; + + return false; + } + + public function getResponseNegotiator() { + $negotiator = parent::getResponseNegotiator(); + $self = $this; + + // Register a new callback + $negotiator->setCallback('SubsiteList', function() use(&$self) { + return $self->SubsiteList(); + }); + + return $negotiator; + } + + /** + * Provide the list of available subsites as a cms-section-agnostic PJAX handler. + */ + public function SubsiteList() { + return $this->renderWith('SubsiteList'); + } + +} diff --git a/javascript/LeftAndMain_Subsites.js b/javascript/LeftAndMain_Subsites.js index 5966440..8988eb6 100644 --- a/javascript/LeftAndMain_Subsites.js +++ b/javascript/LeftAndMain_Subsites.js @@ -58,7 +58,7 @@ */ $('.cms-container .cms-menu-list li a').entwine({ onclick: function(e) { - $('.cms-container').subsiteFetchPjaxFragment('admin/subsites/', 'SubsiteList'); + $('.cms-container').subsiteFetchPjaxFragment('SubsiteXHRController', 'SubsiteList'); this._super(e); } }); @@ -68,7 +68,7 @@ */ $('.cms-container .SubsiteAdmin .cms-edit-form fieldset.ss-gridfield').entwine({ onreload: function(e) { - $('.cms-container').subsiteFetchPjaxFragment('admin/subsites/', 'SubsiteList'); + $('.cms-container').subsiteFetchPjaxFragment('SubsiteXHRController', 'SubsiteList'); this._super(e); } }); @@ -81,7 +81,7 @@ */ $('.cms-container .cms-content-fields .subsite-model').entwine({ onadd: function(e) { - $('.cms-container').subsiteFetchPjaxFragment('admin/subsites/', 'SubsiteList'); + $('.cms-container').subsiteFetchPjaxFragment('SubsiteXHRController', 'SubsiteList'); this._super(e); } }); From 5b00ba352f14102ae225196112e4e81a305a500c Mon Sep 17 00:00:00 2001 From: Mateusz Uzdowski Date: Wed, 16 Oct 2013 13:25:06 +1300 Subject: [PATCH 2/3] API Refactor to always redirect to accessible Admin location. Tries to find an accessible section in the current site, falls back to searching across all sites and all sections. Also adds more powerful and generic functionss: Subsites::all_sites - get the full list Subsites::all_accessible_sites - get Member accessible list LeftAndMainExtension::sectionSites - get section-specific list --- code/extensions/LeftAndMainSubsites.php | 228 ++++++++++++++---------- code/model/Subsite.php | 65 ++++++- templates/LeftAndMain_Menu.ss | 2 +- 3 files changed, 193 insertions(+), 102 deletions(-) diff --git a/code/extensions/LeftAndMainSubsites.php b/code/extensions/LeftAndMainSubsites.php index 5177e53..a8d6868 100644 --- a/code/extensions/LeftAndMainSubsites.php +++ b/code/extensions/LeftAndMainSubsites.php @@ -1,7 +1,7 @@ owner->class}.currentPage"); - } - - // Update current subsite in session - Subsite::changeSubsite($_GET['SubsiteID']); - - //Redirect to clear the current page - return $this->owner->redirect('admin/'); - } // Set subsite ID based on currently shown record $req = $this->owner->getRequest(); @@ -50,24 +33,61 @@ class LeftAndMainSubsites extends Extension { function updatePageOptions(&$fields) { $fields->push(new HiddenField('SubsiteID', 'SubsiteID', Subsite::currentSubsiteID())); } - - /* - * Returns a list of the subsites accessible to the current user + + /** + * Find all subsites accessible for current user on this controller. + * + * @return ArrayList of {@link Subsite} instances. */ - public function Subsites() { - // figure out what permission the controller needs - // Subsite::accessible_sites() expects something, so if there's no permission - // then fallback to using CMS_ACCESS_LeftAndMain. - $permission = 'CMS_ACCESS_' . $this->owner->class; - $available = Permission::get_codes(false); - if(!isset($available[$permission])) { - $permission = $this->owner->stat('required_permission_codes'); - if(!$permission) { - $permission = 'CMS_ACCESS_LeftAndMain'; + function sectionSites($includeMainSite = true, $mainSiteTitle = "Main site", $member = null) { + // Rationalise member arguments + if(!$member) $member = Member::currentUser(); + if(!$member) return new ArrayList(); + if(!is_object($member)) $member = DataObject::get_by_id('Member', $member); + + // Collect permissions - honour the LeftAndMain::required_permission_codes, current model requires + // us to check if the user satisfies ALL permissions. Code partly copied from LeftAndMain::canView. + $codes = array(); + $extraCodes = Config::inst()->get($this->owner->class, 'required_permission_codes'); + if($extraCodes !== false) { + if($extraCodes) $codes = array_merge($codes, (array)$extraCodes); + else $codes[] = "CMS_ACCESS_{$this->owner->class}"; + } else { + // Check overriden - all subsites accessible. + return Subsite::all_sites(); + } + + // Find subsites satisfying all permissions for the Member. + $codesPerSite = array(); + $sitesArray = array(); + foreach ($codes as $code) { + $sites = Subsite::accessible_sites($code, $includeMainSite, $mainSiteTitle, $member); + foreach ($sites as $site) { + // Build the structure for checking how many codes match. + $codesPerSite[$site->ID][$code] = true; + + // Retain Subsite objects for later. + $sitesArray[$site->ID] = $site; } } - return Subsite::accessible_sites($permission); + // Find sites that satisfy all codes conjuncitvely. + $accessibleSites = new ArrayList(); + foreach ($codesPerSite as $siteID => $siteCodes) { + if (count($siteCodes)==count($codes)) { + $accessibleSites->push($sitesArray[$siteID]); + } + } + + return $accessibleSites; + } + + /* + * Returns a list of the subsites accessible to the current user. + * It's enough for any section to be accessible for the section to be included. + */ + public function Subsites() { + return Subsite::all_accessible_sites(); } /* @@ -101,38 +121,26 @@ class LeftAndMainSubsites extends Extension { return $output; } - /* - * Returns a subset of the main menu, filtered by admins that have - * a subsiteCMSShowInMenu method returning true - * - * @return ArrayList - */ - public function SubsiteMainMenu(){ + public function alternateMenuDisplayCheck($controllerName) { + if(!class_exists($controllerName)){ + return false; + } + + // Check subsite support. if(Subsite::currentSubsiteID() == 0){ - return $this->owner->MainMenu(); - } - // loop main menu items, add all items that have subsite support - $mainMenu = $this->owner->MainMenu(); - $subsitesMenu = new ArrayList(); - - foreach($mainMenu as $menuItem){ - - $controllerName = $menuItem->MenuItem->controller; - - if(class_exists($controllerName)){ - $controller = singleton($controllerName); - - if($controller->hasMethod('subsiteCMSShowInMenu') && $controller->subsiteCMSShowInMenu()){ - $subsitesMenu->push($menuItem); - } + // Main site always supports everything. + return true; + } else { + $controller = singleton($controllerName); + if($controller->hasMethod('subsiteCMSShowInMenu') && $controller->subsiteCMSShowInMenu()){ + return true; } - - if($menuItem->Code == 'Help'){ - $subsitesMenu->push($menuItem); - } - } - return $subsitesMenu; + + // It's not necessary to check access permissions here. Framework calls canView on the controller, + // which in turn uses the Permission API which is augmented by our GroupSubsites. + + return false; } public function CanAddSubsites() { @@ -140,58 +148,90 @@ class LeftAndMainSubsites extends Extension { } /** - * Alternative security checker for LeftAndMain. - * If security isn't found, then it will switch to a subsite where we do have access. + * 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. */ - public function alternateAccessCheck() { + public function onBeforeInit() { + // 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. + if ($this->owner->request->isAjax()) { + return; + } + + // 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) + if(!Session::get('SubsiteID') || $_GET['SubsiteID'] != Session::get('SubsiteID')) { + Session::clear("{$this->owner->class}.currentPage"); + } + + // Update current subsite in session + Subsite::changeSubsite($_GET['SubsiteID']); + + //Redirect to clear the current page + return $this->owner->redirect('admin/'); + } + $className = $this->owner->class; - // Switch to the subsite of the current page + // 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); } } - - // Switch to a subsite that this user can actually access. - $member = Member::currentUser(); - if($member && Permission::checkMember($member, 'ADMIN')) return true; // admin can access all subsites - - $sites = Subsite::accessible_sites("CMS_ACCESS_{$this->owner->class}", true)->map('ID', 'Title'); - if(is_object($sites)) $sites = $sites->toArray(); - if($sites && !isset($sites[Subsite::currentSubsiteID()])) { - $siteIDs = array_keys($sites); - Subsite::changeSubsite($siteIDs[0]); - return true; + // If we can view current URL there is nothing to do. + if ($this->owner->canView()) { + return; } - - // Switch to a different top-level menu item + + // Admin can access everything, no point in checking. + $member = Member::currentUser(); + if($member && Permission::checkMember($member, 'ADMIN')) return; + + // Check if we have access to current section on the current subsite. + $accessibleSites = $this->owner->sectionSites($member); + if ($accessibleSites->count() && $accessibleSites->find('ID', Subsite::currentSubsiteID())) { + // Current section can be accessed on the current site, all good. + return; + } + + // If the current section is not accessible, try at least to stick to the same subsite. $menu = CMSMenu::get_menu_items(); foreach($menu as $candidate) { - if($candidate->controller != $this->owner->class) { - $sites = Subsite::accessible_sites("CMS_ACCESS_{$candidate->controller}", true)->map('ID', 'Title'); - if(is_object($sites)) $sites = $sites->toArray(); - - if($sites && !isset($sites[Subsite::currentSubsiteID()])) { - $siteIDs = array_keys($sites); - Subsite::changeSubsite($siteIDs[0]); - $cClass = $candidate->controller; - $cObj = new $cClass(); - $this->owner->redirect($cObj->Link()); - return null; + if($candidate->controller && $candidate->controller!=$this->owner->class) { + + $accessibleSites = singleton($candidate->controller)->sectionSites($member); + if ($accessibleSites->count() && $accessibleSites->find('ID', Subsite::currentSubsiteID())) { + // Section is accessible, redirect there. + $this->owner->redirect(singleton($candidate->controller)->Link()); + return; } } } - - // If all of those fail, you really don't have access to the CMS - return null; + + // Finally, if no section is available, move to any other permitted subsite. + foreach($menu as $candidate) { + if($candidate->controller && $candidate->controller != $this->owner->class) { + + $accessibleSites = singleton($candidate->controller)->sectionSites($member); + if ($accessibleSites->count()) { + Subsite::changeSubsite($accessibleSites->First()->ID); + $this->owner->redirect(singleton($candidate->controller)->Link()); + return; + } + } + } + } - + function augmentNewSiteTreeItem(&$item) { $item->SubsiteID = isset($_POST['SubsiteID']) ? $_POST['SubsiteID'] : Subsite::currentSubsiteID(); } - + function onAfterSave($record) { if($record->hasMethod('NormalRelated') && ($record->NormalRelated() || $record->ReverseRelated())) { $this->owner->response->addHeader('X-Status', rawurlencode(_t('LeftAndMainSubsites.Saved', 'Saved, please update related pages.'))); diff --git a/code/model/Subsite.php b/code/model/Subsite.php index 056e013..6f3a92d 100644 --- a/code/model/Subsite.php +++ b/code/model/Subsite.php @@ -310,10 +310,9 @@ JS; if(isset($_GET['SubsiteID'])) { $id = (int)$_GET['SubsiteID']; - } - else if (Subsite::$use_session_subsiteid) { + } else if (Subsite::$use_session_subsiteid) { $id = Session::get('SubsiteID'); - } + } if($id === NULL) { $id = self::getSubsiteIDForDomain(); @@ -522,12 +521,64 @@ JS; return $duplicate; } + /** + * Return all subsites, regardless of permissions (augmented with main site). + * + * @return SS_List List of {@link Subsite} objects (DataList or ArrayList). + */ + public static function all_sites($includeMainSite = true, $mainSiteTitle = "Main site") { + $subsites = Subsite::get(); + + if($includeMainSite) { + $subsites = $subsites->toArray(); + + $mainSite = new Subsite(); + $mainSite->Title = $mainSiteTitle; + array_unshift($subsites, $mainSite); + + $subsites = ArrayList::create($subsites); + } + + return $subsites; + } + + /* + * Returns an ArrayList of the subsites accessible to the current user. + * It's enough for any section to be accessible for the site to be included. + * + * @return ArrayList of {@link Subsite} instances. + */ + public static function all_accessible_sites($includeMainSite = true, $mainSiteTitle = "Main site", $member = null) { + // Rationalise member arguments + if(!$member) $member = Member::currentUser(); + if(!$member) return new ArrayList(); + if(!is_object($member)) $member = DataObject::get_by_id('Member', $member); + + $subsites = new ArrayList(); + + // Collect subsites for all sections. + $menu = CMSMenu::get_viewable_menu_items(); + foreach($menu as $candidate) { + if ($candidate->controller) { + $accessibleSites = singleton($candidate->controller)->sectionSites( + $includeMainSite, + $mainSiteTitle, + $member + ); + + // Replace existing keys so no one site appears twice. + $subsites->merge($accessibleSites); + } + } + + $subsites->removeDuplicates(); + + return $subsites; + } /** - * Return the subsites that the current user can access. - * Look for one of the given permission codes on the site. - * - * Sites will only be included if they have a Title + * Return the subsites that the current user can access by given permission. + * Sites will only be included if they have a Title. * * @param $permCode array|string Either a single permission code or an array of permission codes. * @param $includeMainSite If true, the main site will be included if appropriate. diff --git a/templates/LeftAndMain_Menu.ss b/templates/LeftAndMain_Menu.ss index f531f37..3a2f4ce 100644 --- a/templates/LeftAndMain_Menu.ss +++ b/templates/LeftAndMain_Menu.ss @@ -25,7 +25,7 @@