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

feat: GetAllObjects has wrong result in RBAC with domains model #1411

Merged

Conversation

truc0
Copy link
Contributor

@truc0 truc0 commented Jun 12, 2024

Fix: #1409

Short description

  • add GetValuesForFieldInPolicyAllTypesByName function to adynamically get fieldIndex for each ptype
  • add test for getting objects (subjects and actions are also included) in RBAC with domains mode

Details

The root cause

GetAll{things} and GetAllNamed{things} functions (things can be actions, subjects, roles) use hard-coded fieldIndex while fetching the corresponding resources, which causes incorrect behavior when the field indexes change.

According to the documentation, the RBAC mode with domains have the convenient putting domain as the second param of a policy, while the subject is located at the second param (fieldIndex=1) in other mode.

Note: Conventionally, the domain token name in policy definition is dom and is placed as the second token (sub, dom, obj, act).

Solution of this PR

To make casbin works correctly, the fieldIndex of "things"(actions, subjects, roles) should be determined from the config file (instead of hard-coded).

For GetAllNamed{things} functions, the Model.get_field_index method is used for getting correct index from config.

For GetAll{things}, the fieldIndex may be different in different ptype. Thus, a new function GetValuesForFieldInPolicyAllTypesByName is defined to find fieldIndex in different ptype by field's name (e.g. obj, act).

What's more

Changes to documentation

Now casbin can detect the position of dom and act correctly with no extra configuration needed. Setting the field index of dom using SetFieldIndex API explicitly is no longer necessary. The document may need to be changed.

Similar situation

Now GetAllRoles and GetAllNamedRoles is still using hard-coded field index (but it won't fail in RBAC with domains mode), this can be optimized using similar API.

Close #1409

- add GetValuesForFieldInPolicyAllTypesByName function to adynamically get fieldIndex for each ptype
- add test for getting objects (subjects and actions are also included) in RBAC with domains mode
@casbin-bot
Copy link
Member

@tangyang9464 @JalinWang please review

@hsluoyz hsluoyz changed the title fix: GetAllObjects has wrong result in RBAC with domains mode feat: GetAllObjects has wrong result in RBAC with domains mode Jun 12, 2024
@hsluoyz hsluoyz changed the title feat: GetAllObjects has wrong result in RBAC with domains mode feat: GetAllObjects has wrong result in RBAC with domains model Jun 12, 2024
@hsluoyz hsluoyz merged commit 963e1c4 into casbin:master Jun 12, 2024
11 checks passed
Copy link

🎉 This PR is included in version 2.96.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

RBAC with Domain model ,get_all_subjects error
3 participants