feat: Support hierarchical permissions in method

[bgeneto]

Description

This pull request enhances the can() method in CodeIgniter Shield to support hierarchical permissions, allowing for more granular control over user access. Previously, Shield only supported single-level permissions (e.g., users.create), see #1229 This change enables checking permissions with multiple levels, like admin.users.crud.create, using wildcard matching at each level.

The core change involves modifying the can() method to generate a series of wildcard checks based on the input permission string. It now iteratively checks for all possible parent levels. For example, for admin.users.create, it will check for admin.users.create, admin.users.*, and admin.*.

A new test case, testCanNestedPerms, has been added to verify the correct behavior of the hierarchical permission checks, including various positive and negative scenarios.

This improvement provides greater flexibility in defining and managing permissions within applications built with Shield. It allows for a more natural and organized permission structure, reflecting the hierarchical nature of many application features.

Checklist:

• [x] Securely signed commits
• [x] Component(s) with PHPDoc blocks, only if necessary or adds value (No new PHPDoc blocks were necessary as the existing documentation sufficiently covers the functionality. The method signature remains unchanged.)
• [x] Unit testing, with >80% coverage (New test case testCanNestedPerms added to specifically cover the hierarchical permission logic.)
• [x] User guide updated (changed authorization.md file)
• [x] Conforms to style guide

User Guide Updates (Required):

The following section needs to be added/updated in the User Guide (likely within the "Authorization" or "Permissions" section):

Hierarchical Permissions

Shield now supports hierarchical permissions, allowing you to define permissions with multiple levels separated by dots (.). This enables a more granular and organized approach to authorization.

You can use the wildcard character (*) at any level to represent all permissions within that level.

Example:

If a group has the permission users.manage.*, it will have access to:

• users.manage.create
• users.manage.edit
• users.manage.delete
• users.manage.anything.else

However, it will not have access to:

• users.view
• users.anything.else

The can() method will automatically check all parent levels when evaluating a permission.

Note: This update significantly improves the flexibility of the authorization system. Be sure to review your existing permissions and consider how you can leverage hierarchical permissions to simplify and improve your authorization logic.

[jozefrebjak]

@bgeneto

Hi,

nice PR, I'm currently using something similar. If I understand correctly

For example permissions:

    public array $permissions = [
        'crm.customer.list'           => 'Can access the CRM customer list',
        'crm.customer.show'           => 'Can access the CRM customer',
        'crm.customer.edit'           => 'Can edit a CRM customer',
        'crm.customer.create'         => 'Can create a new CRM customer',
        'crm.customer.delete'         => 'Can delete a CRM customer',
    ];

In matrix then I don't need to write 5 rows in array like:

    public array $matrix = [
        'crm' => [
            'crm.access',
            'crm.customer.list',
            'crm.customer.show',
            'crm.customer.create',
            'crm.customer.edit',
            'crm.customer.delete',
        ],
    ];

but just 1 for crm.customer.* permissions like:

    public array $matrix = [
        'crm' => [
            'crm.access',
            'crm.customer.*',
        ],
    ];

It will work also deeper like for permissions like :

            'crm.service.note.list',
            'crm.service.note.show',
            'crm.service.note.edit',
            'crm.service.note.create',
            'crm.service.note.delete',

to be only:

            'crm.service.note.*,

[michalsn]

Looks useful, although it has a bit of limited functionality. I imagine that for some roles, we would like to allow editing only, not deletion.

So I wonder why not support also this:

foo.*.baz

However, even in the proposed version, this can be error-prone - by oversight. Literally listing all permissions is safer.

Let's get more opinions.

[bgeneto]

Looks useful, although it has a bit of limited functionality. I imagine that for some roles, we would like to allow editing only, not deletion.

So I wonder why not support also this:

foo.*.baz

However, even in the proposed version, this can be error-prone - by oversight. Literally listing all permissions is safer.

Let's get more opinions.

Problem is... Shield does not allows multiple levels of permissions, only one level. And that's also very limited functionality...

[datamweb]

Note: Originally posted by kenjis in https://github.com/codeigniter4/shield/discussions/1177#discussioncomment-10435334

