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

Permissions vs. REST find/list - not applied? #343

Closed
fabien opened this issue Jun 22, 2014 · 10 comments
Closed

Permissions vs. REST find/list - not applied? #343

fabien opened this issue Jun 22, 2014 · 10 comments

Comments

@fabien
Copy link
Contributor

fabien commented Jun 22, 2014

As far as I can see enableAuth / Model.checkAccess doesn't prevent an instance from being listed. In other words, (array) output is not filtered based on the current permissions, and thus exposes data without restrictions.

Of course there will be quite some performance impact, performing (async) checkAccess calls on the results to be returned, but Loopback should be secure by default. One should be able to at least enable/disable this kind of filtering easily.

In general it should be possible to quickly do a security audit and see all the exposed resources at a glance - currently it's easy to miss any unintentionally exposed api endpoints, as it's not clear what the Loopback default behavior actually is.

@karlmikko
Copy link

@fabien As there may be multiple ACL and these ACL can be custom, and multiple ACL could resolve true for any given instance, it will be very difficult to place a default behaviour for find/list.

The current way loopback handles this is by using hasMany. So Users would haveMany books, then you ACL deny GET /books for everyone. Then you can call /users/[id]/books to get all the books for that user. Then you can edit that book with $owner ACL PUT /books/[id].

@fabien
Copy link
Contributor Author

fabien commented Jun 22, 2014

@karlmikko makes sense, thanks. I guess it's also possible to use an 'after remote' hook and perform the filtering - at the cost of performance perhaps?

@karlmikko
Copy link

@fabien I have been using Model.beforeRemote('**', function(ctx, instance, done){ to change a scope. That way you can look at the ctx see who is logged in, do some role checks, then alter the filter/where (find/count) to change what is visible. This way you get no performance hit as the db will do all the work, with good indexes this will be very fast.

@fabien
Copy link
Contributor Author

fabien commented Jun 22, 2014

@karlmikko that sounds useful too, indeed. Loopback is quite flexible in that regard. Although I would have expected that the default behavior would just apply whatever ACL is defined for findById() to also apply to find() (listing).

@karlmikko
Copy link

@fabien I expected the behaviour to be the same also. But when you look at the code closer you see that isn't really possible very easily and the hasMany seems to work well. Maybe in the future they may be able to implement this.

@raymondfeng
Copy link
Member

Moving forward, I think we need to have a balanced solution for ACLs:

  1. The declarative ACLs to check requests against model APIs
  2. Developers should be able to hook some code to a remote model and/or its methods to further enforce more checks before/after the DB call

@raymondfeng raymondfeng added this to the 2.0.0 milestone Jul 10, 2014
@fabien
Copy link
Contributor Author

fabien commented Jul 11, 2014

@raymondfeng +1 for the hooks, and possibly a way to easily merge filter/where in a beforeRemote call in a way that the find* related methods pick up the added scope. So in essence, some concept of chained scopes would be ideal, and apparently, it's already implemented in Juggler somehow:

      // Define scope-chaining, such as
      // Station.scope('active', {where: {isActive: true}});
      // Station.scope('subway', {where: {isUndeground: true}});
      // Station.active.subway(cb);

It just needs a runtime scope as part of the remoting code.

@fabien
Copy link
Contributor Author

fabien commented Jul 11, 2014

@karlmikko see: loopbackio/loopback-datasource-juggler#172

I am using this as a solution based on your beforeRemote technique - a combination of a hasMany and beforeRemote handler to further scope down if needed.

It's not just meant for security scoping though, and should allow dynamic scoping in general. Thoughts are welcome.

@fabien
Copy link
Contributor Author

fabien commented Aug 23, 2014

I posted some more info on how to tackle this here: #337

Basically, I'm using NodeJS domains and scoping at the MongoDB connector-level to ensure that all interactions are scoped accordingly. This might make its way into core somehow (generalized into loopback/juggler), so any input is welcome.

From my testing sofar this seems very flexible and easy to integrate into various scenarios, like multi-tenancy.

@bajtos bajtos mentioned this issue Sep 30, 2014
47 tasks
@bajtos bajtos modified the milestone: #Rel lb 2.0.0 Sep 30, 2014
@superkhau superkhau changed the title Security: permissions vs. REST find/list - not applied? Permissions vs. REST find/list - not applied? Feb 4, 2016
@cgole cgole removed the ease-of-use label Jan 7, 2017
@bajtos
Copy link
Member

bajtos commented Nov 12, 2019

LoopBack version 3 is in LTS, we won't be adding any new features.

@bajtos bajtos closed this as completed Nov 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants