Add wildcard permissions

[stefanriehl]

Allows using wildcard permissions based on the default wildcard permission implementation of Apache Shiro.

Enable wildcard permissions in the config file:

// config/permission.php
'enable_wildcard_permission' => true,

When enabled, wildcard permissions offers you a flexible representation for a variety of permission schemes.

Some examples:

// user can only do the actions create, update and view on both resources posts and users
$user->givePermissionTo('posts,users.create,update,view');

// user can do the actions create, update, view on any available resource
$user->givePermissionTo('*.create,update,view');

// user can do any action on posts with ids 1, 4 and 6 
$user->givePermissionTo('posts.*.1,4,6');

See full documentation at

// docs/basic-usage/wildcard-permissions.md

[drbyte]

@stefanriehl This looks great.

Question: all the wildcard lookup logic is agnostic of the user's current guard. Is that intentional?

[stefanriehl]

@stefanriehl This looks great.

Question: all the wildcard lookup logic is agnostic of the user's current guard. Is that intentional?

Good point. You're right. Will change.

[stefanriehl]

@drbyte To be honest, I am not yet completely satisfied with the implementation. Why? Well, let me show an example:

public function it_can_verify_permission_instances_not_assigned_to_user()
{
    $userPermission = Permission::create(['name' => 'posts.*']);
    $permission = Permission::create(['name' => 'posts.create']);

    $user->givePermissionTo([$userPermission]);

    // all these asserts should be true, but are currently false

    $this->assertTrue($user->hasPermissionTo('posts.create'));
    // because permission exists in db and wildcard permission won't be called

    $this->assertTrue($user->hasPermissionTo($permission));
    $this->assertTrue($user->hasPermissionTo($permission->id));
    // because we can only handle strings right now
}

That doesn't seem logical to me! So, I would like to change the implementation:

If wildcard permissions are enabled -> the application can verify permissions by id, instance or string against user permissions (if id does not exist, an exception will be thrown).

I already did the implementation with more tests, but I would like to get your thoughts before updating.

[drbyte]

I would like to change the implementation

Makes sense.

I haven't seen your proposed new approach, but perhaps it makes sense to just be binary about it: either the wildcard switch is on and that's the only matching that's used, or it's off and it defaults to the prior matching logic.

[stefanriehl]

either the wildcard switch is on and that's the only matching that's used, or it's off and it defaults to the prior matching logic.

That's what I did. I will push tomorrow. Thanks!

[drbyte]

@stefanriehl I do like this improvement. Feels more intuitive that the switch is all-or-nothing, not hybrid.

Thoughts about performance? caching needs?

[stefanriehl]

@stefanriehl I do like this improvement. Feels more intuitive that the switch is all-or-nothing, not hybrid.

Thoughts about performance? caching needs?

Well, I did some performance tests. Wildcard permissions vs. DB permissions:

• MBP 16GB
• Local Docker with PHP 7.3.13 / Ubuntu 16.04
• sqlite
• 1 User has n permissions
• we're using permission caching
• the last inserted permission will be checked, so that we can be sure that all wildcard permissions will be tested. That is of course the worst case.

Permissions	DB	Wildcard
10	0.00015	0.0005
25	0.00024	0.00082
50	0.00041	0.0009
100	0.00074	0.00149
250	0.00173	0.0032
500	0.00385	0.00656

Wildcard permission check is ~ 2.2x slower than DB permission. With the restriction that probably the permission to be checked is not the last one in the permission list. In a real scenario the wildcard query surely would be faster. Interesting: the more permissions a user has, the lower the factor.

@drbyte What do you think? Is it necessary to consider caching for wildcard permissions? In my opinion caching would only make sense, if we'll cache the positive/negative checked permissions for a user. But this implementation is more complex I think.

[drbyte]

I agree: it's probably suitable to revisit caching requirements for this feature after feedback from use-in-the-wild, where specific use-cases can be cited, so we fix actual problems instead of over-engineering something that just takes more effort to support and maintain.

[drbyte]

I'm satisfied that the public signature of these changes is minimal.
The tests seem to cover all use-cases that I can think of.

Are you satisfied with the completeness of this? Ready for merge?

[stefanriehl]

@drbyte Yes I'm fine. I'll just correct the spelling mistake.

[drbyte]

Thanks!