From 0c080cfab206529d56e9ca2ecb106cfb00d3827f Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Thu, 19 Dec 2024 21:44:01 +0100 Subject: [PATCH 1/2] New `LanguagePermissions` class Co-authored-by: Ahmet Bora --- src/Cms/Language.php | 8 + src/Cms/LanguagePermissions.php | 22 +++ src/Cms/LanguageRules.php | 18 +-- src/Cms/ModelPermissions.php | 10 +- .../Cms/Languages/LanguagePermissionsTest.php | 145 ++++++++++++++++++ tests/Cms/Languages/LanguageRulesTest.php | 4 +- 6 files changed, 187 insertions(+), 20 deletions(-) create mode 100644 src/Cms/LanguagePermissions.php create mode 100644 tests/Cms/Languages/LanguagePermissionsTest.php diff --git a/src/Cms/Language.php b/src/Cms/Language.php index e1f3a4410c..c5053c359a 100644 --- a/src/Cms/Language.php +++ b/src/Cms/Language.php @@ -426,6 +426,14 @@ public function pattern(): string return $path . '/(:all?)'; } + /** + * Returns the permissions object for this language + */ + public function permissions(): LanguagePermissions + { + return new LanguagePermissions($this); + } + /** * Returns the absolute path to the language file */ diff --git a/src/Cms/LanguagePermissions.php b/src/Cms/LanguagePermissions.php new file mode 100644 index 0000000000..d1e27cbd00 --- /dev/null +++ b/src/Cms/LanguagePermissions.php @@ -0,0 +1,22 @@ + + * @link https://getkirby.com + * @copyright Bastian Allgeier + * @license https://getkirby.com/license + */ +class LanguagePermissions extends ModelPermissions +{ + protected string $category = 'languages'; + + protected function canDelete(): bool + { + return $this->model->isDeletable() === true; + } +} diff --git a/src/Cms/LanguageRules.php b/src/Cms/LanguageRules.php index 1b5b9373c8..d0f65e5147 100644 --- a/src/Cms/LanguageRules.php +++ b/src/Cms/LanguageRules.php @@ -37,9 +37,7 @@ public static function create(Language $language): void ); } - $user = App::instance()->user(); - - if ($user?->role()->permissions()->for('languages', 'create') !== true) { + if ($language->permissions()->create() !== true) { throw new PermissionException( key: 'language.create.permission' ); @@ -54,15 +52,7 @@ public static function create(Language $language): void */ public static function delete(Language $language): void { - if ($language->isDeletable() === false) { - throw new LogicException( - message: 'The language cannot be deleted' - ); - } - - $user = App::instance()->user(); - - if ($user?->role()->permissions()->for('languages', 'delete') !== true) { + if ($language->permissions()->delete() !== true) { throw new PermissionException( key: 'language.delete.permission' ); @@ -93,9 +83,7 @@ public static function update( ); } - $user = $kirby->user(); - - if ($user?->role()->permissions()->for('languages', 'update') !== true) { + if ($newLanguage->permissions()->update() !== true) { throw new PermissionException( key: 'language.update.permission' ); diff --git a/src/Cms/ModelPermissions.php b/src/Cms/ModelPermissions.php index a700b26318..ba9b0010ba 100644 --- a/src/Cms/ModelPermissions.php +++ b/src/Cms/ModelPermissions.php @@ -16,17 +16,21 @@ abstract class ModelPermissions { protected string $category; - protected ModelWithContent $model; + protected ModelWithContent|Language $model; protected array $options; protected Permissions $permissions; protected User $user; - public function __construct(ModelWithContent $model) + public function __construct(ModelWithContent|Language $model) { $this->model = $model; - $this->options = $model->blueprint()->options(); $this->user = $model->kirby()->user() ?? User::nobody(); $this->permissions = $this->user->role()->permissions(); + + $this->options = match (true) { + $model instanceof ModelWithContent => $model->blueprint()->options(), + default => [] + }; } public function __call(string $method, array $arguments = []): bool diff --git a/tests/Cms/Languages/LanguagePermissionsTest.php b/tests/Cms/Languages/LanguagePermissionsTest.php new file mode 100644 index 0000000000..a343db3929 --- /dev/null +++ b/tests/Cms/Languages/LanguagePermissionsTest.php @@ -0,0 +1,145 @@ + [ + 'index' => '/dev/null' + ], + 'roles' => [ + ['name' => 'admin'], + ['name' => 'editor'] + ] + ]); + + $kirby->impersonate('kirby'); + + $language = new Language(['code' => 'en']); + $perms = $language->permissions(); + + $this->assertTrue($perms->can($action)); + } + + /** + * @covers \Kirby\Cms\ModelPermissions::can + * @dataProvider actionProvider + */ + public function testWithNobody($action) + { + new App([ + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + ['name' => 'admin'], + ['name' => 'editor'] + ] + ]); + + $language = new Language(['code' => 'en']); + $perms = $language->permissions(); + + $this->assertFalse($perms->can($action)); + } + + /** + * @covers \Kirby\Cms\ModelPermissions::can + * @dataProvider actionProvider + */ + public function testWithNoAdmin($action) + { + $app = new App([ + 'languages' => [ + [ + 'code' => 'en' + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + ['name' => 'admin'], + [ + 'name' => 'editor', + 'permissions' => [ + 'languages' => [ + 'create' => false, + 'delete' => false, + 'update' => false + ], + ] + ] + ], + 'user' => 'editor@getkirby.com', + 'users' => [ + [ + 'email' => 'admin@getkirby.com', + 'role' => 'admin' + ], + [ + 'email' => 'editor@getkirby.com', + 'role' => 'editor' + ] + ], + ]); + + $language = $app->language('en'); + $perms = $language->permissions(); + + $this->assertSame('editor', $app->role()->name()); + $this->assertFalse($perms->can($action)); + } + + /** + * @covers ::canDelete + */ + public function testCanDeleteWhenNotDeletable() + { + $app = new App([ + 'languages' => [ + [ + 'code' => 'en', + 'default' => true + ], + [ + 'code' => 'de' + ] + ], + 'roots' => [ + 'index' => '/dev/null' + ], + 'roles' => [ + ['name' => 'admin'] + ] + ]); + + $app->impersonate('kirby'); + + $language = $app->language('en'); + $perms = $language->permissions(); + + $this->assertFalse($perms->can('delete')); + } +} diff --git a/tests/Cms/Languages/LanguageRulesTest.php b/tests/Cms/Languages/LanguageRulesTest.php index f8125a5827..a28f963572 100644 --- a/tests/Cms/Languages/LanguageRulesTest.php +++ b/tests/Cms/Languages/LanguageRulesTest.php @@ -159,8 +159,8 @@ public function testDeleteWhenNotDeletable() $language = $this->createMock(Language::class); $language->method('isDeletable')->willReturn(false); - $this->expectException(LogicException::class); - $this->expectExceptionMessage('The language cannot be deleted'); + $this->expectException(PermissionException::class); + $this->expectExceptionMessage('You are not allowed to delete the language'); LanguageRules::delete($language); } From 2be7d6d307e90e73e1daa10813f8388b3e8db5ef Mon Sep 17 00:00:00 2001 From: Lukas Bestle Date: Thu, 19 Dec 2024 21:59:52 +0100 Subject: [PATCH 2/2] Fix `UserPermissions` unit test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without having a current user, the permission would be `false` even if the logic test in `canChangeRole()` would not work, thus making the test useless; in fact just adding the impersonation made the test fail because manually creating a user object didn’t trigger the `isLastAdmin()` logic --- tests/Cms/Users/UserPermissionsTest.php | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/Cms/Users/UserPermissionsTest.php b/tests/Cms/Users/UserPermissionsTest.php index 9c2f87e51f..e8d72b20d5 100644 --- a/tests/Cms/Users/UserPermissionsTest.php +++ b/tests/Cms/Users/UserPermissionsTest.php @@ -122,16 +122,24 @@ public function testWithNoAdmin($action) public function testChangeSingleRole() { - new App([ + $app = new App([ 'roots' => [ 'index' => '/dev/null' ], 'roles' => [ ['name' => 'admin'] + ], + 'users' => [ + [ + 'email' => 'test@getkirby.com', + 'role' => 'admin' + ] ] ]); - $user = new User(['email' => 'test@getkirby.com']); + $app->impersonate('kirby'); + + $user = $app->user('test@getkirby.com'); $perms = $user->permissions(); $this->assertFalse($perms->can('changeRole'));