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

Possible issue involving permissions of models with composite name #72

Closed
deniscsz opened this issue Jul 20, 2022 · 7 comments
Closed

Comments

@deniscsz
Copy link

deniscsz commented Jul 20, 2022

I'm facing an issue involving a mismatch with the name of permissions for models with composite name.

My model is named as IpAddress. After ran install command the permissions were created in the database as:

  • create_ipaddress
  • delete_any_ipaddress
  • delete_ipaddress
  • export_ipaddress
  • update_ipaddress
  • view_any_ipaddress
  • view_ipaddress

Also in the IpAddressPolicy class, the permissions are exactly the same and it is working for super admin at this point.

The problem starts after create a new role with low access and try add permissions for this resource. When we need change the role, the form of the card in edit page has the wrong name in the fields.

Screen Shot 2022-07-20 at 10 07 12

After enable the toogle and save it, I'm able to see permissions duplicated in the database (table permissions): there are two records for each permission, one with ipaddress and other with ip_address. The permissions in DB are like this list:

  • create_ipaddress
  • delete_any_ipaddress
  • delete_ipaddress
  • export_ipaddress
  • update_ipaddress
  • view_any_ipaddress
  • view_ipaddress
  • create_ip_address
  • delete_any_ip_address
  • delete_ip_address
  • export_ip_address
  • update_ip_address
  • view_any_ip_address
  • view_ip_address

After this point all the users (even super_admin) are not able to access the resource, no matter what you try in the settings or in role configuration.

The others resource for Models with regular names, like the model Banner from the screenshot, are still working perfectly.

@deniscsz
Copy link
Author

deniscsz commented Jul 20, 2022

I was able to do a dirty fix just removing the _ from $entity in RoleResource to make the fields of the form match with the permissions generated in DB and Policies, but I don't think it is the right approach for the case.

Let me know if a PR using this approach is welcome and I can create one (deniscsz@0c8a72b).

@coolsam726
Copy link
Contributor

I have experienced the same issue. My quick fix was to find&replace the composite name and replace it with the snake case equivalent. In my case, from projectcategory to project_category. The permissions work, but in the settings the Check All toggle input still remains off as shown below.
image
This would mean every time I generate permissions and policies I will have to modify my policy again. A fix to this will be highly appreciated.

@bezhanSalleh
Copy link
Owner

@deniscsz this issue has been resolved in the draft PR #69 which also takes care of models inside directories and such plus much more...

@deniscsz
Copy link
Author

@coolsam726 I created a PR, take a look if you have interest to use it: #74.

My fix is different of your, I kept the permissions as generated, like projectcategory. I just change some pieces of the shield's form to make it matches. Check all works too in this case.

@deniscsz
Copy link
Author

@bezhanSalleh Ok, thanks for let me know.

@coolsam726
Copy link
Contributor

@bezhanSalleh, thanks for responding to this. Could we leave the issue open until PR #69 is merged then? Or is the fix already released?

@bezhanSalleh
Copy link
Owner

bezhanSalleh commented Jul 22, 2022

@coolsam726 I created a PR, take a look if you have interest to use it: #74.

My fix is different of your, I kept the permissions as generated, like projectcategory. I just change some pieces of the shield's form to make it matches. Check all works too in this case.

i will definitely review it once more. But bear in mind it's not just the composite resource names but also how resources are structured... i.e. in the demo there is Blog and Shop and they both have CategoryResource... so the draft PR also takes care of that situation as well during policy and permission generation...
Screen Shot 2022-07-21 at 3 41 06 PM

@coolsam726 no need it's almost done... will be out in a day or two 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants