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

[Question] How can I improve my model to make enforce() execution faster? #829

Closed
kizjig opened this issue Jul 2, 2021 · 6 comments · Fixed by #801
Closed

[Question] How can I improve my model to make enforce() execution faster? #829

kizjig opened this issue Jul 2, 2021 · 6 comments · Fixed by #801

Comments

@kizjig
Copy link

kizjig commented Jul 2, 2021

Want to prioritize this issue? Try:

issuehunt-to-marktext


What's your scenario? What do you want to achieve?
I want to know if I've made an inefficient model compared to with what would be considered an optimal casbin model given my requirements?

My model and policies are a mix of RBAC with ABAC which come from my project requirements. It includes a superuser/wildcard concept too. It uses the matchingDomainForGFunction=keyMatch4 setting.

I have 4 roles a user can be assigned: role_creator1, role_presenter1, role_superuser1, role_guestviewer1.

I'm finding single enforce() calls to my model and policy are taking 20-30 seconds to execute using casbin v2.31.4 on production server quality hardware when I have ~2200 policy table entries.

  1. Does this performance sound right?
  2. Does it compare similar to what you would expect given my model and policy?
  3. Are there some changes I can make to my model that would give me faster enforce performance?

Your model:

[request_definition]
r = sub, tenant, perm, act, eft

[policy_definition]
p = sub, tenant, perm, act, eft

[role_definition]
g = _, _, _

# A match is effect is allowed if we find some allow and no deny
[policy_effect]
e = some(where (p.eft == allow)) && !some(where (p.eft == deny))

# The matcher syntax g(r.sub, p.sub, r.tenant) will CHECK whether the USER r.sub has a ROLE p.sub IN the tenant r.tenant.
# By specifying the  matchingDomainForGFunction matcher functions as 'keyMatch4' we can support the super user approach.
[matchers]
m = (g(r.sub, p.sub, r.tenant) && keyMatch4(r.tenant, p.tenant) && r.perm == p.perm && regexMatch(r.act, p.act))

Your policy:

p,role_creator1,tenant/{id},tenant,view,allow
p,role_creator1,tenant/{id},dashboard,login,allow
p,role_creator1,tenant/{id},model_upload_max_size,medium,allow
p,role_creator1,tenant/{id},presentation,(^local$)|(^remote$),allow
p,role_creator1,tenant/{id},invite,(^create$)|(^edit$)|(^view$),allow
p,role_creator1,tenant/{id},library,(^create$)|(^edit$)|(^view$)|(^delete$),allow
p,role_creator1,tenant/{id},desktop,login,allow
p,role_presenter1,tenant/{id},tenant,view,allow
p,role_presenter1,tenant/{id},dashboard,login,deny
p,role_presenter1,tenant/{id},desktop,login,allow
p,role_presenter1,tenant/{id},presentation,(^local$)|(^remote$),allow
p,role_presenter1,tenant/{id},invite,(^create$)|(^edit$)|(^view$),allow
p,role_superuser1,tenant/{id},tenant,(^view$)|(^manage$),allow
p,role_superuser1,tenant/{id},dashboard,login,allow
p,role_superuser1,tenant/{id},model_upload_max_size,large,allow
p,role_superuser1,tenant/{id},presentation,(^local$)|(^remote$),allow
p,role_superuser1,tenant/{id},invite,(^create$)|(^delete$)|(^edit$)|(^view$),allow
p,role_superuser1,tenant/{id},library,(^create$)|(^edit$)|(^view$)|(^delete$),allow
p,role_superuser1,tenant/{id},desktop,login,allow
p,role_guestviewer1,tenant/{id},tenant,view,allow
p,role_guestviewer1,tenant/{id},dashboard,login,deny
p,role_guestviewer1,tenant/{id},desktop,login,deny
p,3914,tenant/8,tenant,billing_contact,allow
p,3914,tenant/8,tenant,manage,allow

g,3332,role_superuser1,tenant/*
g,3315,role_creator1,tenant/8
g,3914,role_creator1,tenant/8

... + ~2200 grouping policy entries for various user ids and tenant ids using the above `role_` prefixed policies.

Your request(s):

3914, tenant/8, dashboard, login, allow ---> true (expected: true)
3332, tenant/2, dashboard, login, allow ---> true (expected: true)
3914, tenant/2, dashboard, login, allow ---> false (expected: false)
@kizjig
Copy link
Author

kizjig commented Jul 6, 2021

I've investigated this question a little bit further and found that using the matchingDomainForGFunction with KeyMatch4 was very slow on my large dataset. And that I could achieve what I was after using the much simpler KeyMatch function instead. Enforce call duration has dropped from ~20 seconds down to ~19 milliseconds with this change.

If I didn't have my super user/domain wildcard requirement, I could achieve the model design without using a domain matching function at all.

With all this said, I'd still be interested to know if there's other improvements I should consider.

@hsluoyz
Copy link
Member

hsluoyz commented Jul 6, 2021

@closetool @tangyang9464

@kilosonc
Copy link
Contributor

kilosonc commented Jul 6, 2021

@kizjig You may want to check this pr #801
There two main reason slow donw your enforce() execution.

  1. AddLink has time complexity of O(N^2).
  2. KeyMatch4 uses regex, and it needs to compile in every execution.

@hsluoyz
Copy link
Member

hsluoyz commented Jul 7, 2021

You may want to check this pr.

@closetool which PR?

@sagilio
Copy link
Member

sagilio commented Jul 7, 2021

@kizjig #801 has reduced the code logic add match cache to improve the performance. Can you have a try on this version?

In fact, I think the 20s is too long when about 2200 grouping policy, can you provide a complete policy sample for our test.

@github-actions
Copy link

github-actions bot commented Jul 8, 2021

🎉 This issue has been resolved in version 2.31.10 🎉

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants