search

loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.96k stars 1.07k forks source link

Object-level ACL's #141

Closed jesperbruunhansen closed 4 years ago

jesperbruunhansen commented 7 years ago

I would like to see an even more advanced ACL implementation in Loopback.next, that supports ACL's on a object-level.

LB v.2/3 has great ways of applying ACL's to an entire model and it's methods, but in many use-cases I want objects/instances only to be accessible to specific users, which has been explicitly granted permission by the client upon creation (pending issue at Loopback repo here).

When the client requests an authenticated GET request, the response will only contain objects that the client has permission to read.

I've made my own mixin to add this feature, which basically manipulates the query at every 'access' event of the datasource.

I'd be happy to help implement this feature into native Loopback.next, should it be considered beneficial for the framework.

jcurlier commented 7 years ago

+1

bajtos commented 7 years ago

Hi @jesperbruunhansen, thank you for opening this issue. Object/instance-level ACLs make a lot of sense to me. I think this is one of the reasons we decided for a complete rewrite in loopback.next - adding generic object/instance-level ACLs was simply too difficult in the old codebase.

@ritch @raymondfeng @superkhau what's your opinion?

I'd be happy to help implement this feature into native Loopback.next, should it be considered beneficial for the framework.

Offer accepted :)

Right now, we are trying to figure out how to design LoopBack.next in such way that ACL checks like the one you are described can be easily contributed by components/plugins, it's probably too early to start the actual work on implementing such component.

jesperbruunhansen commented 7 years ago

Hi @bajtos, and thank you for your explanation.

it's probably too early to start the actual work on implementing such component.

Sure of course, let me know when the time is right and I'll see where I can help!

superkhau commented 7 years ago

@bajtos 100% agree with Object/instance ACLs and even model property ACLs.

kesavkolla commented 7 years ago

If you're doing ACL support at object level then extend it to field level also. Similar to Salesforce data model where users have ability to define roles to field levels too.

RomanovRoman commented 7 years ago

I think about it in several projects. And... IMHO it is a really bad idea for RESTfull api. Because we must have fast token-based ACL mechanics for endpoints. May be we needs in more featured dynamic-roles and nestRemoting? I think we should to consider several concrete real-world use cases in REST and GraphQL in this discussion.

superkhau commented 7 years ago

I think we should to consider several concrete real-world use cases in REST and GraphQL in this discussion.

+1 for more real-world use cases in both REST and GraphQL. I would like to hear more about why it is really a bad idea for RESTful APIs.

RomanovRoman commented 7 years ago

I would like to hear more about why it is really a bad idea for RESTful APIs.

Every URI like: http://domain.tld/api/entity represents concrete collection of concrete entities and users should have from GET request:

It is about proxy and cache. For example, browser wants to GET http://domain.tld/api/entity then nginx-proxy can ping our api server with HEAD http://domain.tld/api/entity request and send response to browser from disk cache without rendering new JSON by api server. That's why we should use nestRemoting and make endpoints like http://domain.tld/api/user/id/entity and http://domain.tld/api/group/gid/member/mid/entity

On the other hand, we needs in javascripted read/write permissions on client with minimal DB requests. IMHO it is not good practice to send unnecessary (unwanted) requests which will be rejected by api server. Full ACL-set request will generate unwanted http-traffic.

PS Javascript is pretty language which makes possible to use several programming paradigms in the same time and place (file). But in APIs (REST, DB, node-modules, etc) we deal with entities therefore we must thinking in OOP-style only. It makes architecture more clear and understandable by human beings.

raymondfeng commented 7 years ago

ACL should be enforced at different tiers based on how much knowledge we acquire about the request (who wants to do what). For APIs, I see the following:

  1. API gateway with oAuth2 to protect endpoints against scopes
  2. Route based ACL - See https://github.com/strongloop-community/loopback-acl-route
  3. Implementation level ACL for controller/repository
  4. Backend (downstream) ACL
jesperbruunhansen commented 7 years ago

Hi guys, and thanks for nice discussion regarding this issue.

+1 for more real-world use cases in both REST and GraphQL

My inspiration and need for this feature comes from the time when I worked on a project, using the Parse-server . When they open-sourced, I basically copied what they did here in order to make the same functionality in Loopback.

Their overall approach to ACL's can be seen here, whereas I like to compare Loopbacks current ACL implementation as the "Class-level" ACL's of Parse.

I like the idea of even more granularity with field-level ACL's as well, as @superkhau and @kesavkolla mentioned.

raymondfeng commented 7 years ago

@jesperbruunhansen I'm with you. Coincidentally, I have been thinking about the possibility to mix conditions into the filter to enforce property level ACLs.

RomanovRoman commented 7 years ago

@raymondfeng instead of property level ACLs... It would be nice to introduce Data Transfer Objects for each triplet(method, resource, role). Then DTOs will define full fieldset for each remote method and will aggregate serialization, parse(deserialization), validation logic which is the same for both client and server.

stale[bot] commented 6 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

virkt25 commented 6 years ago

not stale. post-MVP

dhmlau commented 6 years ago

This involves ACL at the controller level and model instance level.
The ACL at the controller level will be covered by this epic: https://github.com/strongloop/loopback-next/issues/1035

stale[bot] commented 5 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

stale[bot] commented 4 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.