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

API CHANGE Member->canEdit() returns false if the editing member has lower permissions than the edited member, for example if a member with CMS_ACCESS_SecurityAdmin permissions tries to edit an ADMIN (fixes #5651)

Browse Source 
git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.4@110856 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Ingo Schommer 2010-09-16 05:20:09 +00:00 committed by Sam Minnee
parent 12c3bfb1fb
commit 486091e4ec
3 changed files with 51 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
security/Member.php
View File

@ -1280,6 +1280,13 @@ class Member extends DataObject {
// No member found
if(!($member && $member->exists())) return false;
// If the requesting member is not an admin, but has access to manage members,
// he still can't edit other members with ADMIN permission.
// This is a bit weak, strictly speaking he 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);
}

42
tests/security/MemberTest.php
View File

@ -288,19 +288,19 @@ class MemberTest extends FunctionalTest {
$this->assertFalse($member->isPasswordExpired());
}
function testMemberWithNoDateFormatFallsbackToGlobalLocaleDefaultFormat() {
$member = $this->objFromFixture('Member', 'noformatmember');
$this->assertEquals('MMM d, yyyy', $member->DateFormat);
$this->assertEquals('h:mm:ss a', $member->TimeFormat);
}
function testMemberWithNoDateFormatFallsbackToTheirLocaleDefaultFormat() {
$member = $this->objFromFixture('Member', 'delocalemember');
$this->assertEquals('dd.MM.yyyy', $member->DateFormat);
$this->assertEquals('HH:mm:ss', $member->TimeFormat);
}
function testInGroups() {
$staffmember = $this->objFromFixture('Member', 'staffmember');
$managementmember = $this->objFromFixture('Member', 'managementmember');
@ -332,9 +332,9 @@ class MemberTest extends FunctionalTest {
$this->assertFalse($grouplessMember->Groups()->exists());
$this->assertFalse($memberlessGroup->Members()->exists());
$grouplessMember->addToGroupByCode('memberless');
$this->assertEquals($memberlessGroup->Members()->Count(), 1);
$this->assertEquals($grouplessMember->Groups()->Count(), 1);
@ -400,7 +400,7 @@ class MemberTest extends FunctionalTest {
'Non-existant group returns false'
);
}
/**
* Tests that the user is able to view their own record, and in turn, they can
* edit and delete their own record too.
@ -428,11 +428,11 @@ class MemberTest extends FunctionalTest {
$this->assertFalse($member->canView());
$this->assertFalse($member->canDelete());
$this->assertFalse($member->canEdit());
$this->addExtensions($extensions);
$this->session()->inst_set('loggedInAs', null);
}
public function testAuthorisedMembersCanManipulateOthersRecords() {
$extensions = $this->removeExtensions(Object::get_extensions('Member'));
$member = $this->objFromFixture('Member', 'test');
@ -447,7 +447,7 @@ class MemberTest extends FunctionalTest {
$this->addExtensions($extensions);
$this->session()->inst_set('loggedInAs', null);
}
public function testDecoratedCan() {
$extensions = $this->removeExtensions(Object::get_extensions('Member'));
$member = $this->objFromFixture('Member', 'test');
@ -464,7 +464,7 @@ class MemberTest extends FunctionalTest {
$this->assertTrue($member2->canView());
$this->assertFalse($member2->canDelete());
$this->assertFalse($member2->canEdit());
/* Apply a decorator that denies viewing of the Member */
Object::remove_extension('Member', 'MemberTest_ViewingAllowedExtension');
Object::add_extension('Member', 'MemberTest_ViewingDeniedExtension');
@ -473,7 +473,7 @@ class MemberTest extends FunctionalTest {
$this->assertFalse($member3->canView());
$this->assertFalse($member3->canDelete());
$this->assertFalse($member3->canEdit());
/* Apply a decorator that allows viewing and editing but denies deletion */
Object::remove_extension('Member', 'MemberTest_ViewingDeniedExtension');
Object::add_extension('Member', 'MemberTest_EditingAllowedDeletingDeniedExtension');
@ -486,7 +486,7 @@ class MemberTest extends FunctionalTest {
Object::remove_extension('Member', 'MemberTest_EditingAllowedDeletingDeniedExtension');
$this->addExtensions($extensions);
}
/**
* Tests for {@link Member::getName()} and {@link Member::setName()}
*/
@ -500,6 +500,24 @@ class MemberTest extends FunctionalTest {
$member->Surname = '';
$this->assertEquals('Test', $member->getName());
}
function testMembersWithSecurityAdminAccessCantEditAdminsUnlessTheyreAdminsThemselves() {
$adminMember = $this->objFromFixture('Member', 'admin');
$otherAdminMember = $this->objFromFixture('Member', 'other-admin');
$securityAdminMember = $this->objFromFixture('Member', 'test');
$ceoMember = $this->objFromFixture('Member', 'ceomember');
// Careful: Don't read as english language.
// More precisely this should read canBeEditedBy()
$this->assertTrue($adminMember->canEdit($adminMember), 'Admins can edit themselves');
$this->assertTrue($otherAdminMember->canEdit($adminMember), 'Admins can edit other admins');
$this->assertTrue($securityAdminMember->canEdit($adminMember), 'Admins can edit other members');
$this->assertTrue($securityAdminMember->canEdit($securityAdminMember), 'Security-Admins can edit themselves');
$this->assertFalse($adminMember->canEdit($securityAdminMember), 'Security-Admins can not edit other admins');
$this->assertTrue($ceoMember->canEdit($securityAdminMember), 'Security-Admins can edit other members');
}
/**
* Add the given array of member extensions as class names.

14
tests/security/MemberTest.yml
View File

@ -1,7 +1,13 @@
Permission:
admin:
Code: ADMIN
security-admin:
Code: CMS_ACCESS_SecurityAdmin
Group:
admingroup:
Title: Admin
Code: admin
Permissions: =>Permission.admin
securityadminsgroup:
Title: securityadminsgroup
Code: securityadminsgroup
@ -25,6 +31,14 @@ Group:
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