Bug: authorization using asterisk (*) only works one level

bgeneto commented 6 days ago

PHP Version

8.3.13

CodeIgniter4 Version

4.5.5

Shield Version

1.1

Which operating systems have you tested for this bug?

Linux

Which server did you use?

fpm-fcgi

Database

SQLite3, MariaDB, Postgres

Did you customize Shield?

No

What happened?

Assigning permissions to a group using asterisk (*) does not work with multiple level properties: (it works one level only, but, unfortunately, shield documentation does not mentions that lib limitation)

    public array $permissions = [
        'admin.access'        => 'Can access the sites admin area',
        'admin.settings'      => 'Can access the main site settings',
        'admin.settings.theme'  => 'Can change site theme',  // this won't work with assigning permissions using asterisk (*)         
        ...
        'test.permissions.one' => 'Can access permission one', // won't work with asterisk (*) 
        'test.permissions.two' => 'Can access permission two', // won't work with asterisk (*) 
    ];

    public array $matrix = [
        'superadmin' => [
            'admin.*', // only first level will be granted (access and settings, not settings.theme
            'users.*',
            'sys.*',
            'test.*', // no settings will be granted (or, one cannot use can() or hasPermission()
        ],
        ...
        'user' => [
            'test.*', // does not work 
            'test.permissions.*', // does'n work either
        ],

Steps to Reproduce

Add the above config AuthGroups config class. try to check for authorization withing a controller:

if (auth()->user()->can('test.permissions.one')) {
//if (auth()->user()->hasPermission('test.permissions.one')) {
    return "You CAN test.permissions.one!";
} else {
    return "You CAN'T test.permissions.one!";
}

Expected Output

"You CAN test.permissions.one!"

Anything else?

I suspect this limitation is related to CI4 Settings library Known Limitations. IMHO, if this limitation is settings library responsibility, it is very counterproductive for shield to use it.

CosDiabos commented 6 days ago

Did you add the test group to the $groups array? I'm asking because you don't mention it on your post.

 'user' => [
           'test.permissions.*', // also does'n work 
       ],

This snippet seems from the $matrix array. The 'superadmin' group also uses the asterisk as a wildcard. If assigning an user as SA works, it probably is a config problem.

bgeneto commented 5 days ago

Did you add the test group to the $groups array? I'm asking because you don't mention it on your post.

No need. I'm not creating a new (user) group, just new permissions...

This snippet seems from the $matrix array. The 'superadmin' group also uses the asterisk as a wildcard. If assigning an user as SA works, it probably is a config problem.

The superadmin uses * for one level only, you can try with more levels (like 'admin.settings.theme') and it won't work:

    public array $permissions = [
        'admin.access'                => 'Can access the sites admin area',
        'admin.settings'              => 'Can access the main site settings',
        'admin.settings.theme'    => 'Can change site theme',

CosDiabos commented 5 days ago

Hey!

No need. I'm not creating a new (user) group, just new permissions...

You're right, I misunderstood it. Sorry!

So, I spent a little bit of time on this issue, testing some examples and from what I gathered looking at the can() method, it checks if the provided can() permission is an exact match with $permissions array, if it isn't then, looks for the same permission a level up with a wildcard, however it would ignore any further levels like you mention.

$permissions with 'foo.bar.baz' and 'foo.biz.buz' and $matrix 'user' with 'foo.bar.*' and 'foo.biz.buz'

$this->user->addPermission('foo.bar.baz');
$this->user->addPermission('foo.biz.buz');

// Searches for exact match on 'foo.buz' but since it doesn't find, searches for foo.* as you may have full access (*)
$this->user->can('foo.biz'); // False 

// Searches for exact match on 'foo.biz.bus' but since it doesn't find, searches again for foo.* instead of foo.biz.*
$this->user->can('foo.biz.bus'); // False

I came up with a solution for this issue, if you want to test it on your side, you can find it here.

After the patch, it should be fine.

$permissions with 'foo.bar.baz' and 'foo.biz.buz' and $matrix 'user' with 'foo.bar.*' and 'foo.biz.buz'

$this->user->addPermission('foo.bar.baz');
$this->user->addPermission('foo.biz.buz');

$group->can('foo.bar') // false, searches foo.bar, then foo.*
$group->can('foo.bar.baz') // true, searches foo.bar.baz, then foo.bar.*
$group->can('foo.bar.*') // true, searches foo.bar.*, then foo.bar.*

$group->can('foo.biz.buz') // true, searches foo.bar.baz, then foo.bar.*
$group->can('foo.bar') // false, searches foo.bar, then foo.*
$group->can('foo.biz.*') // false, searches foo.biz.*
$group->can('foo.bar.buz') // true, searches foo.bar.buz

Keep in mind that can() only matches exact permission or the parent level with full access (), say that you add test.permissions.item1, test.permissions.item2, test.permissions.item3, checking `$user.can('test.permissions.')` will fail.

bgeneto commented 5 days ago

I came up with a solution for this issue, if you want to test it on your side, you can find it here.

Hello @CosDiabos
I've tried your proposed solution but it fails in some cases. Let's consider the following settings in AuthGroups.php:

    public array $permissions = [
        'perm.lvl1a'          => '',
        'perm.lvl1a.p1'       => '',
        'perm.lvl1a.p2'       => '',
        'perm.lvl1b.p1'       => '',
        'perm.lvl1b.p2'       => '',
        'perm.lvl1c.lvl2a'    => '',
        'perm.lvl1c.lvl2b'    => '',
        'perm.lvl1c.lvl2a.p1' => '',
        'perm.lvl1c.lvl2a.p2' => '',
        'perm.lvl1c.lvl2b.p3' => '',
    ];
    ...
    public array $matrix = [
        'user' => [
            'perm.lvl1c.*',
        ],
    ];

Now the following controller (for authenticated users):

<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function authorizationTest(string $perm): string
    {
        return auth()->user()->can($perm) ? "You have permission $perm" : "You do <b>NOT</b> have permission $perm";
    }
}

Will return:

You have permission perm.lvl1c.lvl2b (OK)
You do NOT have permission perm.lvl1c.lvl2b.p3 (not OK)
You do NOT have permission perm.lvl1c.lvl2b.p2 (not OK)
You do NOT have permission perm.lvl1c.lvl2b.p1 (not OK)

Also, setting

'user' => [
            'perm.*',
        ],

gives, (one example only):

You do NOT have permission perm.lvl1a.p1 (not OK)

CosDiabos commented 4 days ago

Hello!

Yes, my initial patch didn't took in consideration those higher permissions like 'perm.*', I failed to mention that.

But it seems like you solved it yourself, hehe. I just updated my prev patch to handle all these type of cases and it's a basically the same approach as you had.

codeigniter4 / shield