Skip to content

Conversation

@hariprasadiit
Copy link
Contributor

Issue:

Model

[request_definition]
r = sub, cid, lid, obj, act

[policy_definition]
p = sub, cid, lid, obj, act, rule, eft

[role_definition]
g = _, _, _

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

[matchers]
m = g(r.sub.id+'::'+r.sub.type, p.sub, r.cid+'::'+r.lid)&& eval(p.rule) && keyMatch(r.cid, p.cid)&& keyMatch(r.lid, p.lid) && regexMatch(r.act, p.act) && keyMatch2(r.obj, p.obj)

Policy

p, employee, *, *, /e/:eid, update, "r.sub.id == keyGet2(r.obj, p.obj, 'eid')"
p, employee, *, *, /e/:eid, get, true
p, admin, *, *, /e/:eid, update, true
p, admin, cid3, lid1, /kiosk/:kid, (get)|(update), true


g, admin, location-admin, *
g, location-admin, receptionist, *
g, receptionist, employee, *
g, billing-admin, employee, *
g, alice, admin, cid3::*

While loading above policy from database, loadPolicyLine from Helper class will parse r.sub.id == keyGet2(r.obj, p.obj, 'eid') into two tokens as it contains delimiter.

We can workaround this by wrapping r.sub.id == keyGet2(r.obj, p.obj, 'eid') with quotes but hasPolicy will fail because provided policy line contains extra quotes but parsed policy line will not have them.

Solution

Wrap all tokens of policy line before joining them and passing policy line to the csv parser

@nodece
Copy link
Contributor

nodece commented Sep 9, 2021

thanks @hariprasadiit, please fix this CI errors.

@hariprasadiit hariprasadiit force-pushed the master branch 2 times, most recently from 41768d9 to 0581af3 Compare September 9, 2021 15:04
@hariprasadiit
Copy link
Contributor Author

@nodece I'm not really sure why coverage and tests are failing. On my system they're passing. Any help would be appreciated

@nodece
Copy link
Contributor

nodece commented Sep 11, 2021

@hariprasadiit hariprasadiit changed the title fix: token parsing issues if token contains delimiter fix: Token parsing issues if token contains delimiter Sep 11, 2021
@hariprasadiit
Copy link
Contributor Author

@nodece fixed CI issues

Copy link
Contributor

@Zxilly Zxilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Zxilly Zxilly merged commit ba1b5ee into node-casbin:master Sep 11, 2021
@github-actions
Copy link

🎉 This PR is included in version 1.4.2 🎉

The release is available on:

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.

3 participants