[bgeneto]

Note: Originally posted by kenjis in #1177 (reply in thread)

The following is the implementation contained in this PR. I think it is pretty simple and probably much more efficient than nested loops. I can only see one inherent concern in this change, which I will discuss later.

<?php
    public function can(string $permission): bool
    {
        $this->populatePermissions();

        // Check exact match
        if ($this->permissions !== null && $this->permissions !== [] && in_array($permission, $this->permissions, true)) {
            return true;
        }

        // Check wildcard match
        $checks = [];
        $parts  = explode('.', $permission);

        for ($i = count($parts); $i > 0; $i--) {
            $check    = implode('.', array_slice($parts, 0, $i)) . '.*';
            $checks[] = $check;
        }

        return $this->permissions !== null
            && $this->permissions !== []
            && array_intersect($checks, $this->permissions) !== [];
    }

Key Differences Between Original and Improved Code

1. Enhanced Wildcard Matching:

   • Original: Only checks one wildcard pattern - the first segment followed by .*
   • Improved: Checks multiple levels of wildcard patterns by creating a hierarchy of permission checks

2. Wildcard Generation Approach:

   • Original: Manually extracts the first segment using strpos and substr
   • Improved: Uses explode to split the permission string and then constructs multiple wildcard patterns

3. Comparison Method:

   • Original: Uses in_array for a single wildcard check
   • Improved: Uses array_intersect to check multiple wildcard patterns at once

Functional Improvements

The improved implementation provides more granular permission checking by supporting hierarchical wildcards. For example, with a permission like "users.posts.edit":

• Original can()code only checks for "users.*" and since it does not support more than one level it will fail (return false)
• Improved code checks for "users.*", "users.posts.*", and the exact "users.posts.edit"

This allows for more flexible permission structures where wildcards can exist at different levels of the hierarchy.

Concerns

My concern was that array_intersect() performs loose comparisons internally, which could potentially lead to type juggling vulnerabilities. This would happen if a user could somehow manipulate the input to the permission check, forcing it to be a different type (e.g., an integer instead of a string). However, this risk is very low in practice. The $permissions array is loaded from a configuration file, not directly from user input, and the permission string itself is usually hardcoded. Therefore, the data being compared is tightly controlled, making type juggling exploits highly unlikely. While a denial-of-service (DoS) attack is theoretically possible with extremely long permission strings, this is also improbable due to the controlled nature of the input. Overall, I think the added functionality and flexibility of the new implementation significantly outweigh these minor, theoretical concerns.

PS.: one can still lists all permissions literally like this: users.posts.edit, users.posts.read, users.post.delete... and they can() be checked correctly with this PR.

[christianberkman]

I would really love to be able to use this functionality. It would be exactly what I need in my project to simplify permissions! Since it is not a breaking change and optional to use, are there many concerns in implementing this? I understand the loose comparisons are a very low risk as described by @bgeneto .

[michalsn]

I'm happy to approve this PR once we have some documentation in place. It also needs to be rebased, which will hopefully resolve the Psalm errors.

[christianberkman]

@bgeneto Could you rebase your PR? I am happy to contribute some docs or tests (if it is possible for me to contribute to this PR somehow?)

[bgeneto]

@bgeneto Could you rebase your PR? I am happy to contribute some docs or tests (if it is possible for me to contribute to this PR somehow?)

I've added some minor documentation and rebased this PR, but PHPStan and PHPCSFixer errors remain. I'm not sure what else to do. If you'd like to take over, feel free to fork my 'multilevel-hierarchical-permissions' branch.

https://github.com/bgeneto/ci4-shield/tree/multilevel-hierarchical-permissions

[christianberkman]

@michalsn I am seeing "Detect Merge Commits" failure. If I fork @bgeneto's repo will these errors persist, so in other words should I open a fresh PR from the current dev branch?

[michalsn]

@christianberkman Yes, create a fresh branch from dev, and cherry-pick only the clean, relevant commits from the old PR.

Alternatively, you can copy the changes that OP made and just credit him in the commit message, see: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

At the end of the day, I always squash commits on merge, so the effect will be similar.