From bd2b9efede59acc53b0bb506afc8634be5115f1b Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 3 Feb 2009 23:33:28 +0000 Subject: [PATCH] API CHANGE Member->canView() checks for ADMIN or CMS_ACCESS_SecurityAdmin access if not viewing the currently logged-in member. If permissions are enforced in custom interfaces (e.g. social networking frontends), this will impact the output. To loosen permissions, override or decorate Member->canView() ENHANCEMENT Added Group->canDelete() AND Member->canView() ENHANCEMENT Making Member->can*() and Group->can*() methods decoratable git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@71327 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- security/Group.php | 63 +++++++++++++++++++++++++++++++-------------- security/Member.php | 51 +++++++++++++++++++++++++++++++++--- 2 files changed, 92 insertions(+), 22 deletions(-) diff --git a/security/Group.php b/security/Group.php index 8cdb91133..1f1eecc9a 100644 --- a/security/Group.php +++ b/security/Group.php @@ -281,21 +281,29 @@ class Group extends DataObject { 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'") - ) - ); + // DEPRECATED 2.3: use canView() instead + $results = $this->extend('alternateCanView', $member); + if($results && is_array($results)) if(!min($results)) return false; + + // decorated access checks + $results = $this->extend('canEdit', $member); + if($results && is_array($results)) if(!min($results)) return false; + + if( + // 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'") + ) + ) { + return true; } + + return false; } /** @@ -307,11 +315,28 @@ class Group extends DataObject { 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"); - } + // DEPRECATED 2.3: use canView() instead + $results = $this->extend('alternateCanView', $member); + if($results && is_array($results)) if(!min($results)) return false; + + // decorated access checks + $results = $this->extend('canView', $member); + if($results && is_array($results)) if(!min($results)) return false; + + // user needs access to CMS_ACCESS_SecurityAdmin + if(Permission::checkMember($member, "CMS_ACCESS_SecurityAdmin")) return true; + + return false; + } + + public function canDelete($member = null) { + if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); + + // decorated access checks + $results = $this->extend('canDelete', $member); + if($results && is_array($results)) if(!min($results)) return false; + + return $this->canEdit($member); } /** diff --git a/security/Member.php b/security/Member.php index bb477f25a..8484ee65c 100644 --- a/security/Member.php +++ b/security/Member.php @@ -891,12 +891,57 @@ class Member extends DataObject { } } + /** + * Users can view their own record. + * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions. + * This is likely to be customized for social sites etc. with a looser permission model. + */ + function canView($member = null) { + if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); + + // decorated access checks + $results = $this->extend('canView', $member); + if($results && is_array($results)) if(!min($results)) return false; + + // members can usually edit their own record + if($this->ID == $member->ID) return true; + + if( + Permission::checkMember($member, 'ADMIN') + || Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin') + ) { + return true; + } + + return false; + } + + /** + * Users can edit their own record. + * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions + */ function canEdit($member = null) { - if(!$member && $member !== FALSE) $member = Member::currentUser(); + if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); - if($this->ID == Member::currentUserID()) return true; + // decorated access checks + $results = $this->extend('canEdit', $member); + if($results && is_array($results)) if(!min($results)) return false; - return Permission::check('ADMIN'); + return $this->canView($member); + } + + /** + * Users can edit their own record. + * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions + */ + function canDelete($member = null) { + if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); + + // decorated access checks + $results = $this->extend('canDelete', $member); + if($results && is_array($results)) if(!min($results)) return false; + + return $this->canEdit($member); }