search

php-casbin / laravel-authz

An authorization library that supports access control models like ACL, RBAC, ABAC in Laravel.
Apache License 2.0
272 stars 46 forks source link

Policy add command incorrectly inserts policy resulting in error #55

Closed timyourivh closed 1 year ago

timyourivh commented 1 year ago

Package version used: casbin/laravel-authz: ^3.1 (v3.1.4 in lockfile)

When trying out this package I tried adding a policy using the policy:add command. As stated in the documentation found in the README.md:

README.md snippet > ### [Using artisan commands](https://github.com/php-casbin/laravel-authz#using-artisan-commands) > You can create a policy from a console with artisan commands. > (...) > To Role:: > ``` > php artisan policy:add writer,articles,edit > ```

I ran command php artisan policy:add admin,posts,index. This resulted in the following record being inserted:

image

Which I assumed was incorrect. It also throws an error when comparing/checking the rule:

image

I manually changed the rule to the way the Enforcer facade adds it:

image

Which worked. So I think this is a bug.

Sidenote:

The command policy:add implies that it adds a policy while Enforcer::addPermissionForUser() (which does the same) implies that it adds a permission which is inconsistent. I suggest renaming one of the two to match the other.

hsluoyz commented 1 year ago

@timyourivh hi, can you make a PR to fix it?

hsluoyz commented 1 year ago

@leeqvip

leeqvip commented 1 year ago

@timyourivh The fourth parameter of this command is the policy string, separated by commas(,). I followed your steps, but it didn't reproduce in my console. Which terminal do you use, is it a character problem?

timyourivh commented 1 year ago

@leeqvip That appears to be it! Interesting, I have a powershell script to add shorthand's but when using the shorthand for php artisan (which is pa for me) in combination with policy:add some how doesn't work:

image

I was already wondering what could be the issue since I couldn't find any issues or bug in the code responsible :p

timyourivh commented 1 year ago

Note:

The docs state that php artisan vendor:publish will "will create a new model config file and a new lauthz config file" but instead will display a list of all publishable assets available and launch a prompt to choose one. I suggest adding the tags in the tutorial resulting in:

php artisan vendor:publish --tag=laravel-lauthz-config --tag=laravel-lauthz-migrations