mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
BUGFIX Avoid privilege escalation from EDIT_PERMISSIONS to ADMIN through TreeMultiselectField (in Member->getCMSFields()) by checking for admin groups in Member->onChangeGroups()
This commit is contained in:
parent
5d87f2929e
commit
de1f07045b
@ -693,6 +693,24 @@ class Member extends DataObject {
|
||||
MemberPassword::log($this);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* If any admin groups are requested, deny the whole save operation.
|
||||
*
|
||||
* @param Array $ids Database IDs of Group records
|
||||
* @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')) {
|
||||
$adminGroups = Permission::get_groups_by_permission('ADMIN');
|
||||
$adminGroupIDs = ($adminGroups) ? $adminGroups->column('ID') : array();
|
||||
return count(array_intersect($ids, $adminGroupIDs)) == 0;
|
||||
} else {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
|
@ -527,6 +527,35 @@ class MemberTest extends FunctionalTest {
|
||||
$this->assertFalse($adminMember->canEdit($securityAdminMember), 'Security-Admins can not edit other admins');
|
||||
$this->assertTrue($ceoMember->canEdit($securityAdminMember), 'Security-Admins can edit other members');
|
||||
}
|
||||
|
||||
function testOnChangeGroups() {
|
||||
$staffGroup = $this->objFromFixture('Group', 'staffgroup');
|
||||
$adminGroup = $this->objFromFixture('Group', 'admingroup');
|
||||
$staffMember = $this->objFromFixture('Member', 'staffmember');
|
||||
$adminMember = $this->objFromFixture('Member', 'admin');
|
||||
$newAdminGroup = new Group(array('Title' => 'newadmin'));
|
||||
$newAdminGroup->write();
|
||||
Permission::grant($newAdminGroup->ID, 'ADMIN');
|
||||
$newOtherGroup = new Group(array('Title' => 'othergroup'));
|
||||
$newOtherGroup->write();
|
||||
|
||||
$this->assertTrue(
|
||||
$staffMember->onChangeGroups(array($staffGroup->ID)),
|
||||
'Adding existing non-admin group relation is allowed for non-admin members'
|
||||
);
|
||||
$this->assertTrue(
|
||||
$staffMember->onChangeGroups(array($newOtherGroup->ID)),
|
||||
'Adding new non-admin group relation is allowed for non-admin members'
|
||||
);
|
||||
$this->assertFalse(
|
||||
$staffMember->onChangeGroups(array($newAdminGroup->ID)),
|
||||
'Adding new admin group relation is not allowed for non-admin members'
|
||||
);
|
||||
$this->assertTrue(
|
||||
$adminMember->onChangeGroups(array($newAdminGroup->ID)),
|
||||
'Adding new admin group relation is allowed for admin members'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Add the given array of member extensions as class names.
|
||||
|
Loading…
Reference in New Issue
Block a user