Make InheritedPermissions use cache and implement cache flushing

_config/cache.yml

@@ -22,3 +22,7 @@ SilverStripe\Core\Injector\Injector:
       factory: SilverStripe\Core\Cache\CacheFactory
       constructor:
         namespace: 'ratelimiter'
+  Psr\SimpleCache\CacheInterface.InheritedPermissions:
+    factory: SilverStripe\Core\Cache\CacheFactory
+    constructor:
+      namespace: "InheritedPermissions"

_config/security.yml

@@ -38,3 +38,7 @@ SilverStripe\Core\Injector\Injector:
       Authenticators:
         cms: '%$SilverStripe\Security\MemberAuthenticator\CMSMemberAuthenticator'
   SilverStripe\Security\IdentityStore: '%$SilverStripe\Security\AuthenticationHandler'
+  SilverStripe\Security\InheritedPermissionFlusher:
+    properties:
+      Services:
+        - '%$SilverStripe\Security\PermissionChecker.sitetree'

src/Security/InheritedPermissions.php

@@ -8,7 +8,8 @@ use SilverStripe\ORM\DataList;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Hierarchy\Hierarchy;
 use SilverStripe\Versioned\Versioned;
-
+use Psr\SimpleCache\CacheInterface;
+use SilverStripe\Core\Cache\CacheFlusher;
 /**
  * Calculates batch permissions for nested objects for:
  *  - canView: Supports 'Anyone' type
@@ -16,7 +17,7 @@ use SilverStripe\Versioned\Versioned;
  *  - canDelete: Includes special logic for ensuring parent objects can only be deleted if their children can
  *    be deleted also.
  */
