tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-10-22 14:05:37 +02:00
Code Issues Projects Releases Wiki Activity

BUGFIX Avoid privilege escalation from EDIT_PERMISSIONS to ADMIN through TreeMultiselectField (in Member->getCMSFields()) by checking for admin groups in Member->onChangeGroups()

Browse Source
This commit is contained in:
Ingo Schommer 2011-03-09 15:49:41 +13:00
parent 6d6c294ae3
commit 5bc0d008e9
3 changed files with 81 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
security/Member.php
View File

@ -542,6 +542,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;
}
}
/**

29
tests/security/MemberTest.php
View File

@ -260,4 +260,33 @@ class MemberTest extends SapphireTest {
'Non-existant group returns false'
);
}
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'
);
}
}

36
tests/security/MemberTest.yml
View File

@ -1,4 +1,17 @@
Permission:
admin:
Code: ADMIN
security-admin:
Code: CMS_ACCESS_SecurityAdmin
Group:
admingroup:
Title: Admin
Code: admin
Permissions: =>Permission.admin
securityadminsgroup:
Title: securityadminsgroup
Code: securityadminsgroup
Permissions: =>Permission.security-admin
staffgroup:
Title: staffgroup
Code: staffgroup
@ -13,14 +26,26 @@ Group:
ceogroup:
Title: ceogroup
Code: ceogroup
Parent: =>Group.managementgroup
Parent: =>Group.managementgroup
memberlessgroup:
Title: Memberless Group
code: memberless
Member:
admin:
FirstName: Admin
Email: admin@silverstripe.com
Groups: =>Group.admingroup
other-admin:
FirstName: OtherAdmin
Email: other-admin@silverstripe.com
Groups: =>Group.admingroup
test:
FirstName: Test
Surname: User
Email: sam@silverstripe.com
Password: 1nitialPassword
PasswordExpiry: 2030-01-01
Groups: =>Group.securityadminsgroup
expiredpassword:
FirstName: Test
Surname: User
@ -43,4 +68,11 @@ Member:
Groups: =>Group.accountinggroup
ceomember:
Email: ceomember@test.com
Groups: =>Group.ceogroup
Groups: =>Group.ceogroup
grouplessmember:
FirstName: Groupless Member
noformatmember:
Email: noformat@test.com
delocalemember:
Email: delocalemember@test.com
Locale: de_DE