From 4822c6894746cd077af1515c5bde8e75ea059c09 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 3 Feb 2009 22:44:11 +0000 Subject: [PATCH] BUGFIX Don't require ADMIN permissions to view an administrators group - rather set it to readonly through interfaces like SecurityAdmin ENHANCEMENT Modified Group->canEdit() to check for CMS_ACCESS_SecurityAdmin permissions codes (see r70697) BUGFIX Using canView() instead of canEdit() in Group->AllChildrenIncludingDeleted() git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@71320 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- security/Group.php | 49 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/security/Group.php b/security/Group.php index 41027d656..8cdb91133 100644 --- a/security/Group.php +++ b/security/Group.php @@ -271,19 +271,46 @@ class Group extends DataObject { } } - public function canEdit() { - if($this->hasMethod('alternateCanEdit')) return $this->alternateCanEdit(); - else { - return Permission::check("ADMIN") - || (Member::currentUserID() && !DataObject::get("Permission", "GroupID = $this->ID AND Code = 'ADMIN'")); + /** + * Checks for permission-code CMS_ACCESS_SecurityAdmin. + * If the group has ADMIN permissions, it requires the user to have ADMIN permissions as well. + * + * @param $member Member + * @return boolean + */ + public function canEdit($member = null) { + if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); + + if($this->hasMethod('alternateCanEdit')) { + return $this->alternateCanEdit($member); + } else { + return ( + // either we have an ADMIN + (bool)Permission::checkMember($member, "ADMIN") + || ( + // or a privileged CMS user and a group without ADMIN permissions. + // without this check, a user would be able to add himself to an administrators group + // with just access to the "Security" admin interface + Permission::checkMember($member, "CMS_ACCESS_SecurityAdmin") && + !DataObject::get("Permission", "GroupID = $this->ID AND Code = 'ADMIN'") + ) + ); } } - public function canView() { - if($this->hasMethod('alternateCanView')) return $this->alternateCanView(); - else { - return Permission::check("ADMIN") - || (Member::currentUserID() && !DataObject::get("Permission", "GroupID = $this->ID AND Code = 'ADMIN'")); + /** + * Checks for permission-code CMS_ACCESS_SecurityAdmin. + * + * @param $member Member + * @return boolean + */ + public function canView($member = null) { + if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); + + if($this->hasMethod('alternateCanView')) { + return $this->alternateCanView($member); + } else { + return (bool)Permission::checkMember($member, "CMS_ACCESS_SecurityAdmin"); } } @@ -296,7 +323,7 @@ class Group extends DataObject { $filteredChildren = new DataObjectSet(); if($children) foreach($children as $child) { - if($child->canEdit()) $filteredChildren->push($child); + if($child->canView()) $filteredChildren->push($child); } return $filteredChildren;