-class InheritedPermissions implements PermissionChecker
+class InheritedPermissions implements PermissionChecker, CacheFlusher
 {
     use Injectable;
 
@@ -84,20 +85,69 @@ class InheritedPermissions implements PermissionChecker
      */
     protected $cachePermissions = [];
 
+    /**
+     * @var CacheInterface
+     */
+    protected $cacheService;
+
     /**
      * Construct new permissions object
      *
      * @param string $baseClass Base class
+     * @param CacheInterface $cache
      */
-    public function __construct($baseClass)
+    public function __construct($baseClass, CacheInterface $cache = null)
     {
         if (!is_a($baseClass, DataObject::class, true)) {
             throw new InvalidArgumentException('Invalid DataObject class: ' . $baseClass);
         }
+
         $this->baseClass = $baseClass;
+        $this->cacheService = $cache;
+
         return $this;
     }
 
+    /**
+     * Commits the cache
+     */
+    public function __destruct()
+    {
+        // Ensure back-end cache is updated
+        if (!empty($this->cachePermissions) && $this->cacheService) {
+            foreach ($this->cachePermissions as $key => $permissions) {
+                $this->cacheService->set($key, $permissions);
+            }
+            // Prevent double-destruct
+            $this->cachePermissions = [];
+        }
+    }
+
+    /**
+     * Clear the cache for this instance only
+     * @param array $ids A list of member IDs
+     */
+    public function flushCache($ids = null)
+    {
+        if (!$this->cacheService) {
+            return;
+        }
+
+        // Hard flush, e.g. flush=1
+        if (!$ids) {
+            $this->cacheService->clear();
+        }
+
+        if ($ids && is_array($ids)) {
+            foreach ([self::VIEW, self::EDIT, self::DELETE] as $type) {
+                foreach ($ids as $memberID) {
+                    $key = $this->generateCacheKey($type, $memberID);
+                    $this->cacheService->delete($key);
+                }
+            }
+        }
+    }
+
     /**
      * @param DefaultPermissionChecker $callback
      * @return $this
@@ -212,12 +262,13 @@ class InheritedPermissions implements PermissionChecker
         }
 
         // Look in the cache for values
-        $cacheKey = "{$type}-{$memberID}";
+        $cacheKey = $this->generateCacheKey($type, $memberID);
-        if ($useCached && isset($this->cachePermissions[$cacheKey])) {
+        $cachePermissions = $this->getCachePermissions($cacheKey);
-            $cachedValues = array_intersect_key($this->cachePermissions[$cacheKey], $result);
+        if ($useCached && $cachePermissions) {
+            $cachedValues = array_intersect_key($cachePermissions, $result);
 
             // If we can't find everything in the cache, then look up the remainder separately
-            $uncachedIDs = array_keys(array_diff_key($result, $this->cachePermissions[$cacheKey]));
+            $uncachedIDs = array_keys(array_diff_key($result, $cachePermissions));
             if ($uncachedIDs) {
                 $uncachedValues = $this->batchPermissionCheck($type, $uncachedIDs, $member, $globalPermission, false);
                 return $cachedValues + $uncachedValues;
@@ -241,7 +292,7 @@ class InheritedPermissions implements PermissionChecker
         if ($this->isVersioned()) {
             // Check all records for each stage and merge
             $combinedStageResult = [];
-            foreach ([ Versioned::DRAFT, Versioned::LIVE ] as $stage) {
+            foreach ([Versioned::DRAFT, Versioned::LIVE] as $stage) {
                 $stageRecords = Versioned::get_by_stage($this->getBaseClass(), $stage)
                     ->byIDs($ids);
                 // Exclude previously calculated records from later stage calculations
@@ -277,6 +328,7 @@ class InheritedPermissions implements PermissionChecker
         if ($combinedStageResult) {
             $this->cachePermissions[$cacheKey] = $combinedStageResult + $this->cachePermissions[$cacheKey];
         }
+
         return $combinedStageResult;
     }
 
@@ -367,6 +419,12 @@ class InheritedPermissions implements PermissionChecker
         return $result;
     }
 
+    /**
+     * @param array $ids
+     * @param Member|null $member
+     * @param bool $useCached
+     * @return array
+     */
     public function canEditMultiple($ids, Member $member = null, $useCached = true)
     {
         return $this->batchPermissionCheck(
@@ -378,11 +436,23 @@ class InheritedPermissions implements PermissionChecker
         );
     }
 
+    /**
+     * @param array $ids
+     * @param Member|null $member
+     * @param bool $useCached
+     * @return array
+     */
     public function canViewMultiple($ids, Member $member = null, $useCached = true)
     {
         return $this->batchPermissionCheck(self::VIEW, $ids, $member, [], $useCached);
     }
 
+    /**
+     * @param array $ids
+     * @param Member|null $member
+     * @param bool $useCached
+     * @return array
+     */
     public function canDeleteMultiple($ids, Member $member = null, $useCached = true)
     {
         // Validate ids
@@ -400,11 +470,12 @@ class InheritedPermissions implements PermissionChecker
 
         // Look in the cache for values
         $cacheKey = "delete-{$member->ID}";
-        if ($useCached && isset($this->cachePermissions[$cacheKey])) {
+        $cachePermissions = $this->getCachePermissions($cacheKey);
-            $cachedValues = array_intersect_key($this->cachePermissions[$cacheKey], $result);
+        if ($useCached && $cachePermissions) {
+            $cachedValues = array_intersect_key($cachePermissions[$cacheKey], $result);
 
             // If we can't find everything in the cache, then look up the remainder separately
-            $uncachedIDs = array_keys(array_diff_key($result, $this->cachePermissions[$cacheKey]));
+            $uncachedIDs = array_keys(array_diff_key($result, $cachePermissions[$cacheKey]));
             if ($uncachedIDs) {
                 $uncachedValues = $this->canDeleteMultiple($uncachedIDs, $member, false);
                 return $cachedValues + $uncachedValues;
@@ -451,6 +522,11 @@ class InheritedPermissions implements PermissionChecker
         return array_fill_keys($deletable, true) + array_fill_keys($ids, false);
     }
 
+    /**
+     * @param int $id
+     * @param Member|null $member
+     * @return bool|mixed
+     */
     public function canDelete($id, Member $member = null)
     {
         // No ID: Check default permission
@@ -460,7 +536,7 @@ class InheritedPermissions implements PermissionChecker
 
         // Regular canEdit logic is handled by canEditMultiple
         $results = $this->canDeleteMultiple(
-            [ $id ],
+            [$id],
             $member
         );
 
@@ -468,6 +544,11 @@ class InheritedPermissions implements PermissionChecker
         return isset($results[$id]) ? $results[$id] : false;
     }
 
+    /**
+     * @param int $id
+     * @param Member|null $member
+     * @return bool|mixed
+     */
     public function canEdit($id, Member $member = null)
     {
         // No ID: Check default permission
@@ -477,7 +558,7 @@ class InheritedPermissions implements PermissionChecker
 
         // Regular canEdit logic is handled by canEditMultiple
         $results = $this->canEditMultiple(
-            [ $id ],
+            [$id],
             $member
         );
 
@@ -485,6 +566,11 @@ class InheritedPermissions implements PermissionChecker
         return isset($results[$id]) ? $results[$id] : false;
     }
 
+    /**
+     * @param int $id
+     * @param Member|null $member
+     * @return bool|mixed
+     */
     public function canView($id, Member $member = null)
     {
         // No ID: Check default permission
@@ -494,7 +580,7 @@ class InheritedPermissions implements PermissionChecker
 
         // Regular canView logic is handled by canViewMultiple
         $results = $this->canViewMultiple(
-            [ $id ],
+            [$id],
             $member
         );
 
@@ -583,6 +669,9 @@ class InheritedPermissions implements PermissionChecker
         return $singleton->hasExtension(Versioned::class);
     }
 
+    /**
+     * @return $this
+     */
     public function clearCache()
     {
         $this->cachePermissions = [];
@@ -610,4 +699,34 @@ class InheritedPermissions implements PermissionChecker
         $table = DataObject::getSchema()->tableName($this->baseClass);
         return "{$table}_ViewerGroups";
     }
-}
+
+    /**
+     * Gets the permission from cache
+     *
+     * @param $cacheKey
+     * @return mixed
+     */
+    protected function getCachePermissions($cacheKey)
+    {
+        if (isset($this->cachePermissions[$cacheKey])) {
+            return $this->cachePermissions[$cacheKey];
+        }
+
+        if ($this->cacheService) {
+            return $this->cacheService->get($cacheKey);
+        }
+
+        return null;
+    }
+
+    /**
+     * Creates a cache key for a member and type
+     * @param $type
+     * @param $memberID
+     * @return string
+     */
+    protected function generateCacheKey($type, $memberID)
+    {
+        return "{$type}-{$memberID}";
+    }
+}

tests/php/Security/InheritedPermissionsTest.php

@@ -11,6 +11,8 @@ use SilverStripe\Security\PermissionChecker;
 use SilverStripe\Security\Test\InheritedPermissionsTest\TestPermissionNode;
 use SilverStripe\Security\Test\InheritedPermissionsTest\TestDefaultPermissionChecker;
 use SilverStripe\Versioned\Versioned;
+use Psr\SimpleCache\CacheInterface;
+use ReflectionClass;
 
 class InheritedPermissionsTest extends SapphireTest
 {
@@ -266,4 +268,114 @@ class InheritedPermissionsTest extends SapphireTest
             $this->assertFalse($history->canView());
         });
     }
