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

Eval of custom function with multiple parameters: issue with LoadPolicyLine that splits on comma #234

Closed
slinders1985 opened this issue Jan 18, 2022 · 7 comments · Fixed by #235
Assignees
Labels
enhancement Enhancement the exist feature question Further information is requested released

Comments

@slinders1985
Copy link

slinders1985 commented Jan 18, 2022

Our model:

[request_definition]
r = dom, sub, obj, act

[policy_definition]
p = dom, sub, obj, act, eft

[policy_effect]
e = some(where (p.eft == allow)) && !some(where (p.eft == deny))

[matchers]
m = eval(p.dom) && eval(p.sub) && eval(p.obj) && keyMatch2(r.act, p.act)

The policies are loaded from our db using the EFCore-Adapter.
In one of the eval() we want to evaluate a custom function which takes in more than one parameter. The V1 (sub) of the policy will refer to this custom function e.g. customFunction(r.sub, r.dom, "aString").

The custom function gets successfully added to the enforcer, but will never get to the point of execution because of following issue:
when the LoadPolicyLine in NetCasbin.Persist.Helper gets hit, the line will split on the commas, which also splits the customFunction(r.sub, r.dom, "aString") into pieces. Therefore in the InternalEnforceWithChainEffector the policyValues.Count will always be larger than the context.PolicyTokens.Count and throw the ArgumentException.

When I adjust the customFunction so it only receives one parameter, the customFunction gets called correctly in the Casbin evaluation process.

Could the Helper be adjusted so that custom functions with multiple parameters are not split any longer on the comma? Are is there another solution to this issue that I can't see at the moment?

@casbin-bot
Copy link
Member

@sagilio @xcaptain @huazhikui

@casbin-bot casbin-bot added the question Further information is requested label Jan 19, 2022
@hsluoyz
Copy link
Member

hsluoyz commented Jan 19, 2022

@sagilio @hackerchai

@sagilio
Copy link
Member

sagilio commented Jan 19, 2022

I will add the more load line method to support this, like this: casbin/casbin#887

@pspeybro
Copy link

@sagilio, the tryLoadPolicy pull request that you linked, that should resolve the issue?

@sagilio sagilio added the enhancement Enhancement the exist feature label Jan 20, 2022
@sagilio
Copy link
Member

sagilio commented Jan 20, 2022

@sagilio, the tryLoadPolicy pull request that you linked, that should resolve the issue?

Just wait a moment. also need to update EFCore-Adapter

@github-actions
Copy link

🎉 This issue has been resolved in version 1.12.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@slinders1985
Copy link
Author

Thank you! Works like a charm 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement the exist feature question Further information is requested released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants