Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Member::onChangeGroups() should allow ADMIN permission grant if logged in user is admin #375

Merged
merged 1 commit into from

2 participants

@halkyon
Owner

Member::onChangeGroups() now checks for current logged in member as admin, so if the logged in user is has ADMIN permissions, they can grant ADMIN permissions to non-admin members.

I found this bug when on SS 2.4, a logged in admin couldn't grant a new member with admin permissions.

@sminnee sminnee merged commit cbc5d3c into silverstripe:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 27, 2012
  1. @halkyon

    BUGFIX Member::onChangeGroups() should allow ADMIN permission grant i…

    halkyon authored
    …f the logged in user is an ADMIN
This page is out of date. Refresh to see the latest.
Showing with 11 additions and 3 deletions.
  1. +3 −3 security/Member.php
  2. +8 −0 tests/security/MemberTest.php
View
6 security/Member.php
@@ -702,9 +702,9 @@ function onAfterWrite() {
* @return boolean
*/
function onChangeGroups($ids) {
- // Filter out admin groups to avoid privilege escalation,
- // unless the current user is an admin already
- if(!Permission::checkMember($this, 'ADMIN')) {
+ // Filter out admin groups to avoid privilege escalation,
+ // unless the current user is an admin already OR the logged in user is an admin
+ if(!(Permission::check('ADMIN') || Permission::checkMember($this, 'ADMIN'))) {
$adminGroups = Permission::get_groups_by_permission('ADMIN');
$adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array();
return count(array_intersect($ids, $adminGroupIDs)) == 0;
View
8 tests/security/MemberTest.php
@@ -551,6 +551,14 @@ function testOnChangeGroups() {
$staffMember->onChangeGroups(array($newAdminGroup->ID)),
'Adding new admin group relation is not allowed for non-admin members'
);
+
+ $this->session()->inst_set('loggedInAs', $adminMember->ID);
+ $this->assertTrue(
+ $staffMember->onChangeGroups(array($newAdminGroup->ID)),
+ 'Adding new admin group relation is allowed for normal users, when granter is logged in as admin'
+ );
+ $this->session()->inst_set('loggedInAs', null);
+
$this->assertTrue(
$adminMember->onChangeGroups(array($newAdminGroup->ID)),
'Adding new admin group relation is allowed for admin members'
Something went wrong with that request. Please try again.