From f962bcdf0f9d2e535654016036859884a5662902 Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Wed, 16 Aug 2023 20:50:24 +0100 Subject: [PATCH 1/7] Add support for multiple permissions check for users. --- docs/authorization.md | 6 +- src/Authorization/Traits/Authorizable.php | 67 ++++++++++++----------- tests/Authorization/AuthorizableTest.php | 7 +++ 3 files changed, 47 insertions(+), 33 deletions(-) diff --git a/docs/authorization.md b/docs/authorization.md index ee3aa104c..8264748fd 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -111,7 +111,7 @@ The `Authorizable` trait on the `User` entity provides the following methods to #### can() -Allows you to check if a user is permitted to do a specific action. The only argument is the permission string. Returns +Allows you to check if a user is permitted to do a specific action or group or actions. The permission string(s) should be passed as the argument(s). Returns boolean `true`/`false`. Will check the user's direct permissions (**user-level permissions**) first, and then check against all of the user's groups permissions (**group-level permissions**) to determine if they are allowed. @@ -119,6 +119,10 @@ permissions (**group-level permissions**) to determine if they are allowed. if ($user->can('users.create')) { // } +OR +if ($user->can('users.create', 'users.edit')) { + // +} ``` #### inGroup() diff --git a/src/Authorization/Traits/Authorizable.php b/src/Authorization/Traits/Authorizable.php index 9e3f32ff1..bb6f09a97 100644 --- a/src/Authorization/Traits/Authorizable.php +++ b/src/Authorization/Traits/Authorizable.php @@ -228,47 +228,50 @@ public function hasPermission(string $permission): bool * Checks user permissions and their group permissions * to see if the user has a specific permission. * - * @param string $permission string consisting of a scope and action, like `users.create` + * @param string $permission string(s) consisting of a scope and action, like `users.create` */ - public function can(string $permission): bool + public function can(string ...$permissions): bool { - if (strpos($permission, '.') === false) { - throw new LogicException( - 'A permission must be a string consisting of a scope and action, like `users.create`.' - . ' Invalid permission: ' . $permission - ); - } - - $this->populatePermissions(); - - $permission = strtolower($permission); + foreach ($permissions as $permission) { + // Permission must contain a scope and action + if (strpos($permission, '.') === false) { + throw new LogicException( + 'A permission must be a string consisting of a scope and action, like `users.create`.' + . ' Invalid permission: ' . $permission + ); + } - // Check user's permissions - if (in_array($permission, $this->permissionsCache, true)) { - return true; - } + $this->populatePermissions(); - // Check the groups the user belongs to - $this->populateGroups(); + $permission = strtolower($permission); - if (! count($this->groupCache)) { - return false; - } + // Check user's permissions + if (in_array($permission, $this->permissionsCache, true)) { + return true; + } - $matrix = function_exists('setting') - ? setting('AuthGroups.matrix') - : config('AuthGroups')->matrix; + // Check the groups the user belongs to + $this->populateGroups(); - foreach ($this->groupCache as $group) { - // Check exact match - if (isset($matrix[$group]) && in_array($permission, $matrix[$group], true)) { - return true; + if (! count($this->groupCache)) { + return false; } - // Check wildcard match - $check = substr($permission, 0, strpos($permission, '.')) . '.*'; - if (isset($matrix[$group]) && in_array($check, $matrix[$group], true)) { - return true; + $matrix = function_exists('setting') + ? setting('AuthGroups.matrix') + : config('AuthGroups')->matrix; + + foreach ($this->groupCache as $group) { + // Check exact match + if (isset($matrix[$group]) && in_array($permission, $matrix[$group], true)) { + return true; + } + + // Check wildcard match + $check = substr($permission, 0, strpos($permission, '.')) . '.*'; + if (isset($matrix[$group]) && in_array($check, $matrix[$group], true)) { + return true; + } } } diff --git a/tests/Authorization/AuthorizableTest.php b/tests/Authorization/AuthorizableTest.php index 464cfcf1c..2ae9f50a7 100644 --- a/tests/Authorization/AuthorizableTest.php +++ b/tests/Authorization/AuthorizableTest.php @@ -288,6 +288,13 @@ public function testCanCascadesToGroupsSimple(): void $this->assertTrue($this->user->can('admin.access')); } + public function testCanCascadesToGroupsMultiple(): void + { + $this->user->addGroup('superadmin'); + + $this->assertTrue($this->user->can('admin.access', 'users.*')); + } + public function testCanCascadesToGroupsWithWildcards(): void { $this->user->addGroup('superadmin'); From d7ea8445f3cbbbe34fe19cbfcb2cceefec47ce15 Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Wed, 16 Aug 2023 20:59:48 +0100 Subject: [PATCH 2/7] fixed static analysis --- src/Authorization/Traits/Authorizable.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Authorization/Traits/Authorizable.php b/src/Authorization/Traits/Authorizable.php index bb6f09a97..c922fe41f 100644 --- a/src/Authorization/Traits/Authorizable.php +++ b/src/Authorization/Traits/Authorizable.php @@ -226,9 +226,10 @@ public function hasPermission(string $permission): bool /** * Checks user permissions and their group permissions - * to see if the user has a specific permission. + * to see if the user has a specific permission or group + * of permissions. * - * @param string $permission string(s) consisting of a scope and action, like `users.create` + * @param string $permissions string(s) consisting of a scope and action, like `users.create` */ public function can(string ...$permissions): bool { From 35828ffc3eef12c518d08f5bec5fb339d7279338 Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Fri, 18 Aug 2023 00:07:56 +0100 Subject: [PATCH 3/7] Added tests for user's direct permissions and user's group-level permissions --- tests/Authorization/AuthorizableTest.php | 28 ++++++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/tests/Authorization/AuthorizableTest.php b/tests/Authorization/AuthorizableTest.php index 2ae9f50a7..fac80ec94 100644 --- a/tests/Authorization/AuthorizableTest.php +++ b/tests/Authorization/AuthorizableTest.php @@ -288,13 +288,6 @@ public function testCanCascadesToGroupsSimple(): void $this->assertTrue($this->user->can('admin.access')); } - public function testCanCascadesToGroupsMultiple(): void - { - $this->user->addGroup('superadmin'); - - $this->assertTrue($this->user->can('admin.access', 'users.*')); - } - public function testCanCascadesToGroupsWithWildcards(): void { $this->user->addGroup('superadmin'); @@ -312,6 +305,27 @@ public function testCanGetsInvalidPermission(): void $this->assertTrue($this->user->can('developer')); } + /** + * @see https://github.com/codeigniter4/shield/pull/791#discussion_r1297712860 + */ + public function testCanWorksWithMultiplePermissions(): void + { + // Check for user's direct permissions (user-level permissions) + $this->user->addPermission('users.create', 'users.edit'); + + $this->assertTrue($this->user->can('users.create', 'users.edit')); + $this->assertFalse($this->user->can('beta.access', 'admin.access')); + + $this->user->removePermission('users.create', 'users.edit'); + + $this->assertFalse($this->user->can('users.edit', 'users.create')); + + // Check for user's group permissions (group-level permissions) + $this->user->addGroup('superadmin'); + + $this->assertTrue($this->user->can('admin.access', 'beta.access')); + } + /** * @see https://github.com/codeigniter4/shield/pull/238 */ From eb50f850350c2a4a2900c1258313c2b2d6bdc236 Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Fri, 18 Aug 2023 00:10:04 +0100 Subject: [PATCH 4/7] Updated docs --- docs/authorization.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/authorization.md b/docs/authorization.md index 8264748fd..06a2cbc9d 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -119,7 +119,7 @@ permissions (**group-level permissions**) to determine if they are allowed. if ($user->can('users.create')) { // } -OR +// Or if ($user->can('users.create', 'users.edit')) { // } From acbeb07ecbdd792a8afd90e8b89afae3e18c5fb8 Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Fri, 18 Aug 2023 07:32:36 +0100 Subject: [PATCH 5/7] Applied code suggestions --- src/Authorization/Traits/Authorizable.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Authorization/Traits/Authorizable.php b/src/Authorization/Traits/Authorizable.php index c922fe41f..6cee326ce 100644 --- a/src/Authorization/Traits/Authorizable.php +++ b/src/Authorization/Traits/Authorizable.php @@ -233,6 +233,12 @@ public function hasPermission(string $permission): bool */ public function can(string ...$permissions): bool { + // Get user's permissions and store in cache + $this->populatePermissions(); + + // Check the groups the user belongs to + $this->populateGroups(); + foreach ($permissions as $permission) { // Permission must contain a scope and action if (strpos($permission, '.') === false) { @@ -242,8 +248,6 @@ public function can(string ...$permissions): bool ); } - $this->populatePermissions(); - $permission = strtolower($permission); // Check user's permissions @@ -251,9 +255,6 @@ public function can(string ...$permissions): bool return true; } - // Check the groups the user belongs to - $this->populateGroups(); - if (! count($this->groupCache)) { return false; } From c39b70d566ebafea3bb05623f5e9ee9bb16868ff Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Mon, 21 Aug 2023 08:40:32 +0100 Subject: [PATCH 6/7] Added test for wildcard permission matrix --- tests/Authorization/AuthorizableTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Authorization/AuthorizableTest.php b/tests/Authorization/AuthorizableTest.php index fac80ec94..1dc098abf 100644 --- a/tests/Authorization/AuthorizableTest.php +++ b/tests/Authorization/AuthorizableTest.php @@ -324,6 +324,7 @@ public function testCanWorksWithMultiplePermissions(): void $this->user->addGroup('superadmin'); $this->assertTrue($this->user->can('admin.access', 'beta.access')); + $this->assertTrue($this->user->can('admin.*', 'users.*')); } /** From 6d9e5be14c51305c438ce8ac9f300e860ce3a7dd Mon Sep 17 00:00:00 2001 From: Samuel Asor <8720569+sammyskills@users.noreply.github.com> Date: Mon, 21 Aug 2023 04:40:33 +0100 Subject: [PATCH 7/7] Update docs/authorization.md Co-authored-by: kenjis --- docs/authorization.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/authorization.md b/docs/authorization.md index 06a2cbc9d..7265ce6c2 100644 --- a/docs/authorization.md +++ b/docs/authorization.md @@ -119,7 +119,8 @@ permissions (**group-level permissions**) to determine if they are allowed. if ($user->can('users.create')) { // } -// Or + +// If multiple permissions are specified, true is returned if the user has any of them. if ($user->can('users.create', 'users.edit')) { // }