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

How $owner is checked is too loose #2933

Closed
1 of 2 tasks
cfjedimaster opened this issue Nov 10, 2016 · 4 comments
Closed
1 of 2 tasks

How $owner is checked is too loose #2933

cfjedimaster opened this issue Nov 10, 2016 · 4 comments
Assignees

Comments

@cfjedimaster
Copy link

cfjedimaster commented Nov 10, 2016

Bug or feature request

  • Bug
  • Feature request

Description of feature (or steps to reproduce if bug)

When you define an ACL related to $owner, LB uses the following rules to determine if you are the owner:

  1. a relation to a model that extends user. 2) a simple property called userId. 3) a simple property called owner

However, imagine a model called cat. It has a relation called creator that points to a User. It also has a relation called "personIPukeOn", that also points to a User. In this scenario, if a cat pukes on me, but I'm not the owner, LB will still I'm the owner because the relationship points to a User model.

Link to sample repo to reproduce issue (if bug)

Expected result

Actual result (if bug)

Additional information (Node.js version, LoopBack version, etc)

@bajtos
Copy link
Member

bajtos commented Apr 5, 2017

@ebarault could you PTAL? Has this scenario been already addressed, or is it waiting for #3140 to get landed?

@bajtos bajtos self-assigned this Apr 5, 2017
@bajtos bajtos added the triaging label Apr 5, 2017
@ebarault
Copy link
Contributor

ebarault commented Apr 5, 2017

basically the situation described here implies 2 different User models, at a time where authentication with multiple user based models was not supported, in such a configuration at that time, yes the scenario could happen, but the setup was not correct.

in #3140 the situation will be solved OOTB provided that multiple user models are configured, and with a feature flag for setups with only one user model.

so yes, it will be fixed when #3140 is landed

@bajtos
Copy link
Member

bajtos commented Apr 28, 2017

Closing as a duplicate of #3081 then.

@bajtos bajtos closed this as completed Apr 28, 2017
@bajtos bajtos removed the triaging label Apr 28, 2017
@cfjedimaster
Copy link
Author

I missed the earlier comment from @ebarault. I want to be clear that in my scenario, there was one User model. There were 2 uses of a property that pointed to an instance of User, but only one User model.

Does that change things or is this still a duplicate?

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

No branches or pull requests

3 participants