Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
bgeneto opened this issue Nov 8, 2024 · 5 comments · May be fixed by #1229
Open

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

bgeneto opened this issue Nov 8, 2024 · 5 comments · May be fixed by #1229
Labels
bug Something isn't working

Comments

@bgeneto
Copy link
Contributor

bgeneto commented Nov 8, 2024

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.

@bgeneto bgeneto added the bug Something isn't working label Nov 8, 2024
@bgeneto bgeneto changed the title Bug: authorization using asterisk (*) only works on root context Bug: authorization using asterisk (*) only works one level Nov 8, 2024
@CosDiabos
Copy link

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
Copy link
Contributor Author

bgeneto commented Nov 8, 2024

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
Copy link

CosDiabos commented Nov 9, 2024

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
Copy link
Contributor Author

bgeneto commented Nov 9, 2024

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
Copy link

CosDiabos commented Nov 10, 2024

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.

@bgeneto bgeneto linked a pull request Nov 13, 2024 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants
@bgeneto @CosDiabos and others