From 3832c97de4ae30972a716f03d6e6be912d0215a0 Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Tue, 17 Dec 2024 21:35:31 +0100 Subject: [PATCH 1/3] Move `$app->roles()` to `AppUsers` trait Improves consistency and overview --- src/Cms/App.php | 8 -------- src/Cms/AppUsers.php | 8 ++++++++ tests/Cms/App/AppRolesTest.php | 35 ---------------------------------- tests/Cms/App/AppUsersTest.php | 31 ++++++++++++++++++++++++++++-- 4 files changed, 37 insertions(+), 45 deletions(-) delete mode 100644 tests/Cms/App/AppRolesTest.php diff --git a/src/Cms/App.php b/src/Cms/App.php index 88a9084211..10288260cc 100644 --- a/src/Cms/App.php +++ b/src/Cms/App.php @@ -1331,14 +1331,6 @@ public function response(): Responder return $this->response ??= new Responder(); } - /** - * Returns all user roles - */ - public function roles(): Roles - { - return $this->roles ??= Roles::load($this->root('roles')); - } - /** * Returns a system root */ diff --git a/src/Cms/AppUsers.php b/src/Cms/AppUsers.php index 6f2f7be524..23b8470dee 100644 --- a/src/Cms/AppUsers.php +++ b/src/Cms/AppUsers.php @@ -67,6 +67,14 @@ public function impersonate( } } + /** + * Returns all user roles + */ + public function roles(): Roles + { + return $this->roles ??= Roles::load($this->root('roles')); + } + /** * Set the currently active user id * diff --git a/tests/Cms/App/AppRolesTest.php b/tests/Cms/App/AppRolesTest.php deleted file mode 100644 index 370ef20f13..0000000000 --- a/tests/Cms/App/AppRolesTest.php +++ /dev/null @@ -1,35 +0,0 @@ - [ - [ - 'name' => 'editor', - 'title' => 'Editor' - ] - ] - ]); - - $this->assertCount(2, $app->roles()); - $this->assertSame('editor', $app->roles()->last()->name()); - } - - public function testLoad() - { - $app = new App([ - 'roots' => [ - 'site' => static::FIXTURES - ] - ]); - - $this->assertCount(2, $app->roles()); - $this->assertSame('editor', $app->roles()->last()->name()); - } -} diff --git a/tests/Cms/App/AppUsersTest.php b/tests/Cms/App/AppUsersTest.php index ba634f80d0..5acaf187a2 100644 --- a/tests/Cms/App/AppUsersTest.php +++ b/tests/Cms/App/AppUsersTest.php @@ -110,7 +110,34 @@ public function testImpersonateErrorMissingUser() $this->app->impersonate('homer@simpsons.com'); } - public function testLoad() + public function testRolesSet() + { + $app = new App([ + 'roles' => [ + [ + 'name' => 'editor', + 'title' => 'Editor' + ] + ] + ]); + + $this->assertCount(2, $app->roles()); + $this->assertSame('editor', $app->roles()->last()->name()); + } + + public function testRolesLoad() + { + $app = new App([ + 'roots' => [ + 'site' => static::FIXTURES + ] + ]); + + $this->assertCount(2, $app->roles()); + $this->assertSame('editor', $app->roles()->last()->name()); + } + + public function testUsersLoad() { $app = $this->app->clone([ 'roots' => [ @@ -122,7 +149,7 @@ public function testLoad() $this->assertSame('user@getkirby.com', $app->users()->first()->email()); } - public function testSet() + public function testUsersSet() { $app = $this->app->clone([ 'users' => [ From 023eace20872e1bf2b4e072a9f004384048eb9a5 Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Tue, 17 Dec 2024 21:35:53 +0100 Subject: [PATCH 2/3] New `$app->role()` method --- src/Cms/AppUsers.php | 19 +++++++++++ tests/Cms/App/AppUsersTest.php | 61 ++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/Cms/AppUsers.php b/src/Cms/AppUsers.php index 23b8470dee..f2b63990e3 100644 --- a/src/Cms/AppUsers.php +++ b/src/Cms/AppUsers.php @@ -75,6 +75,25 @@ public function roles(): Roles return $this->roles ??= Roles::load($this->root('roles')); } + /** + * Returns a specific user role by id + * or the role of the current user if no id is given + * + * @param bool $allowImpersonation If set to false, only the role of the + * actually logged in user will be returned + * (when `$id` is passed as `null`) + */ + public function role( + string|null $id = null, + bool $allowImpersonation = true + ): Role|null { + if ($id !== null) { + return $this->roles()->find($id); + } + + return $this->user(null, $allowImpersonation)?->role(); + } + /** * Set the currently active user id * diff --git a/tests/Cms/App/AppUsersTest.php b/tests/Cms/App/AppUsersTest.php index 5acaf187a2..0a296a86a5 100644 --- a/tests/Cms/App/AppUsersTest.php +++ b/tests/Cms/App/AppUsersTest.php @@ -137,6 +137,67 @@ public function testRolesLoad() $this->assertSame('editor', $app->roles()->last()->name()); } + public function testRoleManual() + { + $app = new App([ + 'roles' => [ + [ + 'name' => 'editor', + 'title' => 'Editor' + ] + ] + ]); + + $this->assertSame('editor', $app->role('editor')->name()); + $this->assertNull($app->role('something')); + } + + public function testRoleFromUser() + { + $app = new App([ + 'roles' => [ + [ + 'name' => 'editor', + 'title' => 'Editor' + ] + ], + 'users' => [ + [ + 'email' => 'user@getkirby.com', + 'role' => 'editor' + ] + ] + ]); + + $app->auth()->setUser($app->user('user@getkirby.com')); + + $this->assertSame('editor', $app->role()->name()); + $this->assertSame('editor', $app->role(null, false)->name()); + } + + public function testRoleFromImpersonatedUser() + { + $app = new App([ + 'roles' => [ + [ + 'name' => 'editor', + 'title' => 'Editor' + ] + ], + 'users' => [ + [ + 'email' => 'user@getkirby.com', + 'role' => 'editor' + ] + ] + ]); + + $app->impersonate('user@getkirby.com'); + + $this->assertSame('editor', $app->role()->name()); + $this->assertNull($app->role(null, false)); + } + public function testUsersLoad() { $app = $this->app->clone([ From 44735cadb8ee6e1b8b865553a27db9d6cd256c0b Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Tue, 17 Dec 2024 21:43:48 +0100 Subject: [PATCH 3/3] Use new `$app->role()` method in core code --- config/areas/languages/views.php | 2 +- src/Cms/File.php | 6 +++--- src/Cms/Page.php | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/config/areas/languages/views.php b/config/areas/languages/views.php index 2dcdcd84a3..ccc9ac3cc4 100644 --- a/config/areas/languages/views.php +++ b/config/areas/languages/views.php @@ -22,7 +22,7 @@ // TODO: update following line and adapt for update and // delete options when `languageVariables.*` permissions available - $canUpdate = $kirby->user()?->role()->permissions()->for('languages', 'update') === true; + $canUpdate = $kirby->role()?->permissions()->for('languages', 'update') === true; ksort($foundation); diff --git a/src/Cms/File.php b/src/Cms/File.php index 9bf38cd2db..3113a10c35 100644 --- a/src/Cms/File.php +++ b/src/Cms/File.php @@ -319,7 +319,7 @@ public function isAccessible(): bool } static $accessible = []; - $role = $this->kirby()->user()?->role()->id() ?? '__none__'; + $role = $this->kirby()->role()?->id() ?? '__none__'; $template = $this->template() ?? '__none__'; $accessible[$role] ??= []; @@ -343,7 +343,7 @@ public function isListable(): bool } static $listable = []; - $role = $this->kirby()->user()?->role()->id() ?? '__none__'; + $role = $this->kirby()->role()?->id() ?? '__none__'; $template = $this->template() ?? '__none__'; $listable[$role] ??= []; @@ -358,7 +358,7 @@ public function isListable(): bool public function isReadable(): bool { static $readable = []; - $role = $this->kirby()->user()?->role()->id() ?? '__none__'; + $role = $this->kirby()->role()?->id() ?? '__none__'; $template = $this->template() ?? '__none__'; $readable[$role] ??= []; diff --git a/src/Cms/Page.php b/src/Cms/Page.php index 97f7d80a1e..45e78c1b0b 100644 --- a/src/Cms/Page.php +++ b/src/Cms/Page.php @@ -523,7 +523,7 @@ public function isAccessible(): bool } static $accessible = []; - $role = $this->kirby()->user()?->role()->id() ?? '__none__'; + $role = $this->kirby()->role()?->id() ?? '__none__'; $template = $this->intendedTemplate()->name(); $accessible[$role] ??= []; @@ -695,7 +695,7 @@ public function isListable(): bool } static $listable = []; - $role = $this->kirby()->user()?->role()->id() ?? '__none__'; + $role = $this->kirby()->role()?->id() ?? '__none__'; $template = $this->intendedTemplate()->name(); $listable[$role] ??= []; @@ -753,7 +753,7 @@ public function isPublished(): bool public function isReadable(): bool { static $readable = []; - $role = $this->kirby()->user()?->role()->id() ?? '__none__'; + $role = $this->kirby()->role()?->id() ?? '__none__'; $template = $this->intendedTemplate()->name(); $readable[$role] ??= [];