From 7cc86199e7140c9584660e603bc4e4eb48652150 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Fri, 19 Oct 2018 12:00:50 +1300 Subject: [PATCH] Revert "FIX CMS permission checks for subsite are now handled in the state context" --- src/Controller/SubsiteXHRController.php | 14 +- src/Extensions/GroupSubsites.php | 8 +- src/Extensions/LeftAndMainSubsites.php | 122 +++++++---------- src/Extensions/SiteTreeSubsites.php | 6 - src/Model/Subsite.php | 167 +++++++++++++++++------- tests/php/LeftAndMainSubsitesTest.php | 114 ++++++---------- tests/php/SiteTreeSubsitesTest.php | 65 ++++----- 7 files changed, 256 insertions(+), 240 deletions(-) diff --git a/src/Controller/SubsiteXHRController.php b/src/Controller/SubsiteXHRController.php index 9016f6b..c758d53 100644 --- a/src/Controller/SubsiteXHRController.php +++ b/src/Controller/SubsiteXHRController.php @@ -27,13 +27,25 @@ class SubsiteXHRController extends LeftAndMain return true; } - if (Subsite::all_accessible_sites(true, 'Main site', $member)->count() > 0) { + if (Subsite::all_accessible_sites()->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(); diff --git a/src/Extensions/GroupSubsites.php b/src/Extensions/GroupSubsites.php index 4df1521..2e583f1 100644 --- a/src/Extensions/GroupSubsites.php +++ b/src/Extensions/GroupSubsites.php @@ -90,7 +90,7 @@ class GroupSubsites extends DataExtension implements PermissionProvider // Interface is different if you have the rights to modify subsite group values on // all subsites if (isset($subsiteMap[0])) { - $fields->addFieldToTab('Root.Subsites', OptionsetField::create( + $fields->addFieldToTab('Root.Subsites', new OptionsetField( 'AccessAllSubsites', _t(__CLASS__ . '.ACCESSRADIOTITLE', 'Give this group access to'), [ @@ -100,20 +100,20 @@ class GroupSubsites extends DataExtension implements PermissionProvider )); unset($subsiteMap[0]); - $fields->addFieldToTab('Root.Subsites', CheckboxSetField::create( + $fields->addFieldToTab('Root.Subsites', new CheckboxSetField( 'Subsites', '', $subsiteMap )); } else { if (sizeof($subsiteMap) <= 1) { - $fields->addFieldToTab('Root.Subsites', ReadonlyField::create( + $fields->addFieldToTab('Root.Subsites', new ReadonlyField( 'SubsitesHuman', _t(__CLASS__ . '.ACCESSRADIOTITLE', 'Give this group access to'), reset($subsiteMap) )); } else { - $fields->addFieldToTab('Root.Subsites', CheckboxSetField::create( + $fields->addFieldToTab('Root.Subsites', new CheckboxSetField( 'Subsites', _t(__CLASS__ . '.ACCESSRADIOTITLE', 'Give this group access to'), $subsiteMap diff --git a/src/Extensions/LeftAndMainSubsites.php b/src/Extensions/LeftAndMainSubsites.php index d643d7e..760623f 100644 --- a/src/Extensions/LeftAndMainSubsites.php +++ b/src/Extensions/LeftAndMainSubsites.php @@ -5,9 +5,9 @@ namespace SilverStripe\Subsites\Extensions; 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; @@ -65,13 +65,14 @@ class LeftAndMainSubsites extends LeftAndMainExtension * * @param bool $includeMainSite * @param string $mainSiteTitle - * @param Member|int|null $member - * @return ArrayList List of Subsite instances + * @param null $member + * @return ArrayList of Subsite instances. + * instances. */ public function sectionSites($includeMainSite = true, $mainSiteTitle = 'Main site', $member = null) { - if ($mainSiteTitle === 'Main site') { - $mainSiteTitle = _t('SilverStripe\\Subsites\\Model\\Subsite.MainSiteTitle', 'Main site'); + if ($mainSiteTitle == 'Main site') { + $mainSiteTitle = _t('Subsites.MainSiteTitle', 'Main site'); } // Rationalise member arguments @@ -85,33 +86,49 @@ class LeftAndMainSubsites extends LeftAndMainExtension $member = DataObject::get_by_id(Member::class, $member); } - $accessibleSubsites = ArrayList::create(); - $subsites = Subsite::all_sites($includeMainSite, $mainSiteTitle); - - foreach ($subsites as $subsite) { - /** @var Subsite $subsite */ - $canAccess = SubsiteState::singleton() - ->withState(function (SubsiteState $newState) use ($subsite, $member) { - $newState->setSubsiteId($subsite->ID); - - return $this->canAccess($member); - }); - - if ($canAccess === false) { - // Explicitly denied - continue; + // 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)); } - $accessibleSubsites->push($subsite); + } else { + // Check overriden - all subsites accessible. + return Subsite::all_sites(); } - return $accessibleSubsites; + // 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; + } + } + + // 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. - * - * @return ArrayList */ public function Subsites() { @@ -121,7 +138,6 @@ class LeftAndMainSubsites extends LeftAndMainExtension /* * Generates a list of subsites with the data needed to * produce a dropdown site switcher - * * @return ArrayList */ @@ -199,17 +215,11 @@ class LeftAndMainSubsites extends LeftAndMainExtension /** * 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 = $passedMember ?: Security::getCurrentUser(); + $member = Security::getCurrentUser(); if ($member && (Permission::checkMember($member, 'ADMIN') // 'Full administrative rights' || Permission::checkMember($member, 'CMS_ACCESS_LeftAndMain') // 'Access to all CMS sections' @@ -218,56 +228,18 @@ class LeftAndMainSubsites extends LeftAndMainExtension return true; } - // 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; + // 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()); } /** * Prevent accessing disallowed resources. This happens after onBeforeInit has executed, * so all redirections should've already taken place. - * - * @return false|null */ public function alternateAccessCheck() { - if ($this->owner->canAccess() === false) { - return false; - } + return $this->owner->canAccess(); } /** @@ -359,7 +331,7 @@ class LeftAndMainSubsites extends LeftAndMainExtension // SECOND, check if we need to change subsites due to lack of permissions. - if ($this->owner->canAccess() === false) { + if (!$this->owner->canAccess()) { $member = Security::getCurrentUser(); // Current section is not accessible, try at least to stick to the same subsite. diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index b1f3f74..ecef3a5 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -3,7 +3,6 @@ namespace SilverStripe\Subsites\Extensions; use Page; -use SilverStripe\CMS\Controllers\CMSPagesController; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; @@ -323,11 +322,6 @@ class SiteTreeSubsites extends DataExtension 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'); diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index 704e23d..229604d 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -3,7 +3,6 @@ namespace SilverStripe\Subsites\Model; use SilverStripe\Admin\CMSMenu; -use SilverStripe\Admin\LeftAndMain; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Director; use SilverStripe\Core\Convert; @@ -29,7 +28,6 @@ use SilverStripe\Security\Group; use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; -use SilverStripe\Subsites\Extensions\LeftAndMainSubsites; use SilverStripe\Subsites\State\SubsiteState; use SilverStripe\Versioned\Versioned; use UnexpectedValueException; @@ -374,7 +372,7 @@ class Subsite extends DataObject if ($includeMainSite) { $subsites = $subsites->toArray(); - $mainSite = Subsite::create(); + $mainSite = new Subsite(); $mainSite->Title = $mainSiteTitle; array_unshift($subsites, $mainSite); @@ -384,7 +382,7 @@ class Subsite extends DataObject 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. * @@ -392,26 +390,32 @@ class Subsite extends DataObject */ public static function all_accessible_sites($includeMainSite = true, $mainSiteTitle = 'Main site', $member = null) { + // Rationalise member arguments + if (!$member) { + $member = Security::getCurrentUser(); + } + if (!$member) { + return ArrayList::create(); + } + if (!is_object($member)) { + $member = DataObject::get_by_id(Member::class, $member); + } + $subsites = ArrayList::create(); // Collect subsites for all sections. $menu = CMSMenu::get_viewable_menu_items(); foreach ($menu as $candidate) { - if (!$candidate->controller) { - continue; + if ($candidate->controller) { + $accessibleSites = singleton($candidate->controller)->sectionSites( + $includeMainSite, + $mainSiteTitle, + $member + ); + + // Replace existing keys so no one site appears twice. + $subsites->merge($accessibleSites); } - - // Determine which subsites can access the current admin section - /** @var LeftAndMain|LeftAndMainSubsites $controller */ - $controller = singleton($candidate->controller); - $accessibleSites = $controller->sectionSites( - $includeMainSite, - $mainSiteTitle, - $member - ); - - // Replace existing keys so no one site appears twice. - $subsites->merge($accessibleSites); } $subsites->removeDuplicates(); @@ -423,11 +427,11 @@ class Subsite extends DataObject * Return the subsites that the current user can access by given permission. * Sites will only be included if they have a Title. * - * @param array|string $permCode Either a single permission code or an array of permission codes. - * @param bool $includeMainSite If true, the main site will be included if appropriate. - * @param string $mainSiteTitle The label to give to the main site - * @param Member|int $member The member attempting to access the sites - * @return ArrayList List of {@link Subsite} instances + * @param $permCode array|string Either a single permission code or an array of permission codes. + * @param $includeMainSite bool If true, the main site will be included if appropriate. + * @param $mainSiteTitle string The label to give to the main site + * @param $member int|Member The member attempting to access the sites + * @return DataList|ArrayList of {@link Subsite} instances */ public static function accessible_sites( $permCode, @@ -435,42 +439,107 @@ class Subsite extends DataObject $mainSiteTitle = 'Main site', $member = null ) { - $memberCacheKey = is_object($member) ? $member->ID : 0; + + // Rationalise member arguments + if (!$member) { + $member = Member::currentUser(); + } + if (!$member) { + return new ArrayList(); + } + if (!is_object($member)) { + $member = DataObject::get_by_id(Member::class, $member); + } + + // Rationalise permCode argument + if (is_array($permCode)) { + $SQL_codes = "'" . implode("', '", Convert::raw2sql($permCode)) . "'"; + } else { + $SQL_codes = "'" . Convert::raw2sql($permCode) . "'"; + } // Cache handling - $cacheKey = json_encode($permCode) . '-' . $memberCacheKey . '-' . $includeMainSite . '-' . $mainSiteTitle; + $cacheKey = $SQL_codes . '-' . $member->ID . '-' . $includeMainSite . '-' . $mainSiteTitle; if (isset(self::$cache_accessible_sites[$cacheKey])) { return self::$cache_accessible_sites[$cacheKey]; } - $accessibleSubsites = ArrayList::create(); - $subsites = self::all_sites($includeMainSite, $mainSiteTitle); - foreach ($subsites as $subsite) { - // Exclude subsites with no title - if (empty($subsite->Title)) { - continue; - } + /** @skipUpgrade */ + $subsites = DataList::create(Subsite::class) + ->where("\"Subsite\".\"Title\" != ''") + ->leftJoin('Group_Subsites', '"Group_Subsites"."SubsiteID" = "Subsite"."ID"') + ->innerJoin( + 'Group', + '"Group"."ID" = "Group_Subsites"."GroupID" OR "Group"."AccessAllSubsites" = 1' + ) + ->innerJoin( + 'Group_Members', + "\"Group_Members\".\"GroupID\"=\"Group\".\"ID\" AND \"Group_Members\".\"MemberID\" = $member->ID" + ) + ->innerJoin( + 'Permission', + "\"Group\".\"ID\"=\"Permission\".\"GroupID\" + AND \"Permission\".\"Code\" + IN ($SQL_codes, 'CMS_ACCESS_LeftAndMain', 'ADMIN')" + ); - /** @var Subsite $subsite */ - $canAccess = SubsiteState::singleton() - ->withState(function (SubsiteState $newState) use ($member, $subsite, $permCode) { - // Mock each individual subsite and run permission checks in it - $newState->setSubsiteId($subsite->ID); - - return Permission::checkMember($member, $permCode); - }); - - if ($canAccess === false) { - // Explicitly denied access - continue; - } - - $accessibleSubsites->push($subsite); + if (!$subsites) { + $subsites = new ArrayList(); } - self::$cache_accessible_sites[$cacheKey] = $accessibleSubsites; + /** @var DataList $rolesSubsites */ + /** @skipUpgrade */ + $rolesSubsites = DataList::create(Subsite::class) + ->where("\"Subsite\".\"Title\" != ''") + ->leftJoin('Group_Subsites', '"Group_Subsites"."SubsiteID" = "Subsite"."ID"') + ->innerJoin( + 'Group', + '"Group"."ID" = "Group_Subsites"."GroupID" OR "Group"."AccessAllSubsites" = 1' + ) + ->innerJoin( + 'Group_Members', + "\"Group_Members\".\"GroupID\"=\"Group\".\"ID\" AND \"Group_Members\".\"MemberID\" = $member->ID" + ) + ->innerJoin('Group_Roles', '"Group_Roles"."GroupID"="Group"."ID"') + ->innerJoin('PermissionRole', '"Group_Roles"."PermissionRoleID"="PermissionRole"."ID"') + ->innerJoin( + 'PermissionRoleCode', + "\"PermissionRole\".\"ID\"=\"PermissionRoleCode\".\"RoleID\" + AND \"PermissionRoleCode\".\"Code\" + IN ($SQL_codes, 'CMS_ACCESS_LeftAndMain', 'ADMIN')" + ); - return $accessibleSubsites; + if (!$subsites && $rolesSubsites) { + return $rolesSubsites; + } + + $subsites = new ArrayList($subsites->toArray()); + + if ($rolesSubsites) { + foreach ($rolesSubsites as $subsite) { + if (!$subsites->find('ID', $subsite->ID)) { + $subsites->push($subsite); + } + } + } + + if ($includeMainSite) { + if (!is_array($permCode)) { + $permCode = [$permCode]; + } + if (self::hasMainSitePermission($member, $permCode)) { + $subsites = $subsites->toArray(); + + $mainSite = new Subsite(); + $mainSite->Title = $mainSiteTitle; + array_unshift($subsites, $mainSite); + $subsites = ArrayList::create($subsites); + } + } + + self::$cache_accessible_sites[$cacheKey] = $subsites; + + return $subsites; } /** diff --git a/tests/php/LeftAndMainSubsitesTest.php b/tests/php/LeftAndMainSubsitesTest.php index 5be30e9..09df18f 100644 --- a/tests/php/LeftAndMainSubsitesTest.php +++ b/tests/php/LeftAndMainSubsitesTest.php @@ -7,10 +7,8 @@ use SilverStripe\AssetAdmin\Controller\AssetAdmin; use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\CMS\Controllers\CMSPageEditController; use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\FunctionalTest; use SilverStripe\Security\Member; -use SilverStripe\Subsites\Extensions\LeftAndMainSubsites; use SilverStripe\Subsites\Model\Subsite; use SilverStripe\Subsites\State\SubsiteState; @@ -33,91 +31,57 @@ class LeftAndMainSubsitesTest extends FunctionalTest return $obj; } - /** - * @dataProvider sectionSitesProvider - * - * @param string $identifier - * @param string $className - * @param array $expected - * @param string $message - * @param string $assertion - */ - public function testSectionSites($identifier, $className, $expected, $message, $assertion = 'assertListEquals') + public function testSectionSites() { - $member = $this->objFromFixture(Member::class, $identifier); + $member = $this->objFromFixture(Member::class, 'subsite1member'); - /** @var CMSMain|LeftAndMainSubsites $cmsmain */ - $cmsMain = Injector::inst()->create($className); - $subsites = $cmsMain->sectionSites(true, 'Main site', $member); - $this->$assertion($expected, $subsites, $message); + $cmsmain = singleton(CMSMain::class); + $subsites = $cmsmain->sectionSites(true, 'Main site', $member); + $this->assertDOSEquals([ + ['Title' => 'Subsite1 Template'] + ], $subsites, 'Lists member-accessible sites for the accessible controller.'); + + $assetadmin = singleton(AssetAdmin::class); + $subsites = $assetadmin->sectionSites(true, 'Main site', $member); + $this->assertDOSEquals([], $subsites, 'Does not list any sites for forbidden controller.'); + + $member = $this->objFromFixture(Member::class, 'editor'); + + $cmsmain = singleton(CMSMain::class); + $subsites = $cmsmain->sectionSites(true, 'Main site', $member); + $this->assertDOSContains([ + ['Title' => 'Main site'] + ], $subsites, 'Includes the main site for members who can access all sites.'); } - /** - * @return array[] - */ - public function sectionSitesProvider() - { - return [ - [ - 'subsite1member', - CMSMain::class, - [['Title' => 'Subsite1 Template']], - 'Lists member-accessible sites for the accessible controller.', - ], - [ - 'subsite1member', - AssetAdmin::class, - [[]], - 'Does not list any sites for forbidden controller.', - ], - [ - 'editor', - CMSMain::class, - [['Title' => 'Main site']], - 'Includes the main site for members who can access all sites.', - 'assertListContains', - ], - ]; - } - - /** - * @dataProvider accessChecksProvider - * - * @param string $identifier - */ - public function testAccessChecksDontChangeCurrentSubsite($identifier) + public function testAccessChecksDontChangeCurrentSubsite() { $this->logInAs('admin'); + $ids = []; - /** @var Subsite $subsite */ - $subsite = $this->objFromFixture(Subsite::class, $identifier); - $id = $subsite->ID; + $subsite1 = $this->objFromFixture(Subsite::class, 'domaintest1'); + $subsite2 = $this->objFromFixture(Subsite::class, 'domaintest2'); + $subsite3 = $this->objFromFixture(Subsite::class, 'domaintest3'); + $ids[] = $subsite1->ID; + $ids[] = $subsite2->ID; + $ids[] = $subsite3->ID; + $ids[] = 0; // Enable session-based subsite tracking. SubsiteState::singleton()->setUseSessions(true); - Subsite::changeSubsite($id); - $this->assertEquals($id, SubsiteState::singleton()->getSubsiteId(), 'Subsite ID is in the state'); + foreach ($ids as $id) { + Subsite::changeSubsite($id); + $this->assertEquals($id, SubsiteState::singleton()->getSubsiteId()); - $left = new LeftAndMain(); - $this->assertTrue($left->canView(), "Admin user can view subsites LeftAndMain with id = '$id'"); - $this->assertEquals( - $id, - SubsiteState::singleton()->getSubsiteId(), - 'The current subsite has not been changed in the process of checking permissions for admin user.' - ); - } - - /** - * @return array[] - */ - public function accessChecksProvider() - { - return [ - ['domaintest1'], - ['domaintest3'], - ['domaintest3'], - ]; + $left = new LeftAndMain(); + $this->assertTrue($left->canView(), "Admin user can view subsites LeftAndMain with id = '$id'"); + $this->assertEquals( + $id, + SubsiteState::singleton()->getSubsiteId(), + 'The current subsite has not been changed in the process of checking permissions for admin user.' + ); + } } public function testShouldChangeSubsite() diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index 6efe3e7..efb16f8 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -112,41 +112,46 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest $this->assertEquals($expected_path, $path); } - /** - * @dataProvider canEditProvider - * - * @param string $memberIdentifier - * @param string $subsiteIdentifier - * @param string $pageIdentifier - * @param bool $expected - * @param string $message - */ - public function testCanEditSiteTree($memberIdentifier, $subsiteIdentifier, $pageIdentifier, $expected, $message) + public function testCanEditSiteTree() { - $this->logInAs($memberIdentifier); + $admin = $this->objFromFixture(Member::class, 'admin'); + $subsite1member = $this->objFromFixture(Member::class, 'subsite1member'); + $subsite2member = $this->objFromFixture(Member::class, 'subsite2member'); + $mainpage = $this->objFromFixture('Page', 'home'); + $subsite1page = $this->objFromFixture('Page', 'subsite1_home'); + $subsite2page = $this->objFromFixture('Page', 'subsite2_home'); + $subsite1 = $this->objFromFixture(Subsite::class, 'subsite1'); + $subsite2 = $this->objFromFixture(Subsite::class, 'subsite2'); - /** @var Page $page */ - $page = $this->objFromFixture(Page::class, $pageIdentifier); + // Cant pass member as arguments to canEdit() because of GroupSubsites + $this->logInAs($admin); - if (is_string($subsiteIdentifier)) { - $subsiteIdentifier = $this->idFromFixture(Subsite::class, $subsiteIdentifier); - } - Subsite::changeSubsite($subsiteIdentifier); + $this->assertTrue( + (bool)$subsite1page->canEdit(), + 'Administrators can edit all subsites' + ); - $this->assertSame($expected, (bool) $page->canEdit(), $message); - } + // @todo: Workaround because GroupSubsites->augmentSQL() is relying on session state + Subsite::changeSubsite($subsite1); - /** - * @return array[] - */ - public function canEditProvider() - { - return [ - ['admin', 0, 'subsite1_home', true, 'Administrators can edit all subsites'], - ['subsite1member', 'subsite1', 'subsite1_home', true, 'Can edit on subsite if group belongs to subsite'], - ['subsite2member', 'subsite1', 'subsite1_home', false, 'Cannot edit on subsite if group doesn\'t belong'], - ['subsite2member', 0, 'home', false, 'Cannot edit pages on main site if not in correct group'], - ]; + $this->logInAs($subsite1member->ID); + $this->assertTrue( + (bool)$subsite1page->canEdit(), + 'Members can edit pages on a subsite if they are in a group belonging to this subsite' + ); + + $this->logInAs($subsite2member->ID); + $this->assertFalse( + (bool)$subsite1page->canEdit(), + 'Members cant edit pages on a subsite if they are not in a group belonging to this subsite' + ); + + // @todo: Workaround because GroupSubsites->augmentSQL() is relying on session state + Subsite::changeSubsite(0); + $this->assertFalse( + $mainpage->canEdit(), + 'Members cant edit pages on the main site if they are not in a group allowing this' + ); } /**