diff --git a/security/Member.php b/security/Member.php index 6b9da5ff0..69d1562a6 100644 --- a/security/Member.php +++ b/security/Member.php @@ -1464,85 +1464,99 @@ class Member extends DataObject implements TemplateGlobalProvider { return $labels; } - /** - * 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. - */ - public function canView($member = null) { - if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); + /** + * 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. + */ + public function canView($member = null) { + //get member + if(!($member instanceof Member)) { + $member = Member::currentUser(); + } + //check for extensions, we do this first as they can overrule everything + $extended = $this->extendedCan(__FUNCTION__, $member); + if($extended !== null) { + return $extended; + } - // extended access checks - $results = $this->extend('canView', $member); - if($results && is_array($results)) { - if(!min($results)) return false; - else return true; - } + //need to be logged in and/or most checks below rely on $member being a Member + if(!$member) { + return false; + } + // members can usually view their own record + if($this->ID == $member->ID) { + return true; + } + //standard check + return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin'); + } + /** + * Users can edit their own record. + * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions + */ + public function canEdit($member = null) { + //get member + if(!($member instanceof Member)) { + $member = Member::currentUser(); + } + //check for extensions, we do this first as they can overrule everything + $extended = $this->extendedCan(__FUNCTION__, $member); + if($extended !== null) { + return $extended; + } - // members can usually edit their own record - if($member && $this->ID == $member->ID) return true; + //need to be logged in and/or most checks below rely on $member being a Member + if(!$member) { + return false; + } - if( - Permission::checkMember($member, 'ADMIN') - || Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin') - ) { - return true; - } + // HACK: we should not allow for an non-Admin to edit an Admin + if(!Permission::checkMember($member, 'ADMIN') && Permission::checkMember($this, 'ADMIN')) { + return false; + } + // members can usually edit their own record + if($this->ID == $member->ID) { + return true; + } + //standard check + return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin'); + } + /** + * Users can edit their own record. + * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions + */ + public function canDelete($member = null) { + if(!($member instanceof Member)) { + $member = Member::currentUser(); + } + //check for extensions, we do this first as they can overrule everything + $extended = $this->extendedCan(__FUNCTION__, $member); + if($extended !== null) { + return $extended; + } - return false; - } - - /** - * Users can edit their own record. - * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions - */ - public function canEdit($member = null) { - if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); - - // extended access checks - $results = $this->extend('canEdit', $member); - if($results && is_array($results)) { - if(!min($results)) return false; - else return true; - } - - // No member found - if(!($member && $member->exists())) return false; - - // If the requesting member is not an admin, but has access to manage members, - // they still can't edit other members with ADMIN permission. - // This is a bit weak, strictly speaking they shouldn't be allowed to - // perform any action that could change the password on a member - // with "higher" permissions than himself, but thats hard to determine. - if(!Permission::checkMember($member, 'ADMIN') && Permission::checkMember($this, 'ADMIN')) return false; - - return $this->canView($member); - } - - /** - * Users can edit their own record. - * Otherwise they'll need ADMIN or CMS_ACCESS_SecurityAdmin permissions - */ - public function canDelete($member = null) { - if(!$member || !(is_a($member, 'Member')) || is_numeric($member)) $member = Member::currentUser(); - - // extended access checks - $results = $this->extend('canDelete', $member); - if($results && is_array($results)) { - if(!min($results)) return false; - else return true; - } - - // No member found - if(!($member && $member->exists())) return false; - - // Members are not allowed to remove themselves, - // since it would create inconsistencies in the admin UIs. - if($this->ID && $member->ID == $this->ID) return false; - - return $this->canEdit($member); - } + //need to be logged in and/or most checks below rely on $member being a Member + if(!$member) { + return false; + } + // Members are not allowed to remove themselves, + // since it would create inconsistencies in the admin UIs. + if($this->ID && $member->ID == $this->ID) { + return false; + } + // HACK: if you want to delete a member, you have to be a member yourself. + // this is a hack because what this should do is to stop a user + // deleting a member who has more privileges (e.g. a non-Admin deleting an Admin) + if(Permission::checkMember($this, 'ADMIN')) { + if( ! Permission::checkMember($member, 'ADMIN')) { + return false; + } + } + //standard check + return Permission::checkMember($member, 'CMS_ACCESS_SecurityAdmin'); + } /** * Validate this member object.