+
+    public function testPermissionsPersistCache()
+    {
+        /* @var CacheInterface $cache */
+        $cache = Injector::inst()->create(CacheInterface::class . '.InheritedPermissions');
+        $cache->clear();
+
+        $member = $this->objFromFixture(Member::class, 'editor');
+
+        /** @var TestPermissionNode $history */
+        $history = $this->objFromFixture(TestPermissionNode::class, 'history');
+        /** @var TestPermissionNode $historyGallery */
+        $historyGallery = $this->objFromFixture(TestPermissionNode::class, 'history-gallery');
+        $permissionChecker = new InheritedPermissions(TestPermissionNode::class, $cache);
+
+        $viewKey = $this->generateCacheKey($permissionChecker, InheritedPermissions::VIEW, $member->ID);
+        $editKey = $this->generateCacheKey($permissionChecker, InheritedPermissions::EDIT, $member->ID);
+
+        $this->assertNull($cache->get($editKey));
+        $this->assertNull($cache->get($viewKey));
+
+        $permissionChecker->canEditMultiple([$history->ID, $historyGallery->ID], $member);
+        $this->assertNull($cache->get($editKey));
+        $this->assertNull($cache->get($viewKey));
+
+        unset($permissionChecker);
+        $this->assertTrue(is_array($cache->get($editKey)));
+        $this->assertNull($cache->get($viewKey));
+        $this->assertArrayHasKey($history->ID, $cache->get($editKey));
+        $this->assertArrayHasKey($historyGallery->ID, $cache->get($editKey));
+
+        $permissionChecker = new InheritedPermissions(TestPermissionNode::class, $cache);
+        $permissionChecker->canViewMultiple([$history->ID], $member);
+        $this->assertNotNull($cache->get($editKey));
+        $this->assertNull($cache->get($viewKey));
+
+        unset($permissionChecker);
+        $this->assertTrue(is_array($cache->get($viewKey)));
+        $this->assertTrue(is_array($cache->get($editKey)));
+        $this->assertArrayHasKey($history->ID, $cache->get($viewKey));
+        $this->assertArrayNotHasKey($historyGallery->ID, $cache->get($viewKey));
+    }
+
+    public function testPermissionsFlushCache()
+    {
+        /* @var CacheInterface $cache */
+        $cache = Injector::inst()->create(CacheInterface::class . '.InheritedPermissions');
+        $cache->clear();
+        
+        $permissionChecker = new InheritedPermissions(TestPermissionNode::class, $cache);
+        $member1 = $this->objFromFixture(Member::class, 'editor');
+        $member2 = $this->objFromFixture(Member::class, 'admin');
+        $editKey1 = $this->generateCacheKey($permissionChecker, InheritedPermissions::EDIT, $member1->ID);
+        $editKey2 = $this->generateCacheKey($permissionChecker, InheritedPermissions::EDIT, $member2->ID);
+        $viewKey1 = $this->generateCacheKey($permissionChecker, InheritedPermissions::VIEW, $member1->ID);
+        $viewKey2 = $this->generateCacheKey($permissionChecker, InheritedPermissions::VIEW, $member2->ID);
+
+        foreach([$editKey1, $editKey2, $viewKey1, $viewKey2] as $key) {
+            $this->assertNull($cache->get($key));
+        }
+
+        /** @var TestPermissionNode $history */
+        $history = $this->objFromFixture(TestPermissionNode::class, 'history');
+        /** @var TestPermissionNode $historyGallery */
+        $historyGallery = $this->objFromFixture(TestPermissionNode::class, 'history-gallery');
+
+        $permissionChecker->canEditMultiple([$history->ID, $historyGallery->ID], $member1);
+        $permissionChecker->canViewMultiple([$history->ID, $historyGallery->ID], $member1);
+        $permissionChecker->canEditMultiple([$history->ID, $historyGallery->ID], $member2);
+        $permissionChecker->canViewMultiple([$history->ID, $historyGallery->ID], $member2);
+
+        unset($permissionChecker);
+
+        foreach([$editKey1, $editKey2, $viewKey1, $viewKey2] as $key) {
+            $this->assertNotNull($cache->get($key));
+        }
+        $permissionChecker = new InheritedPermissions(TestPermissionNode::class, $cache);
+
+        // Non existent ID
+        $permissionChecker->flushCache('dummy');
+        foreach([$editKey1, $editKey2, $viewKey1, $viewKey2] as $key) {
+            $this->assertNotNull($cache->get($key));
+        }
+
+        // Precision strike
+        $permissionChecker->flushCache([$member1->ID]);
+        // Member1 should be clear
+        $this->assertNull($cache->get($editKey1));
+        $this->assertNull($cache->get($viewKey1));
+        // Member 2 is unaffected
+        $this->assertNotNull($cache->get($editKey2));
+        $this->assertNotNull($cache->get($viewKey2));
+
+        // Nuclear
+        $permissionChecker->flushCache();
+        foreach([$editKey1, $editKey2, $viewKey1, $viewKey2] as $key) {
+            $this->assertNull($cache->get($key));
+        }
+
+    }
+
+    protected function generateCacheKey(InheritedPermissions $inst, $type, $memberID)
+    {
+        $reflection = new ReflectionClass(InheritedPermissions::class);
+        $method = $reflection->getMethod('generateCacheKey');
+        $method->setAccessible(true);
+
+        return $method->invokeArgs($inst, [$type, $memberID]);
+    }
+
 }