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

Non-existent attributes still match in a rule head specializer in Node.JS library #749

Closed
leina05 opened this issue Feb 19, 2021 · 3 comments · Fixed by #1215
Closed

Non-existent attributes still match in a rule head specializer in Node.JS library #749

leina05 opened this issue Feb 19, 2021 · 3 comments · Fixed by #1215
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@leina05
Copy link
Contributor

leina05 commented Feb 19, 2021

A rule like:

allow(actor, action, resource: {attr: attr});

still matches even if the object passed in as resource does not have the attr attribute. This has to do with how the library handles undefined (it passed it back into Polar instead of treating it as a failed match).

@leina05 leina05 added the bug Something isn't working label Feb 19, 2021
@samscott89
Copy link
Member

Is there a distinction in Node between a non-existent attribute and one which is undefined?

e.g.

d = { a: 1 }
d.b
// returns undefined

vs

d = { a: 1, b: undefined }
d.b
// also returns undefined

@dhatch
Copy link
Contributor

dhatch commented Feb 19, 2021

> x = {b: undefined}
{ b: undefined }
> x.hasOwnProperty("b")
true
> x.hasOwnProperty("b")
true
> x.b
undefined
> x.hasOwnProperty("foo")
false
> x.foo
undefined

@dhatch
Copy link
Contributor

dhatch commented Feb 19, 2021

Should be doing a similar check like this: https://github.com/osohq/oso/blob/main/languages/python/oso/polar/query.py#L98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants