diff --git a/code/AuditHookManyManyList.php b/code/AuditHookManyManyList.php index 513079a..e36fde3 100644 --- a/code/AuditHookManyManyList.php +++ b/code/AuditHookManyManyList.php @@ -8,6 +8,10 @@ use SilverStripe\Security\Member; use SilverStripe\Security\Security; +/** + * AuditHookManyManyList is meant to override ManyManyList. When a Member is + * removed from a Group, it logs the event. + */ class AuditHookManyManyList extends ManyManyList { /** @@ -18,7 +22,9 @@ public function removeByID($itemID) { parent::removeByID($itemID); - if ($this->getJoinTable() == 'Group_Members') { + $memberGroupJoinTable = Member::singleton()->Groups()->getJoinTable(); + + if ($this->getJoinTable() === $memberGroupJoinTable) { $currentMember = Security::getCurrentUser(); if (!($currentMember && $currentMember->exists())) { return; diff --git a/code/AuditHookMemberGroupSet.php b/code/AuditHookMemberGroupSet.php index 197fb13..fed662d 100644 --- a/code/AuditHookMemberGroupSet.php +++ b/code/AuditHookMemberGroupSet.php @@ -8,6 +8,10 @@ use SilverStripe\Security\Member_GroupSet; use SilverStripe\Security\Security; +/** + * AuditHookMemberGroupSet is meant to override Member_GroupSet. When a Group + * is removed from a Member, it logs the event. + */ class AuditHookMemberGroupSet extends Member_GroupSet { /** @@ -18,7 +22,9 @@ public function removeByID($itemID) { parent::removeByID($itemID); - if ($this->getJoinTable() === 'Group_Members') { + $memberGroupJoinTable = Member::singleton()->Groups()->getJoinTable(); + + if ($this->getJoinTable() === $memberGroupJoinTable) { $currentMember = Security::getCurrentUser(); if (!$currentMember || !$currentMember->exists()) { return; diff --git a/tests/AuditHookMFATest.yml b/tests/AuditHookMFATest.yml index b96d90e..e3c9506 100644 --- a/tests/AuditHookMFATest.yml +++ b/tests/AuditHookMFATest.yml @@ -1,5 +1,17 @@ +SilverStripe\Security\Group: + # One test could have ambiguous results if the member and the group have the same ID. + # So we're creating a bunch of dummy groups to make sure the IDs are different. + noop: + Title: Noop + foobar: + Title: Foobar + prisoners: + Title: Prisoners + Code: prisoners + SilverStripe\Security\Member: leslie_lawless: FirstName: Leslie Surname: Lawless Email: leslie@example.com + Groups: '=>SilverStripe\Security\Group.prisoners' diff --git a/tests/AuditHookManyManyListTest.php b/tests/AuditHookManyManyListTest.php new file mode 100644 index 0000000..df0de37 --- /dev/null +++ b/tests/AuditHookManyManyListTest.php @@ -0,0 +1,62 @@ +writer = new Logger; + + // Phase singleton out, so the message log is purged. + Injector::inst()->unregisterNamedObject('AuditLogger'); + Injector::inst()->registerService($this->writer, 'AuditLogger'); + + $this->member = $this->objFromFixture(Member::class, 'leslie_lawless'); + $this->group = $this->objFromFixture(Group::class, 'prisoners'); + } + + public function testRemoveByID(): void + { + $this->assertEquals( + 1, + $this->group->Members()->filter('ID', $this->member->ID)->count(), + 'Leslie Lawless is part of the prisoners group' + ); + + $this->group->Members()->removeByID($this->member->ID); + + $message = $this->writer->getLastMessage(); + $currentUser = Security::getCurrentUser(); + $this->assertStringContainsString( + sprintf( + '"%s" (ID: %d) removed Member "%s" (ID: %d) from Group "%s" (ID: %d)', + $currentUser->Email, + $currentUser->ID, + $this->member->Email, + $this->member->ID, + $this->group->Title, + $this->group->ID, + ), + $message, + 'Log contains who removed Leslie Lawless from the prisoners group' + ); + } +} diff --git a/tests/AuditHookMemberGroupSetTest.php b/tests/AuditHookMemberGroupSetTest.php new file mode 100644 index 0000000..2edf598 --- /dev/null +++ b/tests/AuditHookMemberGroupSetTest.php @@ -0,0 +1,62 @@ +writer = new Logger; + + // Phase singleton out, so the message log is purged. + Injector::inst()->unregisterNamedObject('AuditLogger'); + Injector::inst()->registerService($this->writer, 'AuditLogger'); + + $this->member = $this->objFromFixture(Member::class, 'leslie_lawless'); + $this->group = $this->objFromFixture(Group::class, 'prisoners'); + } + + public function testRemoveByID(): void + { + $this->assertEquals( + 1, + $this->member->Groups()->filter('ID', $this->group->ID)->count(), + 'The Prisoners group is one of Leslie Lawless\' groups.' + ); + + $this->member->Groups()->removeByID($this->group->ID); + + $message = $this->writer->getLastMessage(); + $currentUser = Security::getCurrentUser(); + $this->assertStringContainsString( + sprintf( + '"%s" (ID: %d) removed Member "%s" (ID: %d) from Group "%s" (ID: %d)', + $currentUser->Email, + $currentUser->ID, + $this->member->Email, + $this->member->ID, + $this->group->Title, + $this->group->ID, + ), + $message, + 'Log contains who removed "Prisoners" from Leslie Lawless\' groups.' + ); + } +}