From eecb9f64d3ece2554ca6f7f68429a50b0de53eb0 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Mon, 4 Dec 2017 13:12:13 +1300 Subject: [PATCH 1/5] Add new InheritedPermissionFlusher extension, CacheFlusher service --- _config/extensions.yml | 9 ++ src/Core/Cache/CacheFlusher.php | 16 +++ src/Security/Group.php | 11 +- src/Security/InheritedPermissionFlusher.php | 104 ++++++++++++++++++ .../InheritedPermissionsFlusherTest.php | 100 +++++++++++++++++ .../InheritedPermissionsFlusherTest.yml | 35 ++++++ .../TestCacheFlusher.php | 67 +++++++++++ 7 files changed, 340 insertions(+), 2 deletions(-) create mode 100644 _config/extensions.yml create mode 100644 src/Core/Cache/CacheFlusher.php create mode 100644 src/Security/InheritedPermissionFlusher.php create mode 100644 tests/php/Security/InheritedPermissionsFlusherTest.php create mode 100644 tests/php/Security/InheritedPermissionsFlusherTest.yml create mode 100644 tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php diff --git a/_config/extensions.yml b/_config/extensions.yml new file mode 100644 index 000000000..26f8ed4f2 --- /dev/null +++ b/_config/extensions.yml @@ -0,0 +1,9 @@ +--- +Name: coreextensions +--- +SilverStripe\Security\Member: + extensions: + - SilverStripe\Security\InheritedPermissionFlusher +SilverStripe\Security\Group: + extensions: + - SilverStripe\Security\InheritedPermissionFlusher \ No newline at end of file diff --git a/src/Core/Cache/CacheFlusher.php b/src/Core/Cache/CacheFlusher.php new file mode 100644 index 000000000..6d5788415 --- /dev/null +++ b/src/Core/Cache/CacheFlusher.php @@ -0,0 +1,16 @@ +removeFilterOn('Group_Members'); }); } + // Now set all children groups as a new foreign key - $groups = Group::get()->byIDs($this->collateFamilyIDs()); - $result = $result->forForeignID($groups->column('ID'))->where($filter); + $familyIDs = $this->collateFamilyIDs(); + if (!empty($familyIDs)) { + $groups = Group::get()->byIDs($familyIDs); + $groupIDs = $groups->column('ID'); + if (!empty($groupIDs)) { + $result = $result->forForeignID($groupIDs)->where($filter); + } + } return $result; } diff --git a/src/Security/InheritedPermissionFlusher.php b/src/Security/InheritedPermissionFlusher.php new file mode 100644 index 000000000..fd5c257d1 --- /dev/null +++ b/src/Security/InheritedPermissionFlusher.php @@ -0,0 +1,104 @@ +flushCache(); + } + + /** + * @param DataObject $owner + */ + public function setOwner($owner) + { + if (!$owner instanceof Member && !$owner instanceof Group) { + throw new InvalidArgumentException(sprintf( + '%s can only be applied to %s or %s', + __CLASS__, + Member::class, + Group::class + )); + } + + parent::setOwner($owner); + } + + /** + * @param CacheFlusher[] + */ + public function setServices($services) + { + foreach ($services as $service) { + if (!$service instanceof CacheFlusher) { + throw new InvalidArgumentException(sprintf( + '%s.services must contain only %s instances. %s provided.', + __CLASS__, + CacheFlusher::class, + get_class($service) + )); + } + } + + $this->services = $services; + } + + /** + * @return CacheFlusher[] + */ + public function getServices() + { + return $this->services; + } + + /** + * Flushes all registered CacheFlusher services + */ + public function flushCache() + { + $ids = $this->getMemberIDList(); + foreach ($this->services as $service) { + $service->flushCache($ids); + } + } + + /** + * Get a list of member IDs that need their permissions flushed + * + * @return array|null + */ + protected function getMemberIDList() + { + if (!$this->owner) { + return null; + } + + if (!$this->owner->exists()) { + return null; + } + + if ($this->owner instanceof Group) { + return $this->owner->Members()->exists() + ? $this->owner->Members()->column('ID') + : null; + } + + return [$this->owner->ID]; + } +} \ No newline at end of file diff --git a/tests/php/Security/InheritedPermissionsFlusherTest.php b/tests/php/Security/InheritedPermissionsFlusherTest.php new file mode 100644 index 000000000..e940405b3 --- /dev/null +++ b/tests/php/Security/InheritedPermissionsFlusherTest.php @@ -0,0 +1,100 @@ +load([ + CacheInterface::class . '.TestFlusherCache' => [ + 'factory' => CacheFactory::class, + 'constructor' => ['namespace' => 'TestFlusherCache'] + ] + ]); + } + + public function testMemberFlushesPermissions() + { + $cache = Injector::inst()->create(CacheInterface::class . '.TestFlusherCache'); + $flusher = new TestCacheFlusher($cache); + $extension = new InheritedPermissionFlusher(); + $extension->setServices([$flusher]); + Injector::inst()->registerService($extension, InheritedPermissionFlusher::class); + $editor = $this->objFromFixture(Member::class, 'editor'); + $admin = $this->objFromFixture(Member::class, 'admin'); + $editorKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $editor->ID); + $adminKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $admin->ID); + $cache->set($editorKey, 'uncle'); + $cache->set($adminKey, 'cheese'); + $editor->flushCache(); + + $this->assertNull($cache->get($editorKey)); + $this->assertEquals('cheese', $cache->get($adminKey)); + + $admin->flushCache(); + $this->assertNull($cache->get($editorKey)); + $this->assertNull($cache->get($adminKey)); + } + + public function testGroupFlushesPermissions() + { + $cache = Injector::inst()->create(CacheInterface::class . '.TestFlusherCache'); + $flusher = new TestCacheFlusher($cache); + $extension = new InheritedPermissionFlusher(); + $extension->setServices([$flusher]); + Injector::inst()->registerService($extension, InheritedPermissionFlusher::class); + $editors = $this->objFromFixture(Group::class, 'editors'); + $admins = $this->objFromFixture(Group::class, 'admins'); + + // Populate the cache for all members in each group + foreach ($editors->Members() as $editor) { + $editorKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $editor->ID); + $cache->set($editorKey, 'uncle'); + } + foreach ($admins->Members() as $admin) { + $adminKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $admin->ID); + $cache->set($adminKey, 'cheese'); + } + + // Clear the cache for all members in the editors group + $editors->flushCache(); + + foreach ($editors->Members() as $editor) { + $editorKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $editor->ID); + $this->assertNull($cache->get($editorKey)); + } + // Admins group should be unaffected + foreach ($admins->Members() as $admin) { + $adminKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $admin->ID); + $this->assertEquals('cheese', $cache->get($adminKey)); + } + + + $admins->flushCache(); + // Admins now affected + foreach ($admins->Members() as $admin) { + $adminKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $admin->ID); + $this->assertNull($cache->get($adminKey)); + } + foreach ($editors->Members() as $editor) { + $editorKey = $flusher->generateCacheKey(TestCacheFlusher::$categories[0], $editor->ID); + $this->assertNull($cache->get($editorKey)); + } + } +} \ No newline at end of file diff --git a/tests/php/Security/InheritedPermissionsFlusherTest.yml b/tests/php/Security/InheritedPermissionsFlusherTest.yml new file mode 100644 index 000000000..063d90aa6 --- /dev/null +++ b/tests/php/Security/InheritedPermissionsFlusherTest.yml @@ -0,0 +1,35 @@ +SilverStripe\Security\Group: + editors: + Title: Editors + admins: + Title: Administrators + allsections: + Title: All Section Editors + securityadmins: + Title: Security Admins + +SilverStripe\Security\Permission: + admins: + Code: ADMIN + Group: =>SilverStripe\Security\Group.admins + editors: + Code: CMS_ACCESS_CMSMain + Group: =>SilverStripe\Security\Group.editors + testpermission: + Code: TEST_NODE_ACCESS + Group: =>SilverStripe\Security\Group.editors + + +SilverStripe\Security\Member: + editor: + FirstName: Test + Surname: Editor + Groups: =>SilverStripe\Security\Group.editors + admin: + FirstName: Test + Surname: Administrator + Groups: =>SilverStripe\Security\Group.admins + allsections: + Groups: =>SilverStripe\Security\Group.allsections + securityadmin: + Groups: =>SilverStripe\Security\Group.securityadmins \ No newline at end of file diff --git a/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php b/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php new file mode 100644 index 000000000..b6a3c17f6 --- /dev/null +++ b/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php @@ -0,0 +1,67 @@ +cache = $cache; + } + + /** + * Clear the cache for this instance only + * @param array $ids A list of member IDs + */ + public function flushCache($ids = null) + { + if (!$this->cache) { + return; + } + + // Hard flush, e.g. flush=1 + if (!$ids) { + $this->cache->clear(); + } + + if ($ids && is_array($ids)) { + foreach (self::$categories as $category) { + foreach ($ids as $memberID) { + $key = $this->generateCacheKey($category, $memberID); + $this->cache->delete($key); + } + } + } + } + + /** + * @param $category + * @param $memberID + * @return string + */ + public function generateCacheKey($category, $memberID) + { + return "{$category}__{$memberID}"; + } +} \ No newline at end of file From 4857816c9e319e3d261afa013990dec4b3cd8618 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Wed, 6 Dec 2017 22:03:05 +1300 Subject: [PATCH 2/5] Revisions per robbieaverill --- src/Security/InheritedPermissionFlusher.php | 10 +++++----- tests/php/Security/InheritedPermissionsFlusherTest.php | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Security/InheritedPermissionFlusher.php b/src/Security/InheritedPermissionFlusher.php index fd5c257d1..bd784fa3b 100644 --- a/src/Security/InheritedPermissionFlusher.php +++ b/src/Security/InheritedPermissionFlusher.php @@ -42,6 +42,8 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable /** * @param CacheFlusher[] + * @throws InvalidArgumentException + * @return $this */ public function setServices($services) { @@ -57,6 +59,8 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable } $this->services = $services; + + return $this; } /** @@ -85,11 +89,7 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable */ protected function getMemberIDList() { - if (!$this->owner) { - return null; - } - - if (!$this->owner->exists()) { + if (!$this->owner || !$this->owner->exists()) { return null; } diff --git a/tests/php/Security/InheritedPermissionsFlusherTest.php b/tests/php/Security/InheritedPermissionsFlusherTest.php index e940405b3..716234b72 100644 --- a/tests/php/Security/InheritedPermissionsFlusherTest.php +++ b/tests/php/Security/InheritedPermissionsFlusherTest.php @@ -16,7 +16,7 @@ class InheritedPermissionsFlusherTest extends SapphireTest { protected static $fixture_file = 'InheritedPermissionsFlusherTest.yml'; - public function setUp() + protected function setUp() { parent::setUp(); From 86458941be8fe7658ab72ac13c0089865a6ebb11 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Thu, 7 Dec 2017 16:07:40 +1300 Subject: [PATCH 3/5] Refactor to MemberCacheFlusher --- .../{CacheFlusher.php => MemberCacheFlusher.php} | 6 +++--- src/Security/InheritedPermissionFlusher.php | 8 ++++---- .../TestCacheFlusher.php | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) rename src/Core/Cache/{CacheFlusher.php => MemberCacheFlusher.php} (62%) diff --git a/src/Core/Cache/CacheFlusher.php b/src/Core/Cache/MemberCacheFlusher.php similarity index 62% rename from src/Core/Cache/CacheFlusher.php rename to src/Core/Cache/MemberCacheFlusher.php index 6d5788415..63f872b4e 100644 --- a/src/Core/Cache/CacheFlusher.php +++ b/src/Core/Cache/MemberCacheFlusher.php @@ -6,11 +6,11 @@ namespace SilverStripe\Core\Cache; * Defines a service that can flush its cache for a list of members * @package SilverStripe\Core\Cache */ -interface CacheFlusher +interface MemberCacheFlusher { /** - * @param null $ids + * @param null $memberIDs * @return mixed */ - public function flushCache($ids = null); + public function flushMemberCache($memberIDs = null); } \ No newline at end of file diff --git a/src/Security/InheritedPermissionFlusher.php b/src/Security/InheritedPermissionFlusher.php index bd784fa3b..440d88a7f 100644 --- a/src/Security/InheritedPermissionFlusher.php +++ b/src/Security/InheritedPermissionFlusher.php @@ -6,7 +6,7 @@ use Psr\Log\InvalidArgumentException; use SilverStripe\Core\Flushable; use SilverStripe\ORM\DataExtension; use SilverStripe\ORM\DataObject; -use SilverStripe\Core\Cache\CacheFlusher; +use SilverStripe\Core\Cache\MemberCacheFlusher; class InheritedPermissionFlusher extends DataExtension implements Flushable { @@ -48,11 +48,11 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable public function setServices($services) { foreach ($services as $service) { - if (!$service instanceof CacheFlusher) { + if (!$service instanceof MemberCacheFlusher) { throw new InvalidArgumentException(sprintf( '%s.services must contain only %s instances. %s provided.', __CLASS__, - CacheFlusher::class, + MemberCacheFlusher::class, get_class($service) )); } @@ -78,7 +78,7 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable { $ids = $this->getMemberIDList(); foreach ($this->services as $service) { - $service->flushCache($ids); + $service->flushMemberCache($ids); } } diff --git a/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php b/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php index b6a3c17f6..b3e5972ab 100644 --- a/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php +++ b/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php @@ -3,9 +3,9 @@ namespace SilverStripe\Security\Tests\InheritedPermissionsFlusherTest; use Psr\SimpleCache\CacheInterface; -use SilverStripe\Core\Cache\CacheFlusher; +use SilverStripe\Core\Cache\MemberCacheFlusher; -class TestCacheFlusher implements CacheFlusher +class TestCacheFlusher implements MemberCacheFlusher { /** * @var array @@ -32,22 +32,22 @@ class TestCacheFlusher implements CacheFlusher /** * Clear the cache for this instance only - * @param array $ids A list of member IDs + * @param array $memberIDs A list of member IDs */ - public function flushCache($ids = null) + public function flushMemberCache($memberIDs = null) { if (!$this->cache) { return; } // Hard flush, e.g. flush=1 - if (!$ids) { + if (!$memberIDs) { $this->cache->clear(); } - if ($ids && is_array($ids)) { + if ($memberIDs && is_array($memberIDs)) { foreach (self::$categories as $category) { - foreach ($ids as $memberID) { + foreach ($memberIDs as $memberID) { $key = $this->generateCacheKey($category, $memberID); $this->cache->delete($key); } From 8b429bf47b298cc31c41cab869b7a7fc4c2e510a Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Thu, 7 Dec 2017 16:12:05 +1300 Subject: [PATCH 4/5] update docblock --- src/Security/InheritedPermissionFlusher.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Security/InheritedPermissionFlusher.php b/src/Security/InheritedPermissionFlusher.php index 440d88a7f..0d33a9d5e 100644 --- a/src/Security/InheritedPermissionFlusher.php +++ b/src/Security/InheritedPermissionFlusher.php @@ -11,7 +11,7 @@ use SilverStripe\Core\Cache\MemberCacheFlusher; class InheritedPermissionFlusher extends DataExtension implements Flushable { /** - * @var CacheFlusher[] + * @var MemberCacheFlusher[] */ protected $services = []; @@ -41,7 +41,7 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable } /** - * @param CacheFlusher[] + * @param MemberCacheFlusher[] * @throws InvalidArgumentException * @return $this */ @@ -64,7 +64,7 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable } /** - * @return CacheFlusher[] + * @return MemberCacheFlusher[] */ public function getServices() { @@ -72,7 +72,7 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable } /** - * Flushes all registered CacheFlusher services + * Flushes all registered MemberCacheFlusher services */ public function flushCache() { From ee27329728304391c6cd684df3c925ff3db79287 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 11 Dec 2017 16:46:21 +1300 Subject: [PATCH 5/5] Minor linting / style updates --- src/Core/Cache/MemberCacheFlusher.php | 6 ++---- src/Security/InheritedPermissionFlusher.php | 4 ++-- tests/php/Security/InheritedPermissionsFlusherTest.php | 9 +++++---- tests/php/Security/InheritedPermissionsFlusherTest.yml | 2 +- .../InheritedPermissionsFlusherTest/TestCacheFlusher.php | 3 ++- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Core/Cache/MemberCacheFlusher.php b/src/Core/Cache/MemberCacheFlusher.php index 63f872b4e..7d0069164 100644 --- a/src/Core/Cache/MemberCacheFlusher.php +++ b/src/Core/Cache/MemberCacheFlusher.php @@ -4,13 +4,11 @@ namespace SilverStripe\Core\Cache; /** * Defines a service that can flush its cache for a list of members - * @package SilverStripe\Core\Cache */ interface MemberCacheFlusher { /** - * @param null $memberIDs - * @return mixed + * @param array $memberIDs */ public function flushMemberCache($memberIDs = null); -} \ No newline at end of file +} diff --git a/src/Security/InheritedPermissionFlusher.php b/src/Security/InheritedPermissionFlusher.php index 0d33a9d5e..19ca9de7e 100644 --- a/src/Security/InheritedPermissionFlusher.php +++ b/src/Security/InheritedPermissionFlusher.php @@ -77,7 +77,7 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable public function flushCache() { $ids = $this->getMemberIDList(); - foreach ($this->services as $service) { + foreach ($this->getServices() as $service) { $service->flushMemberCache($ids); } } @@ -101,4 +101,4 @@ class InheritedPermissionFlusher extends DataExtension implements Flushable return [$this->owner->ID]; } -} \ No newline at end of file +} diff --git a/tests/php/Security/InheritedPermissionsFlusherTest.php b/tests/php/Security/InheritedPermissionsFlusherTest.php index 716234b72..8cadcd0cc 100644 --- a/tests/php/Security/InheritedPermissionsFlusherTest.php +++ b/tests/php/Security/InheritedPermissionsFlusherTest.php @@ -4,13 +4,12 @@ namespace SilverStripe\Security\Tests; use Psr\SimpleCache\CacheInterface; use SilverStripe\Core\Cache\CacheFactory; +use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Security\Group; use SilverStripe\Security\InheritedPermissionFlusher; use SilverStripe\Security\Member; -use SilverStripe\Security\Group; -use SilverStripe\Core\Injector\Injector; use SilverStripe\Security\Tests\InheritedPermissionsFlusherTest\TestCacheFlusher; -use SilverStripe\Core\Config\Config; class InheritedPermissionsFlusherTest extends SapphireTest { @@ -59,7 +58,9 @@ class InheritedPermissionsFlusherTest extends SapphireTest $extension = new InheritedPermissionFlusher(); $extension->setServices([$flusher]); Injector::inst()->registerService($extension, InheritedPermissionFlusher::class); + /** @var Group $editors */ $editors = $this->objFromFixture(Group::class, 'editors'); + /** @var Group $admins */ $admins = $this->objFromFixture(Group::class, 'admins'); // Populate the cache for all members in each group @@ -97,4 +98,4 @@ class InheritedPermissionsFlusherTest extends SapphireTest $this->assertNull($cache->get($editorKey)); } } -} \ No newline at end of file +} diff --git a/tests/php/Security/InheritedPermissionsFlusherTest.yml b/tests/php/Security/InheritedPermissionsFlusherTest.yml index 063d90aa6..495c90690 100644 --- a/tests/php/Security/InheritedPermissionsFlusherTest.yml +++ b/tests/php/Security/InheritedPermissionsFlusherTest.yml @@ -32,4 +32,4 @@ SilverStripe\Security\Member: allsections: Groups: =>SilverStripe\Security\Group.allsections securityadmin: - Groups: =>SilverStripe\Security\Group.securityadmins \ No newline at end of file + Groups: =>SilverStripe\Security\Group.securityadmins diff --git a/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php b/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php index b3e5972ab..066ff9ca0 100644 --- a/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php +++ b/tests/php/Security/InheritedPermissionsFlusherTest/TestCacheFlusher.php @@ -32,6 +32,7 @@ class TestCacheFlusher implements MemberCacheFlusher /** * Clear the cache for this instance only + * * @param array $memberIDs A list of member IDs */ public function flushMemberCache($memberIDs = null) @@ -64,4 +65,4 @@ class TestCacheFlusher implements MemberCacheFlusher { return "{$category}__{$memberID}"; } -} \ No newline at end of file +}