Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

BUG Members should not be allowed to delete themselves (fixes #8121)

  • Loading branch information...
commit 22eeaa4ac10622f616063af9fb76c240aa037d79 1 parent bf5590d
@chillu chillu authored
Showing with 35 additions and 3 deletions.
  1. +6 −2 security/Member.php
  2. +29 −1 tests/security/MemberTest.php
View
8 security/Member.php
@@ -1294,16 +1294,20 @@ public function canEdit($member = null) {
*/
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);
}
View
30 tests/security/MemberTest.php
@@ -432,7 +432,7 @@ public function testCanManipulateOwnRecord() {
/* Logged in users can edit their own record */
$this->session()->inst_set('loggedInAs', $member->ID);
$this->assertTrue($member->canView());
- $this->assertTrue($member->canDelete());
+ $this->assertFalse($member->canDelete());
$this->assertTrue($member->canEdit());
/* Other uses cannot view, delete or edit others records */
@@ -653,6 +653,34 @@ public function testValidateAutoLoginToken() {
$this->assertFalse($m2->validateAutoLoginToken($m1Token), 'Fails token validity test against other member.');
}
+ public function testCanDelete() {
+ $admin1 = $this->objFromFixture('Member', 'admin');
+ $admin2 = $this->objFromFixture('Member', 'other-admin');
+ $member1 = $this->objFromFixture('Member', 'grouplessmember');
+ $member2 = $this->objFromFixture('Member', 'noformatmember');
+
+ $this->assertTrue(
+ $admin1->canDelete($admin2),
+ 'Admins can delete other admins'
+ );
+ $this->assertTrue(
+ $member1->canDelete($admin2),
+ 'Admins can delete non-admins'
+ );
+ $this->assertFalse(
+ $admin1->canDelete($admin1),
+ 'Admins can not delete themselves'
+ );
+ $this->assertFalse(
+ $member1->canDelete($member2),
+ 'Non-admins can not delete other non-admins'
+ );
+ $this->assertFalse(
+ $member1->canDelete($member1),
+ 'Non-admins can not delete themselves'
+ );
+ }
+
}
class MemberTest_ViewingAllowedExtension extends DataExtension implements TestOnly {
Please sign in to comment.
Something went wrong with that request. Please try again.