DRY permission caching

getkirby · Jan 18, 2025 · 1800a69 · 1800a69
1 parent 5181783
commit 1800a69
Show file tree

Hide file tree

Showing 8 changed files with 159 additions and 24 deletions.
diff --git a/src/Cms/File.php b/src/Cms/File.php
@@ -318,12 +318,7 @@ public function isAccessible(): bool
 			return false;
 		}
 
-		static $accessible   = [];
-		$role                = $this->kirby()->role()?->id() ?? '__none__';
-		$template            = $this->template() ?? '__none__';
-		$accessible[$role] ??= [];
-
-		return $accessible[$role][$template] ??= $this->permissions()->can('access');
+		return FilePermissions::canFromCache($this, 'access');
 	}
 
 	/**
@@ -342,12 +337,7 @@ public function isListable(): bool
 			return false;
 		}
 
-		static $listable   = [];
-		$role              = $this->kirby()->role()?->id() ?? '__none__';
-		$template          = $this->template() ?? '__none__';
-		$listable[$role] ??= [];
-
-		return $listable[$role][$template] ??= $this->permissions()->can('list');
+		return FilePermissions::canFromCache($this, 'list');
 	}
 
 	/**

diff --git a/src/Cms/FilePermissions.php b/src/Cms/FilePermissions.php
@@ -15,6 +15,14 @@ class FilePermissions extends ModelPermissions
 {
 	protected const CATEGORY = 'files';
 
+	/**
+	 * Used to cache once determined permissions in memory
+	 */
+	protected static function cacheKey(ModelWithContent|Language $model): string
+	{
+		return $model->template() ?? '__none__';
+	}
+
 	protected function canChangeTemplate(): bool
 	{
 		if (count($this->model->blueprints()) <= 1) {

diff --git a/src/Cms/ModelPermissions.php b/src/Cms/ModelPermissions.php
@@ -2,6 +2,7 @@
 
 namespace Kirby\Cms;
 
+use Kirby\Exception\LogicException;
 use Kirby\Toolkit\A;
 
 /**
@@ -18,6 +19,8 @@ abstract class ModelPermissions
 	protected const CATEGORY = 'model';
 	protected array $options;
 
+	protected static array $cache = [];
+
 	public function __construct(protected ModelWithContent|Language $model)
 	{
 		$this->options = match (true) {
@@ -40,6 +43,17 @@ public function __debugInfo(): array
 		return $this->toArray();
 	}
 
+	/**
+	 * Can be overridden by specific child classes
+	 * to return a model-specific value used to
+	 * cache a once determined permission in memory
+	 * @codeCoverageIgnore
+	 */
+	protected static function cacheKey(ModelWithContent|Language $model): string
+	{
+		return '';
+	}
+
 	/**
 	 * Returns whether the current user is allowed to do
 	 * a certain action on the model
@@ -105,6 +119,30 @@ public function can(
 		return $permissions->for(static::category($this->model), $action, $default);
 	}
 
+	/**
+	 * Quickly determines a permission for the current user role
+	 * and model blueprint unless dynamic checking is required
+	 */
+	public static function canFromCache(
+		ModelWithContent|Language $model,
+		string $action,
+		bool $default = false
+	): bool {
+		$role     = $model->kirby()->role()?->id() ?? '__none__';
+		$category = static::category($model);
+		$cacheKey = $category . '.' . $action . '/' . static::cacheKey($model) . '/' . $role;
+
+		if (isset(static::$cache[$cacheKey]) === true) {
+			return static::$cache[$cacheKey];
+		}
+
+		if (method_exists(static::class, 'can' . $action) === true) {
+			throw new LogicException('Cannot use permission cache for dynamically-determined permission');
+		}
+
+		return static::$cache[$cacheKey] = $model->permissions()->can($action, $role, $default);
+	}
+
 	/**
 	 * Returns whether the current user is not allowed to do
 	 * a certain action on the model

diff --git a/src/Cms/Page.php b/src/Cms/Page.php
@@ -522,12 +522,7 @@ public function isAccessible(): bool
 			return false;
 		}
 
-		static $accessible   = [];
-		$role                = $this->kirby()->role()?->id() ?? '__none__';
-		$template            = $this->intendedTemplate()->name();
-		$accessible[$role] ??= [];
-
-		return $accessible[$role][$template] ??= $this->permissions()->can('access');
+		return PagePermissions::canFromCache($this, 'access');
 	}
 
 	/**
@@ -694,12 +689,7 @@ public function isListable(): bool
 			return false;
 		}
 
-		static $listable   = [];
-		$role              = $this->kirby()->role()?->id() ?? '__none__';
-		$template          = $this->intendedTemplate()->name();
-		$listable[$role] ??= [];
-
-		return $listable[$role][$template] ??= $this->permissions()->can('list');
+		return PagePermissions::canFromCache($this, 'list');
 	}
 
 	/**

diff --git a/src/Cms/PagePermissions.php b/src/Cms/PagePermissions.php
@@ -15,6 +15,14 @@ class PagePermissions extends ModelPermissions
 {
 	protected const CATEGORY = 'pages';
 
+	/**
+	 * Used to cache once determined permissions in memory
+	 */
+	protected static function cacheKey(ModelWithContent|Language $model): string
+	{
+		return $model->intendedTemplate()->name();
+	}
+
 	protected function canChangeSlug(): bool
 	{
 		return $this->model->isHomeOrErrorPage() !== true;

diff --git a/src/Cms/UserPermissions.php b/src/Cms/UserPermissions.php
@@ -13,6 +13,14 @@
  */
 class UserPermissions extends ModelPermissions
 {
+	/**
+	 * Used to cache once determined permissions in memory
+	 */
+	protected static function cacheKey(ModelWithContent|Language $model): string
+	{
+		return $model->role()->id();
+	}
+
 	protected function canChangeRole(): bool
 	{
 		// protect admin from role changes by non-admin

diff --git a/tests/Cms/Files/FilePermissionsTest.php b/tests/Cms/Files/FilePermissionsTest.php
@@ -2,6 +2,7 @@
 
 namespace Kirby\Cms;
 
+use Kirby\Exception\LogicException;
 use Kirby\TestCase;
 
 /**
@@ -14,6 +15,9 @@ public function setUp(): void
 		$this->app = new App([
 			'roots' => [
 				'index' => '/dev/null'
+			],
+			'users' => [
+				['id' => 'bastian', 'role' => 'admin']
 			]
 		]);
 	}
@@ -60,6 +64,51 @@ public function testWithNobody($action)
 		$this->assertFalse($perms->can($action));
 	}
 
+	/**
+	 * @covers \Kirby\Cms\ModelPermissions::canFromCache
+	 */
+	public function testCanFromCache()
+	{
+		$this->app->impersonate('bastian');
+
+		$page = new Page(['slug' => 'test']);
+		$file = new File([
+			'filename'  => 'test.jpg',
+			'parent'    => $page,
+			'template'  => 'some-template',
+			'blueprint' => [
+				'name' => 'files/some-template',
+				'options' => [
+					'access' => false,
+					'list'   => false
+				]
+			]
+		]);
+
+		$this->assertFalse(FilePermissions::canFromCache($file, 'access'));
+		$this->assertFalse(FilePermissions::canFromCache($file, 'access'));
+		$this->assertFalse(FilePermissions::canFromCache($file, 'list'));
+		$this->assertFalse(FilePermissions::canFromCache($file, 'list'));
+	}
+
+	/**
+	 * @covers \Kirby\Cms\ModelPermissions::canFromCache
+	 */
+	public function testCanFromCacheDynamic()
+	{
+		$this->expectException(LogicException::class);
+		$this->expectExceptionMessage('Cannot use permission cache for dynamically-determined permission');
+
+		$page = new Page(['slug' => 'test']);
+		$file = new File([
+			'filename' => 'test.jpg',
+			'parent'   => $page,
+			'template' => 'some-template',
+		]);
+
+		FilePermissions::canFromCache($file, 'changeTemplate');
+	}
+
 	/**
 	 * @covers ::canChangeTemplate
 	 */

diff --git a/tests/Cms/Pages/PagePermissionsTest.php b/tests/Cms/Pages/PagePermissionsTest.php
@@ -2,6 +2,7 @@
 
 namespace Kirby\Cms;
 
+use Kirby\Exception\LogicException;
 use Kirby\TestCase;
 
 /**
@@ -270,6 +271,49 @@ public function testWithNobody($action)
 		$this->assertFalse($perms->can($action));
 	}
 
+	/**
+	 * @covers \Kirby\Cms\ModelPermissions::canFromCache
+	 */
+	public function testCanFromCache()
+	{
+		$this->app->impersonate('bastian');
+
+		$page = new Page([
+			'slug'      => 'test',
+			'num'       => 1,
+			'template'  => 'some-template',
+			'blueprint' => [
+				'name' => 'some-template',
+				'options' => [
+					'access' => false,
+					'list'   => false
+				]
+			]
+		]);
+
+		$this->assertFalse(PagePermissions::canFromCache($page, 'access'));
+		$this->assertFalse(PagePermissions::canFromCache($page, 'access'));
+		$this->assertFalse(PagePermissions::canFromCache($page, 'list'));
+		$this->assertFalse(PagePermissions::canFromCache($page, 'list'));
+	}
+
+	/**
+	 * @covers \Kirby\Cms\ModelPermissions::canFromCache
+	 */
+	public function testCanFromCacheDynamic()
+	{
+		$this->expectException(LogicException::class);
+		$this->expectExceptionMessage('Cannot use permission cache for dynamically-determined permission');
+
+		$page = new Page([
+			'slug'     => 'test',
+			'num'      => 1,
+			'template' => 'some-template',
+		]);
+
+		PagePermissions::canFromCache($page, 'changeTemplate');
+	}
+
 	/**
 	 * @covers ::canChangeTemplate
 	 */