diff --git a/src/Cms/File.php b/src/Cms/File.php index 4e6b75ce3f..1322b482d5 100644 --- 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 index a35cc65753..6c7eee8fe5 100644 --- 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 index 4536c50e70..798623bf35 100644 --- 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 index 766369c81e..ce60b171bf 100644 --- 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 index 92aaadea41..01610796e9 100644 --- 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 index ebdd36c6ce..4103fca808 100644 --- 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 index ec1cfac989..3d96b2ec61 100644 --- 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 index 2d8ff56de4..9f51ab01d1 100644 --- 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 */