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

Object-level ACL's #141

Closed
jesperbruunhansen opened this issue Apr 13, 2017 · 17 comments
Closed

Object-level ACL's #141

jesperbruunhansen opened this issue Apr 13, 2017 · 17 comments

Comments

@jesperbruunhansen
Copy link

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
Copy link

+1

@bajtos
Copy link
Member

bajtos commented Apr 19, 2017

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
Copy link
Author

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
Copy link
Contributor

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

@kesavkolla
Copy link

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
Copy link

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
Copy link
Contributor

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
Copy link

RomanovRoman commented Jul 1, 2017

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:

  • the same JSON with 2xx status
  • some response without any entities (4xx, 3xx and so on)

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

@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
Copy link

@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
Copy link

stale bot commented Mar 5, 2018

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 stale bot added the stale label Mar 5, 2018
@virkt25
Copy link
Contributor

virkt25 commented Mar 5, 2018

not stale. post-MVP

@stale stale bot removed the stale label Mar 5, 2018
@dhmlau dhmlau added non-DP3 and removed Core-GA labels Mar 27, 2018
@dhmlau
Copy link
Member

dhmlau commented Mar 27, 2018

This involves ACL at the controller level and model instance level.
The ACL at the controller level will be covered by this epic: #1035

@stale
Copy link

stale bot commented Oct 28, 2019

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 stale bot added the stale label Oct 28, 2019
@stale
Copy link

stale bot commented Nov 27, 2019

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.

@stale stale bot closed this as completed Nov 27, 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

9 participants