diff --git a/code/GroupSubsites.php b/code/GroupSubsites.php index bacf3b1..026a826 100644 --- a/code/GroupSubsites.php +++ b/code/GroupSubsites.php @@ -11,39 +11,80 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider { if($this->owner->class != 'Group') return null; } return array( - 'has_one' => array( - 'Subsite' => 'Subsite', + 'db' => array( + 'AccessAllSubsites' => 'Boolean', + ), + 'many_many' => array( + 'Subsites' => 'Subsite', + ), + 'defaults' => array( + 'AccessAllSubsites' => 1, ), ); } + + /** + * Migrations for GroupSubsites data. + */ + function requireDefaultRecords() { + // Migration for Group.SubsiteID data from when Groups only had a single subsite + $groupFields = DB::getConn()->fieldList('Group'); + + // Detection of SubsiteID field is the trigger for old-style-subsiteID migration + if(isset($groupFields['SubsiteID'])) { + // Migrate subsite-specific data + DB::query('INSERT INTO "Group_Subsites" ("GroupID", "SubsiteID") + SELECT "ID", "SubsiteID" FROM "Group" WHERE "SubsiteID" > 0'); + + // Migrate global-access data + DB::query('UPDATE "Group" SET "AccessAllSubsites" = 1 WHERE "SubsiteID" = 0'); + + // Move the field out of the way so that this migration doesn't get executed again + DB::getConn()->renameField('Group', 'SubsiteID', '_obsolete_SubsiteID'); + + // No subsite access on anything means that we've just installed the subsites module. + // Make all previous groups global-access groups + } else if(!DB::query('SELECT "ID" FROM "Group" WHERE "AccessAllSubsites" = 1 + OR "Group_Subsites"."GroupID" IS NOT NULL + LEFT JOIN "Group_Subsites" ON "Group_Subsites"."GroupID" = "Group"."ID" + AND "Group_Subsites"."SubsiteID" > 0')->value()) { + + DB::query('UPDATE "Group" SET "AccessAllSubsites" = 1'); + } + } + function updateCMSFields(&$fields) { if($this->owner->canEdit() ){ - $subsites = Subsite::accessible_sites(array('ADMIN', 'SECURITY_SUBSITE_GROUP'), true, - "All sites"); + // i18n tab + $fields->findOrMakeTab('Root.Subsites',_t('GroupSubsites.SECURITYTABTITLE','Subsites')); + + $subsites = Subsite::accessible_sites(array('ADMIN', 'SECURITY_SUBSITE_GROUP'), true); $subsiteMap = $subsites->toDropdownMap(); - $tab = $fields->findOrMakeTab( - 'Root.Subsites', - _t('GroupSubsites.SECURITYTABTITLE', 'Subsites') - ); - - // This will trick the $dropdown code below to displaying the correct human val, - // readonly - if(!isset($subsiteMap[$this->owner->SubsiteID])) { - if($this->owner->SubsiteID) $subsiteTitle = $this->owner->Subsite()->Title; - else $subsiteTitle = "All sites"; - $subsiteMap = array($this->owner->SubsiteID => $subsiteTitle); + // 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("AccessAllSubsites", + _t('GroupSubsites.ACCESSRADIOTITLE', 'Give this group access to'), + array( + 1 => _t('GroupSubsites.ACCESSALL', "All subsites"), + 0 => _t('GroupSubsites.ACCESSONLY', "Only these subsites"), + ) + )); + + unset($subsiteMap[0]); + $fields->addFieldToTab("Root.Subsites", new CheckboxSetField("Subsites", "", + $subsiteMap)); + + } else { + if (sizeof($subsiteMap) <= 1) $dropdown = $dropdown->transform(new ReadonlyTransformation()) ; + $tab->push($dropdown); + + $fields->addFieldToTab("Root.Subsites", new CheckboxSetField("Subsites", + _t('GroupSubsites.ACCESSRADIOTITLE', 'Give this group access to'), + $subsiteMap)); } - - $dropdown = new DropdownField( - 'SubsiteID', - _t('GroupSubsites.SECURITYACCESS', 'Limit CMS access to subsites', PR_MEDIUM, 'Dropdown listing existing subsites which this group has access to'), - $subsiteMap - ); - - if (sizeof($subsiteMap) <= 1) $dropdown = $dropdown->transform(new ReadonlyTransformation()) ; - $tab->push($dropdown); } } @@ -54,10 +95,11 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider { * of the security admin interface. */ function alternateTreeTitle() { - if($this->owner->SubsiteID == 0) { + if($this->owner->AccessAllSubsites) { return $this->owner->Title . ' (global group)'; } else { - return $this->owner->Title; //. ' (' . $this->owner->Subsite()->Title . ')'; + $subsites = Convert::raw2xml(implode(", ", $this->owner->Subsites()->column('Title'))); + return $this->owner->Title . " ($subsites)"; } } @@ -68,13 +110,11 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider { if(Subsite::$disable_subsite_filter) return; if(Cookie::get('noSubsiteFilter') == 'true') return; - if(defined('DB::USE_ANSI_SQL')) - $q="\""; - else $q='`'; - - // If you're querying by ID, ignore the sub-site - this is a bit ugly... - if(!$query->where || (strpos($query->where[0], ".{$q}ID{$q} = ") === false && strpos($query->where[0], ".{$q}ID{$q} = ") === false && strpos($query->where[0], ".{$q}ID{$q} = ") === false)) { + $q = defined('Database::USE_ANSI_SQL') ? "\"" : "`"; + + // If you're querying by ID, ignore the sub-site - this is a bit ugly... + if(!$query->filtersOnID()) { if($context = DataObject::context_obj()) $subsiteID = (int) $context->SubsiteID; else $subsiteID = (int) Subsite::currentSubsiteID(); @@ -84,22 +124,53 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider { $query->where[] = $where; break; } + + // Don't filter by Group_Subsites if we've already done that + $hasGroupSubsites = false; + foreach($query->from as $item) if(strpos($item, 'Group_Subsites') !== false) { + $hasGroupSubsites = true; + break; + } + + if(!$hasGroupSubsites) { + if($subsiteID) { + $query->leftJoin("Group_Subsites", "{$q}Group_Subsites{$q}.{$q}GroupID{$q} + = {$q}Group{$q}.{$q}ID{$q} AND {$q}Group_Subsites{$q}.{$q}SubsiteID{$q} = $subsiteID"); + $query->where[] = "({$q}Group_Subsites{$q}.{$q}SubsiteID{$q} IS NOT NULL OR + {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1)"; + } else { + $query->where[] = "{$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1"; + } + } + $query->orderby = "{$q}AccessAllSubsites{$q} DESC" . ($query->orderby ? ', ' : '') . $query->orderby; } } - function augmentBeforeWrite() { - if((!is_numeric($this->owner->ID) || !$this->owner->ID) && !$this->owner->SubsiteID) $this->owner->SubsiteID = Subsite::currentSubsiteID(); + function onBeforeWrite() { + // New record test approximated by checking whether the ID has changed. + // Note also that the after write test is only used when we're *not* on a subsite + if($this->owner->isChanged('ID') && !Subsite::currentSubsiteID()) { + $this->owner->AccessAllSubsites = 1; + } + } + + function onAfterWrite() { + // New record test approximated by checking whether the ID has changed. + // Note also that the after write test is only used when we're on a subsite + if($this->owner->isChanged('ID') && $currentSubsiteID = Subsite::currentSubsiteID()) { + $subsites = $this->owner->Subsites(); + $subsites->add($currentSubsiteID); + } } function alternateCanEdit() { - // Check the CMS_ACCESS_SecurityAdmin privileges on the subsite that owns this group - $oldSubsiteID = Session::get('SubsiteID'); - - Subsite::changeSubsite($this->owner->SubsiteID) ; - $access = Permission::check('CMS_ACCESS_SecurityAdmin'); - Subsite::changeSubsite($oldSubsiteID) ; - - return $access; + // Find the sites that this group belongs to and the sites where we have appropriate perm. + $accessibleSites = Subsite::accessible_sites('CMS_ACCESS_SecurityAdmin')->column('ID'); + $linkedSites = $this->owner->Subsites()->column('ID'); + + // We are allowed to access this site if at we have CMS_ACCESS_SecurityAdmin permission on + // at least one of the sites + return (bool)array_intersect($accessibleSites, $linkedSites); } /** @@ -117,10 +188,10 @@ class GroupSubsites extends DataObjectDecorator implements PermissionProvider { $group = $this->owner->duplicate(false); - $subsiteID = ($subsiteID ? $subsiteID : Subsite::currentSubsiteID()); - $group->SubsiteID = $subsiteID; $group->write(); + $subsite->Groups()->add($group->ID); + // Duplicate permissions $permissions = $this->owner->Permissions(); foreach($permissions as $permission) { diff --git a/code/LeftAndMainSubsites.php b/code/LeftAndMainSubsites.php index 24126ea..cba583d 100644 --- a/code/LeftAndMainSubsites.php +++ b/code/LeftAndMainSubsites.php @@ -13,20 +13,6 @@ class LeftAndMainSubsites extends Extension { Requirements::css('subsites/css/LeftAndMain_Subsites.css'); Requirements::javascript('subsites/javascript/LeftAndMain_Subsites.js'); Requirements::javascript('subsites/javascript/VirtualPage_Subsites.js'); - - // 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. - $sites = Subsite::accessible_sites("CMS_ACCESS_{$this->owner->class}")->toDropdownMap(); - if($sites && !isset($sites[Subsite::currentSubsiteID()])) { - $siteIDs = array_keys($sites); - Subsite::changeSubsite($siteIDs[0]); - } } /** @@ -131,16 +117,24 @@ class LeftAndMainSubsites extends Extension { public function alternateAccessCheck() { $className = $this->owner->class; - if($result = Permission::check("CMS_ACCESS_$className")) { - return $result; - } else { - $otherSites = Subsite::accessible_sites("CMS_ACCESS_$className"); - if($otherSites && $otherSites->TotalItems() > 0) { - $otherSites->First()->activate(); - return Permission::check("CMS_ACCESS_$className"); + + // 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. + $sites = Subsite::accessible_sites("CMS_ACCESS_{$this->owner->class}")->toDropdownMap(); + if($sites && !isset($sites[Subsite::currentSubsiteID()])) { + $siteIDs = array_keys($sites); + Subsite::changeSubsite($siteIDs[0]); + return true; + } + + if(!$sites) return null; + return null; } diff --git a/code/SiteTreeSubsites.php b/code/SiteTreeSubsites.php index 57e3714..e288de3 100644 --- a/code/SiteTreeSubsites.php +++ b/code/SiteTreeSubsites.php @@ -232,16 +232,11 @@ class SiteTreeSubsites extends SiteTreeDecorator { function canEdit($member = null) { if(!$member) $member = Member::currentUser(); - // Check the CMS_ACCESS_CMSMain privileges on the subsite that owns this group - $oldSubsiteID = Session::get('SubsiteID'); - - Subsite::changeSubsite($this->owner->SubsiteID) ; - $access = Permission::checkMember($member, 'CMS_ACCESS_CMSMain'); - Subsite::changeSubsite($oldSubsiteID); + // Find the sites that this user has access to + $goodSites = Subsite::accessible_sites('CMS_ACCESS_CMSMain',true,'all',$member)->column('ID'); - if(!$access) $access = Permission::checkMember($member, 'SUBSITE_ACCESS_ALL'); - - return $access; + // Return true if they have access to this object's site + return in_array(0, $goodSites) || in_array($this->owner->SubsiteID, $goodSites); } /** diff --git a/code/Subsite.php b/code/Subsite.php index 3741ff6..cd299e2 100644 --- a/code/Subsite.php +++ b/code/Subsite.php @@ -34,6 +34,10 @@ class Subsite extends DataObject implements PermissionProvider { static $has_many = array( 'Domains' => 'SubsiteDomain', ); + + static $belongs_many_many = array( + "Groups" => "Group", + ); static $defaults = array( 'IsPublic' => 1, @@ -353,9 +357,11 @@ JS; $groupCount = DB::query(" SELECT COUNT({$q}Permission{$q}.{$q}ID{$q}) FROM {$q}Permission{$q} - INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Permission{$q}.{$q}GroupID{$q} AND {$q}Group{$q}.{$q}SubsiteID{$q} = 0 + INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Permission{$q}.{$q}GroupID{$q} AND {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1 INNER JOIN {$q}Group_Members{$q} USING({$q}GroupID{$q}) - WHERE {$q}Permission{$q}.{$q}Code{$q} IN ('$SQL_perms') AND {$q}MemberID{$q} = {$memberID} + WHERE + {$q}Permission{$q}.{$q}Code{$q} IN ('$SQL_perms') + AND {$q}MemberID{$q} = {$memberID} ")->value(); return ($groupCount > 0); @@ -420,13 +426,17 @@ JS; * @param $includeMainSite If true, the main site will be included if appropriate. * @param $mainSiteTitle The label to give to the main site */ - function accessible_sites($permCode, $includeMainSite = false, $mainSiteTitle = "Main site") { - $member = Member::currentUser(); + function accessible_sites($permCode, $includeMainSite = false, $mainSiteTitle = "Main site", $member = null) { + // For 2.3 and 2.4 compatibility + $q = defined('Database::USE_ANSI_SQL') ? "\"" : "`"; + + // Rationalise member arguments + if(!$member) $member = Member::currentUser(); + if(!$member) return new DataObjectSet(); + if(!is_object($member)) $member = DataObject::get_by_id('Member', $member); if(is_array($permCode)) $SQL_codes = "'" . implode("', '", Convert::raw2sql($permCode)) . "'"; else $SQL_codes = "'" . Convert::raw2sql($permCode) . "'"; - - if(!$member) return new DataObjectSet(); $templateClassList = "'" . implode("', '", ClassInfo::subclassesFor("Subsite_Template")) . "'"; @@ -436,26 +446,38 @@ JS; return DataObject::get( 'Subsite', - "{$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID - AND {$q}Permission{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN') - AND {$q}Subsite{$q}.{$q}Title{$q} != ''", + "{$q}Subsite{$q}.{$q}Title{$q} != ''", '', - "LEFT JOIN {$q}Group{$q} ON ({$q}SubsiteID{$q}={$q}Subsite{$q}.{$q}ID{$q} OR {$q}SubsiteID{$q} = 0) - LEFT JOIN {$q}Group_Members{$q} ON {$q}Group_Members{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q} - LEFT JOIN {$q}Permission{$q} ON {$q}Group{$q}.{$q}ID{$q}={$q}Permission{$q}.{$q}GroupID{$q}" + "LEFT JOIN {$q}Group_Subsites{$q} + ON {$q}Group_Subsites{$q}.{$q}SubsiteID{$q} = {$q}Subsite{$q}.{$q}ID{$q} + INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Group_Subsites{$q}.{$q}GroupID{$q} + OR {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1 + INNER JOIN {$q}Group_Members{$q} + ON {$q}Group_Members{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q} + AND {$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID + INNER JOIN {$q}Permission{$q} + ON {$q}Group{$q}.{$q}ID{$q}={$q}Permission{$q}.{$q}GroupID{$q} + AND {$q}Permission{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN')" ); $rolesSubsites = DataObject::get( 'Subsite', - "{$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID - AND {$q}PermissionRoleCode{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN') - AND {$q}Subsite{$q}.Title != ''", + "{$q}Subsite{$q}.Title != ''", '', - "LEFT JOIN {$q}Group{$q} ON ({$q}SubsiteID{$q}={$q}Subsite{$q}.{$q}ID{$q} OR {$q}SubsiteID{$q} = 0) - LEFT JOIN {$q}Group_Members{$q} ON {$q}Group_Members{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q} - LEFT JOIN {$q}Group_Roles{$q} ON {$q}Group_Roles{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q} - LEFT JOIN {$q}PermissionRole{$q} ON {$q}Group_Roles{$q}.{$q}PermissionRoleID{$q}={$q}PermissionRole{$q}.{$q}ID{$q} - LEFT JOIN {$q}PermissionRoleCode{$q} ON {$q}PermissionRole{$q}.{$q}ID{$q}={$q}PermissionRoleCode{$q}.{$q}RoleID{$q}" + "LEFT JOIN {$q}Group_Subsites{$q} + ON {$q}Group_Subsites{$q}.{$q}SubsiteID{$q} = {$q}Subsite{$q}.{$q}ID{$q} + INNER JOIN {$q}Group{$q} ON {$q}Group{$q}.{$q}ID{$q} = {$q}Group_Subsites{$q}.{$q}GroupID{$q} + OR {$q}Group{$q}.{$q}AccessAllSubsites{$q} = 1 + INNER JOIN {$q}Group_Members{$q} + ON {$q}Group_Members{$q}.{$q}GroupID$q={$q}Group{$q}.{$q}ID{$q} + AND {$q}Group_Members{$q}.{$q}MemberID{$q} = $member->ID + INNER JOIN {$q}Group_Roles{$q} + ON {$q}Group_Roles{$q}.{$q}GroupID{$q}={$q}Group{$q}.{$q}ID{$q} + INNER JOIN {$q}PermissionRole{$q} + ON {$q}Group_Roles{$q}.{$q}PermissionRoleID{$q}={$q}PermissionRole{$q}.{$q}ID{$q} + INNER JOIN {$q}PermissionRoleCode{$q} + ON {$q}PermissionRole{$q}.{$q}ID{$q}={$q}PermissionRoleCode{$q}.{$q}RoleID{$q} + AND {$q}PermissionRoleCode{$q}.{$q}Code{$q} IN ($SQL_codes, 'ADMIN')" ); if(!$subsites && $rolesSubsites) return $rolesSubsites; @@ -603,7 +625,8 @@ class Subsite_Template extends Subsite { * Copy groups from the template to the given subsites. Each of the groups will be created and left * empty. */ - $groups = DataObject::get("Group", "{$q}SubsiteID{$q} = '$this->ID'"); + + $groups = $this->Groups(); if($groups) foreach($groups as $group) { $group->duplicateToSubsite($intranet); } diff --git a/javascript/LeftAndMain_Subsites.js b/javascript/LeftAndMain_Subsites.js index 97f1a9c..314f7fb 100644 --- a/javascript/LeftAndMain_Subsites.js +++ b/javascript/LeftAndMain_Subsites.js @@ -59,6 +59,22 @@ Behaviour.register({ return false; } + }, + + // Subsite tab of Group editor + '#Form_EditForm_AccessAllSubsites' : { + initialize: function () { + this.showHideSubsiteList(); + var i=0,items=this.getElementsByTagName('input'); + for(i=0;iobjFromFixture('Member', 'subsite1member')); + $member1SiteTitles = $member1Sites->column("Title"); + sort($member1SiteTitles); + $this->assertEquals(array('Subsite1 Template'), $member1SiteTitles); + + $adminSites = Subsite::accessible_sites("CMS_ACCESS_CMSMain", false, null, + $this->objFromFixture('Member', 'admin')); + $adminSiteTitles = $adminSites->column("Title"); + sort($adminSiteTitles); + $this->assertEquals(array( + 'Subsite1 Template', + 'Subsite2 Template', + 'Template', + 'Test 1', + 'Test 2', + 'Test 3', + ), $adminSiteTitles); + } + /** * Publish a change on a master page of a newly created sub-site, and verify that the change has been propagated. * Verify that if CustomContent is set, then the changes aren't propagated. diff --git a/tests/SubsiteTest.yml b/tests/SubsiteTest.yml index a6722dd..dc0bf7d 100644 --- a/tests/SubsiteTest.yml +++ b/tests/SubsiteTest.yml @@ -72,14 +72,17 @@ Group: admin: Title: Admin Code: admin + AccessAllSubsites: 1 subsite1_group: Title: subsite1_group Code: subsite1_group - SubsiteID: =>Subsite_Template.subsite1 + AccessAllSubsites: 0 + Subsites: =>Subsite_Template.subsite1 subsite2_group: Title: subsite2_group Code: subsite2_group - SubsiteID: =>Subsite_Template.subsite2 + AccessAllSubsites: 0 + Subsites: =>Subsite_Template.subsite2 Permission: admin: Code: ADMIN