From 0a2153e31cca67fcccb506064d2658c362b2d34c Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Mon, 23 Dec 2024 13:56:40 +0100 Subject: [PATCH] New access and list permissions Co-authored-by: Ahmet Bora --- src/Cms/Language.php | 21 ++ src/Cms/Permissions.php | 7 + src/Cms/Site.php | 8 + src/Cms/User.php | 21 ++ .../Cms/Languages/LanguagePermissionsTest.php | 41 +++ tests/Cms/Languages/LanguageTest.php | 97 ++++++ tests/Cms/Permissions/PermissionsTest.php | 7 + tests/Cms/Site/SitePermissionsTest.php | 34 ++ tests/Cms/Site/SiteTest.php | 65 ++++ tests/Cms/Users/UserPermissionsTest.php | 38 ++- tests/Cms/Users/UserTest.php | 301 ++++++++++++++++++ 11 files changed, 639 insertions(+), 1 deletion(-) diff --git a/src/Cms/Language.php b/src/Cms/Language.php index c5053c359a..2eef32fae2 100644 --- a/src/Cms/Language.php +++ b/src/Cms/Language.php @@ -300,6 +300,14 @@ public function exists(): bool return file_exists($this->root()); } + /** + * Checks if the language is accessible to the current user + */ + public function isAccessible(): bool + { + return $this->permissions()->access(); + } + /** * Checks if this is the default language * for the site. @@ -335,6 +343,19 @@ public function isLast(): bool return App::instance()->languages()->count() === 1; } + /** + * Checks if the language is listable by the current user + */ + public function isListable(): bool + { + // not accessible also means not listable + if ($this->isAccessible() === false) { + return false; + } + + return $this->permissions()->list(); + } + /** * Checks if this is the single language object * @internal diff --git a/src/Cms/Permissions.php b/src/Cms/Permissions.php index a89ff22a49..d584337d7d 100644 --- a/src/Cms/Permissions.php +++ b/src/Cms/Permissions.php @@ -41,8 +41,10 @@ class Permissions 'update' => true ], 'languages' => [ + 'access' => true, 'create' => true, 'delete' => true, + 'list' => true, 'update' => true ], 'pages' => [ @@ -62,10 +64,12 @@ class Permissions 'update' => true ], 'site' => [ + 'access' => true, 'changeTitle' => true, 'update' => true ], 'users' => [ + 'access' => true, 'changeEmail' => true, 'changeLanguage' => true, 'changeName' => true, @@ -73,15 +77,18 @@ class Permissions 'changeRole' => true, 'create' => true, 'delete' => true, + 'list' => true, 'update' => true ], 'user' => [ + 'access' => true, 'changeEmail' => true, 'changeLanguage' => true, 'changeName' => true, 'changePassword' => true, 'changeRole' => true, 'delete' => true, + 'list' => true, 'update' => true ] ]; diff --git a/src/Cms/Site.php b/src/Cms/Site.php index fa8a2a336a..4941bc81e3 100644 --- a/src/Cms/Site.php +++ b/src/Cms/Site.php @@ -266,6 +266,14 @@ public function is($site): bool return $this === $site; } + /** + * Checks if the site is accessible to the current user + */ + public function isAccessible(): bool + { + return $this->permissions()->access(); + } + /** * Returns the root to the media folder for the site * @internal diff --git a/src/Cms/User.php b/src/Cms/User.php index 3eb6868b0b..baa8f2d495 100644 --- a/src/Cms/User.php +++ b/src/Cms/User.php @@ -281,6 +281,14 @@ public function is(User|null $user = null): bool return $this->id() === $user->id(); } + /** + * Checks if the user is accessible to the current user + */ + public function isAccessible(): bool + { + return $this->permissions()->access(); + } + /** * Checks if this user has the admin role */ @@ -298,6 +306,19 @@ public function isKirby(): bool return $this->isAdmin() && $this->id() === 'kirby'; } + /** + * Checks if the user is listable by the current user + */ + public function isListable(): bool + { + // not accessible also means not listable + if ($this->isAccessible() === false) { + return false; + } + + return $this->permissions()->list(); + } + /** * Checks if the current user is this user */ diff --git a/tests/Cms/Languages/LanguagePermissionsTest.php b/tests/Cms/Languages/LanguagePermissionsTest.php index a343db3929..52e157857a 100644 --- a/tests/Cms/Languages/LanguagePermissionsTest.php +++ b/tests/Cms/Languages/LanguagePermissionsTest.php @@ -112,6 +112,47 @@ public function testWithNoAdmin($action) $this->assertFalse($perms->can($action)); } + /** + * @covers \Kirby\Cms\ModelPermissions::can + */ + public function testCaching() + { + $app = new App([ + 'languages' => [ + [ + 'code' => 'en' + ] + ], + 'roles' => [ + [ + 'name' => 'editor', + 'permissions' => [ + 'languages' => [ + 'access' => false, + 'list' => false + ], + ] + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'users' => [ + ['id' => 'bastian', 'role' => 'editor'], + + ] + ]); + + $app->impersonate('bastian'); + + $language = $app->language('en'); + + $this->assertFalse($language->permissions()->can('access')); + $this->assertFalse($language->permissions()->can('access')); + $this->assertFalse($language->permissions()->can('list')); + $this->assertFalse($language->permissions()->can('list')); + } + /** * @covers ::canDelete */ diff --git a/tests/Cms/Languages/LanguageTest.php b/tests/Cms/Languages/LanguageTest.php index fd451faf1d..8a68c5e6cf 100644 --- a/tests/Cms/Languages/LanguageTest.php +++ b/tests/Cms/Languages/LanguageTest.php @@ -393,6 +393,47 @@ public function testExists() $this->assertTrue($language->exists()); } + /** + * @covers ::isAccessible + */ + public function testIsAccessible() + { + $app = new App([ + 'languages' => [ + [ + 'code' => 'en' + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor', + 'permissions' => [ + 'languages' => [ + 'access' => false + ], + ] + ] + ], + 'users' => [ + [ + 'email' => 'editor@getkirby.com', + 'role' => 'editor' + ] + ], + ]); + + $language = $app->language('en'); + + $app->impersonate('editor@getkirby.com'); + $this->assertFalse($language->isAccessible()); + + $app->impersonate('kirby'); + $this->assertTrue($language->isAccessible()); + } + /** * @covers ::isDefault */ @@ -426,6 +467,62 @@ public function testIsDefault() $this->assertFalse($language->isDefault()); } + /** + * @covers ::isListable + */ + public function testIsListable() + { + $app = new App([ + 'languages' => [ + [ + 'code' => 'en' + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor-access', + 'permissions' => [ + 'languages' => [ + 'access' => false + ], + ] + ], + [ + 'name' => 'editor-list', + 'permissions' => [ + 'languages' => [ + 'list' => false + ], + ] + ] + ], + 'users' => [ + [ + 'email' => 'editor-access@getkirby.com', + 'role' => 'editor-access' + ], + [ + 'email' => 'editor-list@getkirby.com', + 'role' => 'editor-list' + ] + ], + ]); + + $language = $app->language('en'); + + $app->impersonate('editor-access@getkirby.com'); + $this->assertFalse($language->isListable()); + + $app->impersonate('editor-list@getkirby.com'); + $this->assertFalse($language->isListable()); + + $app->impersonate('kirby'); + $this->assertTrue($language->isListable()); + } + /** * @covers ::isSingle */ diff --git a/tests/Cms/Permissions/PermissionsTest.php b/tests/Cms/Permissions/PermissionsTest.php index 44aa538b7d..3e3a58a4a9 100644 --- a/tests/Cms/Permissions/PermissionsTest.php +++ b/tests/Cms/Permissions/PermissionsTest.php @@ -27,8 +27,10 @@ public static function actionsProvider(): array ['files', 'replace'], ['files', 'update'], + ['languages', 'access'], ['languages', 'create'], ['languages', 'delete'], + ['languages', 'list'], ['languages', 'update'], ['pages', 'access'], @@ -46,9 +48,11 @@ public static function actionsProvider(): array ['pages', 'sort'], ['pages', 'update'], + ['site', 'access'], ['site', 'changeTitle'], ['site', 'update'], + ['users', 'access'], ['users', 'changeEmail'], ['users', 'changeLanguage'], ['users', 'changeName'], @@ -56,14 +60,17 @@ public static function actionsProvider(): array ['users', 'changeRole'], ['users', 'create'], ['users', 'delete'], + ['users', 'list'], ['users', 'update'], + ['user', 'access'], ['user', 'changeEmail'], ['user', 'changeLanguage'], ['user', 'changeName'], ['user', 'changePassword'], ['user', 'changeRole'], ['user', 'delete'], + ['user', 'list'], ['user', 'update'], ]; } diff --git a/tests/Cms/Site/SitePermissionsTest.php b/tests/Cms/Site/SitePermissionsTest.php index 12c71608bf..dac2260ed4 100644 --- a/tests/Cms/Site/SitePermissionsTest.php +++ b/tests/Cms/Site/SitePermissionsTest.php @@ -9,6 +9,7 @@ class SitePermissionsTest extends TestCase public static function actionProvider(): array { return [ + ['access'], ['changeTitle'], ['update'], ]; @@ -49,4 +50,37 @@ public function testWithNobody($action) $this->assertFalse($perms->can($action)); } + + /** + * @covers \Kirby\Cms\ModelPermissions::can + */ + public function testCaching() + { + $app = new App([ + 'roles' => [ + [ + 'name' => 'editor', + 'permissions' => [ + 'site' => [ + 'access' => false + ], + ] + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'users' => [ + ['id' => 'bastian', 'role' => 'editor'], + + ] + ]); + + $app->impersonate('bastian'); + + $site = $app->site(); + + $this->assertFalse($site->permissions()->can('access')); + $this->assertFalse($site->permissions()->can('access')); + } } diff --git a/tests/Cms/Site/SiteTest.php b/tests/Cms/Site/SiteTest.php index 75631764e0..5741411e01 100644 --- a/tests/Cms/Site/SiteTest.php +++ b/tests/Cms/Site/SiteTest.php @@ -186,6 +186,71 @@ public function testIs() $this->assertFalse($b->is($c)); } + public function testIsAccessible() + { + $app = new App([ + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor', + 'permissions' => [ + 'site' => [ + 'access' => false + ], + ] + ] + ], + 'users' => [ + [ + 'email' => 'editor@getkirby.com', + 'role' => 'editor' + ] + ], + ]); + + $site = $app->site(); + + $app->impersonate('editor@getkirby.com'); + $this->assertFalse($site->isAccessible()); + + $app->impersonate('kirby'); + $this->assertTrue($site->isAccessible()); + } + + public function testIsAccessibleBlueprint() + { + $app = new App([ + 'blueprints' => [ + 'site' => [ + 'options' => ['access' => false] + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor' + ] + ], + 'users' => [ + [ + 'email' => 'editor@getkirby.com', + 'role' => 'editor' + ] + ], + ]); + + $site = $app->site(); + + $app->impersonate('editor@getkirby.com'); + $this->assertFalse($site->isAccessible()); + + $app->impersonate('kirby'); + $this->assertTrue($site->isAccessible()); + } public static function previewUrlProvider(): array { diff --git a/tests/Cms/Users/UserPermissionsTest.php b/tests/Cms/Users/UserPermissionsTest.php index 0b4a06f058..7cf384938d 100644 --- a/tests/Cms/Users/UserPermissionsTest.php +++ b/tests/Cms/Users/UserPermissionsTest.php @@ -9,13 +9,15 @@ class UserPermissionsTest extends TestCase public static function actionProvider(): array { return [ - ['create'], + ['access'], ['changeEmail'], ['changeLanguage'], ['changeName'], ['changePassword'], ['changeRole'], + ['create'], ['delete'], + ['list'], ['update'], ]; } @@ -124,6 +126,40 @@ public function testWithNoAdmin($action) $this->assertTrue($perms2->can($action)); } + /** + * @covers \Kirby\Cms\ModelPermissions::can + */ + public function testCaching() + { + $app = new App([ + 'roots' => [ + 'index' => '/dev/null' + ], + 'users' => [ + ['id' => 'bastian', 'role' => 'admin'], + + ] + ]); + + $app->impersonate('bastian'); + + $user = new User([ + 'role' => 'editor', + 'blueprint' => [ + 'name' => 'users/editor', + 'options' => [ + 'access' => false, + 'list' => false + ] + ] + ]); + + $this->assertFalse($user->permissions()->can('access')); + $this->assertFalse($user->permissions()->can('access')); + $this->assertFalse($user->permissions()->can('list')); + $this->assertFalse($user->permissions()->can('list')); + } + public function testChangeSingleRole() { $app = new App([ diff --git a/tests/Cms/Users/UserTest.php b/tests/Cms/Users/UserTest.php index d9bf69d8e8..651036cabc 100644 --- a/tests/Cms/Users/UserTest.php +++ b/tests/Cms/Users/UserTest.php @@ -72,6 +72,137 @@ public function testInvalidEmail() new User(['email' => []]); } + /** + * @covers ::isAccessible + */ + public function testIsAccessibleUsersUser() + { + $app = new App([ + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor-global', + 'permissions' => [ + 'users' => [ + 'access' => false + ], + ] + ], + [ + 'name' => 'editor-own', + 'permissions' => [ + 'user' => [ + 'access' => false + ], + ] + ], + [ + 'name' => 'editor-both', + 'permissions' => [ + 'user' => [ + 'access' => false + ], + 'users' => [ + 'access' => false + ], + ] + ] + ], + 'users' => [ + [ + 'email' => 'editor-global@getkirby.com', + 'role' => 'editor-global' + ], + [ + 'email' => 'editor-own@getkirby.com', + 'role' => 'editor-own' + ], + [ + 'email' => 'editor-both@getkirby.com', + 'role' => 'editor-both' + ] + ], + ]); + + $userGlobal = $app->user('editor-global@getkirby.com'); + $userOwn = $app->user('editor-own@getkirby.com'); + $userBoth = $app->user('editor-both@getkirby.com'); + + // user with only `users.access` disabled can access themselves + $app->impersonate('editor-global@getkirby.com'); + $this->assertTrue($userGlobal->isAccessible()); + $this->assertFalse($userOwn->isAccessible()); + $this->assertFalse($userBoth->isAccessible()); + + // users with only `user.access` disabled can access everyone else + $app->impersonate('editor-own@getkirby.com'); + $this->assertTrue($userGlobal->isAccessible()); + $this->assertFalse($userOwn->isAccessible()); + $this->assertTrue($userBoth->isAccessible()); + + // users with both disabled can't access anything + $app->impersonate('editor-both@getkirby.com'); + $this->assertFalse($userGlobal->isAccessible()); + $this->assertFalse($userOwn->isAccessible()); + $this->assertFalse($userBoth->isAccessible()); + + // almighty Kirby user can access everything + $app->impersonate('kirby'); + $this->assertTrue($userGlobal->isAccessible()); + $this->assertTrue($userOwn->isAccessible()); + $this->assertTrue($userBoth->isAccessible()); + } + + /** + * @covers ::isAccessible + */ + public function testIsAccessibleBlueprint() + { + $app = new App([ + 'blueprints' => [ + 'users/editor-other' => [ + 'options' => ['access' => false] + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor' + ], + [ + 'name' => 'editor-other' + ] + ], + 'users' => [ + [ + 'email' => 'editor@getkirby.com', + 'role' => 'editor' + ], + [ + 'email' => 'editor-other@getkirby.com', + 'role' => 'editor-other' + ] + ], + ]); + + $user = $app->user('editor@getkirby.com'); + $userOther = $app->user('editor-other@getkirby.com'); + + // no access to role that has the option disabled + $app->impersonate('editor@getkirby.com'); + $this->assertTrue($user->isAccessible()); + $this->assertFalse($userOther->isAccessible()); + + // almighty Kirby user can access everything + $app->impersonate('kirby'); + $this->assertTrue($user->isAccessible()); + $this->assertTrue($userOther->isAccessible()); + } + /** * @covers ::isAdmin */ @@ -119,6 +250,176 @@ public function testIsKirby() $this->assertFalse($user->isKirby()); } + /** + * @covers ::isListable + */ + public function testIsListableUsersUser() + { + $app = new App([ + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor-global', + 'permissions' => [ + 'users' => [ + 'list' => false + ], + ] + ], + [ + 'name' => 'editor-own', + 'permissions' => [ + 'user' => [ + 'list' => false + ], + ] + ], + [ + 'name' => 'editor-both', + 'permissions' => [ + 'user' => [ + 'list' => false + ], + 'users' => [ + 'list' => false + ], + ] + ] + ], + 'users' => [ + [ + 'email' => 'editor-global@getkirby.com', + 'role' => 'editor-global' + ], + [ + 'email' => 'editor-own@getkirby.com', + 'role' => 'editor-own' + ], + [ + 'email' => 'editor-both@getkirby.com', + 'role' => 'editor-both' + ] + ], + ]); + + $userGlobal = $app->user('editor-global@getkirby.com'); + $userOwn = $app->user('editor-own@getkirby.com'); + $userBoth = $app->user('editor-both@getkirby.com'); + + // user with only `users.list` disabled can list themselves + $app->impersonate('editor-global@getkirby.com'); + $this->assertTrue($userGlobal->isListable()); + $this->assertFalse($userOwn->isListable()); + $this->assertFalse($userBoth->isListable()); + + // users with only `user.list` disabled can list everyone else + $app->impersonate('editor-own@getkirby.com'); + $this->assertTrue($userGlobal->isListable()); + $this->assertFalse($userOwn->isListable()); + $this->assertTrue($userBoth->isListable()); + + // users with both disabled can't list anything + $app->impersonate('editor-both@getkirby.com'); + $this->assertFalse($userGlobal->isListable()); + $this->assertFalse($userOwn->isListable()); + $this->assertFalse($userBoth->isListable()); + + // almighty Kirby user can list everything + $app->impersonate('kirby'); + $this->assertTrue($userGlobal->isListable()); + $this->assertTrue($userOwn->isListable()); + $this->assertTrue($userBoth->isListable()); + } + + /** + * @covers ::isListable + */ + public function testIsListableBlueprint() + { + $app = new App([ + 'blueprints' => [ + 'users/editor-other' => [ + 'options' => ['list' => false] + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor' + ], + [ + 'name' => 'editor-other' + ] + ], + 'users' => [ + [ + 'email' => 'editor@getkirby.com', + 'role' => 'editor' + ], + [ + 'email' => 'editor-other@getkirby.com', + 'role' => 'editor-other' + ] + ], + ]); + + $user = $app->user('editor@getkirby.com'); + $userOther = $app->user('editor-other@getkirby.com'); + + // no access to role that has the option disabled + $app->impersonate('editor@getkirby.com'); + $this->assertTrue($user->isListable()); + $this->assertFalse($userOther->isListable()); + + // almighty Kirby user can access everything + $app->impersonate('kirby'); + $this->assertTrue($user->isListable()); + $this->assertTrue($userOther->isListable()); + } + + /** + * @covers ::isListable + */ + public function testIsListableDependentOnAccess() + { + $app = new App([ + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + [ + 'name' => 'editor-both', + 'permissions' => [ + 'user' => [ + 'access' => false + ], + 'users' => [ + 'access' => false + ], + ] + ], + ], + 'users' => [ + [ + 'email' => 'editor-both@getkirby.com', + 'role' => 'editor-both' + ] + ], + ]); + + $user = $app->user('editor-both@getkirby.com'); + + $app->impersonate('editor-both@getkirby.com'); + $this->assertFalse($user->isListable()); + + $app->impersonate('kirby'); + $this->assertTrue($user->isListable()); + } + /** * @covers ::isLoggedIn */