Skip to content

Commit

Permalink
Merge pull request #388 from creative-commoners/pulls/2.1/fix-role-pe…
Browse files Browse the repository at this point in the history 
…rmissions

FIX CMS permission checks for subsite are now handled in the state context
  • Loading branch information
@ScopeyNZ
ScopeyNZ authored Aug 27, 2018
2 parents 039a7a8 + 7681634 commit 70dc70f
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 250 deletions.
14 changes: 1 addition & 13 deletions src/Controller/SubsiteXHRController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,13 @@ public function canView($member = null)
return true;
}

if (Subsite::all_accessible_sites()->count() > 0) {
if (Subsite::all_accessible_sites(true, 'Main site', $member)->count() > 0) {
return true;
}

return false;
}

/**
* Allow access if user allowed into the CMS at all.
*/
public function canAccess()
{
// Allow if any cms access is available
return Permission::check([
'CMS_ACCESS', // Supported by 3.1.14 and up
'CMS_ACCESS_LeftAndMain'
]);
}

public function getResponseNegotiator()
{
$negotiator = parent::getResponseNegotiator();
Expand Down
8 changes: 4 additions & 4 deletions src/Extensions/GroupSubsites.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public function updateCMSFields(FieldList $fields)
// Interface is different if you have the rights to modify subsite group values on
// all subsites
if (isset($subsiteMap[0])) {
$fields->addFieldToTab('Root.Subsites', new OptionsetField(
$fields->addFieldToTab('Root.Subsites', OptionsetField::create(
'AccessAllSubsites',
_t(__CLASS__ . '.ACCESSRADIOTITLE', 'Give this group access to'),
[
Expand All @@ -100,20 +100,20 @@ public function updateCMSFields(FieldList $fields)
));

unset($subsiteMap[0]);
$fields->addFieldToTab('Root.Subsites', new CheckboxSetField(
$fields->addFieldToTab('Root.Subsites', CheckboxSetField::create(
'Subsites',
'',
$subsiteMap
));
} else {
if (sizeof($subsiteMap) <= 1) {
$fields->addFieldToTab('Root.Subsites', new ReadonlyField(
$fields->addFieldToTab('Root.Subsites', ReadonlyField::create(
'SubsitesHuman',
_t(__CLASS__ . '.ACCESSRADIOTITLE', 'Give this group access to'),
reset($subsiteMap)
));
} else {
$fields->addFieldToTab('Root.Subsites', new CheckboxSetField(
$fields->addFieldToTab('Root.Subsites', CheckboxSetField::create(
'Subsites',
_t(__CLASS__ . '.ACCESSRADIOTITLE', 'Give this group access to'),
$subsiteMap
Expand Down
118 changes: 73 additions & 45 deletions src/Extensions/LeftAndMainSubsites.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
use SilverStripe\Admin\AdminRootController;
use SilverStripe\Admin\CMSMenu;
use SilverStripe\Admin\LeftAndMainExtension;
use SilverStripe\CMS\Controllers\CMSPageEditController;
use SilverStripe\CMS\Controllers\CMSPagesController;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\CMS\Controllers\CMSPageEditController;
use SilverStripe\Control\Controller;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Convert;
Expand Down Expand Up @@ -65,14 +65,13 @@ public function updatePageOptions(&$fields)
*
* @param bool $includeMainSite
* @param string $mainSiteTitle
* @param null $member
* @return ArrayList of <a href='psi_element://Subsite'>Subsite</a> instances.
* instances.
* @param Member|int|null $member
* @return ArrayList List of Subsite instances
*/
public function sectionSites($includeMainSite = true, $mainSiteTitle = 'Main site', $member = null)
{
if ($mainSiteTitle == 'Main site') {
$mainSiteTitle = _t('Subsites.MainSiteTitle', 'Main site');
if ($mainSiteTitle === 'Main site') {
$mainSiteTitle = _t('SilverStripe\\Subsites\\Model\\Subsite.MainSiteTitle', 'Main site');
}

// Rationalise member arguments
Expand All @@ -86,49 +85,33 @@ public function sectionSites($includeMainSite = true, $mainSiteTitle = 'Main sit
$member = DataObject::get_by_id(Member::class, $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 = [];
$extraCodes = Config::inst()->get(get_class($this->owner), 'required_permission_codes');
if ($extraCodes !== false) {
if ($extraCodes) {
$codes = array_merge($codes, (array)$extraCodes);
} else {
$codes[] = sprintf('CMS_ACCESS_%s', get_class($this->owner));
}
} else {
// Check overriden - all subsites accessible.
return Subsite::all_sites();
}
$accessibleSubsites = ArrayList::create();
$subsites = Subsite::all_sites($includeMainSite, $mainSiteTitle);

// Find subsites satisfying all permissions for the Member.
$codesPerSite = [];
$sitesArray = [];
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;
}
}
foreach ($subsites as $subsite) {
/** @var Subsite $subsite */
$canAccess = SubsiteState::singleton()
->withState(function (SubsiteState $newState) use ($subsite, $member) {
$newState->setSubsiteId($subsite->ID);

// 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 $this->canAccess($member);
});

if ($canAccess === false) {
// Explicitly denied
continue;
}
$accessibleSubsites->push($subsite);
}

return $accessibleSites;
return $accessibleSubsites;
}

/*
* 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.
*
* @return ArrayList
*/
public function Subsites()
{
Expand All @@ -138,6 +121,7 @@ public function Subsites()
/*
* Generates a list of subsites with the data needed to
* produce a dropdown site switcher
*
* @return ArrayList
*/

Expand Down Expand Up @@ -215,11 +199,17 @@ public function shouldChangeSubsite($adminClass, $recordSubsiteID, $currentSubsi

/**
* Check if the current controller is accessible for this user on this subsite.
*
* @param Member|int|null $member Will be added as a concrete param in 3.x
* @return false|null False if a decision was explicitly made to deny access, otherwise null to delegate to core
*/
public function canAccess()
{
// Allow us to accept a Member object passed in as an argument without breaking semver
$passedMember = func_num_args() ? func_get_arg(0) : null;

// Admin can access everything, no point in checking.
$member = Security::getCurrentUser();
$member = $passedMember ?: Security::getCurrentUser();
if ($member
&& (Permission::checkMember($member, 'ADMIN') // 'Full administrative rights'
|| Permission::checkMember($member, 'CMS_ACCESS_LeftAndMain') // 'Access to all CMS sections'
Expand All @@ -228,18 +218,56 @@ public function canAccess()
return true;
}

// Check if we have access to current section on the current subsite.
$accessibleSites = $this->owner->sectionSites(true, 'Main site', $member);
return $accessibleSites->count() && $accessibleSites->find('ID', SubsiteState::singleton()->getSubsiteId());
// Check we have a member
if (!$member) {
return null;
}

// Check that some subsites exist first
if (!Subsite::get()->exists()) {
return null;
}

// Get the current subsite ID
$currentSubsiteId = SubsiteState::singleton()->getSubsiteId();

// Check against the current user's associated groups
$allowedInSubsite = false;
foreach ($member->Groups() as $group) {
// If any of the current user's groups have been given explicit access to the subsite, delegate to core
if ($group->AccessAllSubsites) {
$allowedInSubsite = true;
break;
}

// Check if any of the current user's groups have been given explicit access to the current subsite
$groupSubsiteIds = $group->Subsites()->column('ID');
if (in_array($currentSubsiteId, $groupSubsiteIds)) {
$allowedInSubsite = true;
break;
}
}

// If we know that the user is not allowed in this subsite, explicitly say this
if (!$allowedInSubsite) {
return false;
}

// Delegate to core
return null;
}

/**
* Prevent accessing disallowed resources. This happens after onBeforeInit has executed,
* so all redirections should've already taken place.
*
* @return false|null
*/
public function alternateAccessCheck()
{
return $this->owner->canAccess();
if ($this->owner->canAccess() === false) {
return false;
}
}

/**
Expand Down Expand Up @@ -331,7 +359,7 @@ public function onBeforeInit()

// SECOND, check if we need to change subsites due to lack of permissions.

if (!$this->owner->canAccess()) {
if ($this->owner->canAccess() === false) {
$member = Security::getCurrentUser();

// Current section is not accessible, try at least to stick to the same subsite.
Expand Down
6 changes: 6 additions & 0 deletions src/Extensions/SiteTreeSubsites.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace SilverStripe\Subsites\Extensions;

use Page;
use SilverStripe\CMS\Controllers\CMSPagesController;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\Control\Controller;
use SilverStripe\Control\Director;
Expand Down Expand Up @@ -322,6 +323,11 @@ public function canEdit($member = null)
return null;
}

// Check general subsite section access for CMS
if (CMSPagesController::singleton()->canAccess($member) === false) {
return false;
}

// Find the sites that this user has access to
$goodSites = Subsite::accessible_sites('CMS_ACCESS_CMSMain', true, 'all', $member)->column('ID');

Expand Down
Loading

0 comments on commit 70dc70f

Please sign in to comment.