Loopback: ACL for models extending User Model is not working

[jannyHou]

1. slc loopback to create a new lb app
2. slc loopback:model to create a model extends User model
3. node . and open explorer
4. Try endpoint Post/ myusers/ :

Result:

{
  "username": "string",
  "email": "[email protected]",
  "id": 1
}

Try endpoint Get/ myusers/ :
Result:

[
  {
    "username": "string",
    "email": "[email protected]",
    "id": 1
  }
]

By default, people should not be able to Get /myusers without login.

[bajtos]

I think ACLs are not inherited when extending a model. See #157 for more details.

[bajtos]

I think there is one catch: ACL is represented as a list of access-control entries and AFAIK we don't have a declarative way for clearing this list from model JSON file. This means that when a child model inherits ACL from the parent, it is not possible to drop parent's ACL and build a new ACL from scratch.

We should find a solution for that while working on the fix for this issue.

Since this was discovered when extending a built-in model, I think it relates to #397 A better way for customizing built-in models and models from components

[cfjedimaster]

@bajtos Are you saying this is a bug _everywhere_, or that it wasn't intended to work in the first place?

To your second point - what if the logic was this: Given Child extends Parent - if my child.json file has _no_ acls property, then it inherits. If it has "acls":[], that means "I'm not inheriting Parent, I'm setting acls to an empty array."

That may not be backwards compat though.

[sahilanguralla]

I am in middle of something and I need it. please fix it ASAP. Also please suggest any workaround for the time being, if possible. Thanks in advance.

[bajtos]

Are you saying this is a bug everywhere, or that it wasn't intended to work in the first place?

I believe that we have never implemented inheritance of ACL. I don't think it was an intentional decision, just the result of the way how we implemented the feature.

To your second point - what if the logic was this: Given Child extends Parent - if my child.json file has no acls property, then it inherits. If it has "acls":[], that means "I'm not inheriting Parent, I'm setting acls to an empty array."

I think the typical scenario would be to built a custom ACL in the JSON file, which would not be supported if you had to specify "acls":[].

Here are few other ideas of what can be done:

1) When "acls" is not defined, or set to a special value like "inherit" (notice it's a string, not an array as usually), the model will inherit "acls" from the parent. If "acls" is set, parent ACL will be discarded and the ACL provided in the child model will be used instead.

// inherits from parent
{
  "base": "User",
  "acls": "inherit" 
}

// defines custom ACL
{
  "base": "User",
  "acls": [
    {
      "principalType": "ROLE",
      "principalId": "$everyone",
      "permission": "DENY"
    },
    {
      "principalType": "ROLE",
      "principalId": "admin",
      "permission": "ALLOW",
    }
  ]
}

2) Another approach is to create a special ACL entry to clear the inherited list:

// defines custom ACL
{
  "base": "User",
  "acls": [
    "reset",
    {
      "principalType": "ROLE",
      "principalId": "$everyone",
      "permission": "DENY"
    },
    {
      "principalType": "ROLE",
      "principalId": "admin",
      "permission": "ALLOW",
    }
  ]
}

3) I think the cleanest option is to add a model setting specifying whether parent ACL should be inherited:

{
  "base": "User",
  "inheritAcls": false, // true by default
  "acls": []
}

The last option looks best to me.

@raymondfeng @ritch @superkhau thoughts?

[bajtos]

Also if we add a setting inheritAcls, then we can implement it in 2.x and modify model generator to set the flag to true. That way we get backwards compatibility for existing codebase and still have the benefit of doing things the right way for newly created models.

[bajtos]

@sahilanguralla

I am in middle of something and I need it. please fix it ASAP.

If you can contribute a pull request fixing the issue then I am happy to help you along the way.

Also please suggest any workaround for the time being, if possible. Thanks in advance.

Copy the acls entry from LoopBack's common/models/user.json into your newly created model.

[cfjedimaster]

Personally, it feels wrong to me that some things inherit automatically and
some would not (ACLs).

"I think the typical scenario would be to built a custom ACL in the JSON
file, which would not be supported if you had to specify "acls":[]."

I'm not quite sure what you mean here. If I'm building Child(extends
Parent) and I want to specify custom acls, then I'd type them at the
property. If I wanted to specify custom acls and specifically _no_ ACLs,
then [] implies an empty array.

If I leave the property off, my assumption would be that it would inherit
it from Parent.

I don't like the "reset" idea because it wouldn't be consistent. Like, as a
user, I may see that and assume I could apply it to properties too.

On Tue, May 3, 2016 at 2:31 AM, Miroslav Bajtoš [email protected]
wrote:

@sahilanguralla https://github.com/sahilanguralla

I am in middle of something and I need it. please fix it ASAP.

If you can contribute a pull request fixing the issue then I am happy to
help you along the way.

Also please suggest any workaround for the time being, if possible. Thanks
in advance.

Copy the acls entry from LoopBack's common/models/user.json
https://github.com/strongloop/loopback/blob/6b40c69bb3bebbf1b7651b881f6f8b139922112a/common/models/user.json#L36-L91
into your newly created model.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/strongloop/loopback/issues/2270#issuecomment-216458231

Raymond Camden, Developer Advocate for StrongLoop at IBM

Email : [email protected]
Blog : www.raymondcamden.com
Twitter: raymondcamden

[arma-gast]

What do you think about giving most general rules from child ACLs higher weight than most specific rules from parent? It will allow developer to deny access to all inherited methods with one rule and allow access with more specific ones. On the other hand, if developer applies deny all to all rule in the child model, he will need to copy rules from parent model. It is not DRY. But, at last, with general denial rule in child model developer will be sure, that no new methods will be accessible if parent model extended.

[cfjedimaster]

I'm honestly having a hard time wrapping my head around it - can you create a demo? I don't mean a working demo, just the JSON files.

[arma-gast]

Say, I define a model Person, which inherits User:

{
  "name": "Person",
  "plural": "people",
  "base": "User",
  "properties": {},
  "validations": [],
  "relations": {},
  "acls": [
    {
      "principalType": "ROLE",
      "principalId": "$everyone",
      "permission": "DENY"
    }
  ],
  "methods": {}
}

I get the following log when try POST /people/login:

  loopback:security:acl The following ACLs were searched:  +2ms
  loopback:security:acl ---ACL--- +0ms
  loopback:security:acl model Person +1ms
  loopback:security:acl property login +0ms
  loopback:security:acl principalType ROLE +0ms
  loopback:security:acl principalId $everyone +0ms
  loopback:security:acl accessType * +0ms
  loopback:security:acl permission ALLOW +0ms
  loopback:security:acl with score: +0ms 8004
  loopback:security:acl ---ACL--- +0ms
  loopback:security:acl model Person +0ms
  loopback:security:acl property * +0ms
  loopback:security:acl principalType ROLE +0ms
  loopback:security:acl principalId $everyone +0ms
  loopback:security:acl accessType * +0ms
  loopback:security:acl permission DENY +0ms
  loopback:security:acl with score: +0ms 7495
  loopback:security:acl ---ACL--- +0ms
  loopback:security:acl model Person +0ms
  loopback:security:acl property * +1ms
  loopback:security:acl principalType ROLE +0ms
  loopback:security:acl principalId $everyone +0ms
  loopback:security:acl accessType * +0ms
  loopback:security:acl permission DENY +0ms
  loopback:security:acl with score: +0ms 7495
  loopback:security:acl ---Resolved--- +0ms
  loopback:security:access-context ---AccessRequest--- +0ms
  loopback:security:access-context  model Person +0ms
  loopback:security:access-context  property login +0ms
  loopback:security:access-context  accessType EXECUTE +0ms
  loopback:security:access-context  permission ALLOW +0ms
  loopback:security:access-context  isWildcard() false +0ms
  loopback:security:access-context  isAllowed() true +0ms

Correct me if I am wrong.

We see 3 rules: one from ACL of model Person (general denial) and two inherited from User (general denial and allowing method /login).
The allowing rule has higher weight, because it is more specific. Even it is inherited.
I think, rules from model itself must have higher weight than inherited ones.

For now, there is no way I can deny access to methods that will be added to model User in the future.

[cfjedimaster]

So given that, yes, I would have expected the ACL in Person to block everything.

[mora260]

Hi everyone! Right now I'm facing a problem similar to yours, but not the same thing. I'm creating a Model extending from the built-in User model of loopback, and the ACL i actually being inherited completely. But in all the forums I've read, the problem seems to be the other way around.

I don't know what is going on...

[jannyHou]

@mora260 and anyone runs into the same issue, we fixed acl problem in pr https://github.com/strongloop/loopback-workspace/commit/bd6c93c03eadc168e5c55308aee2883848d18e6a

And it's released in [email protected]
Please update loobpack-workspace in your genrator-loopback, or a more quick way, reinstall your strongloop to get it work.

I am glad to help if you still have problem.

[Sloff]

Hi @jannyHou, I'm experiencing the same problem as @mora260. My model extending the user model is inheriting all of the acls.

My strongloop is the latest version

[jannyHou]

@Sloff @mora260 Could you explain more on what problem you are having?
Let's see now we have a custom user MyUser that based on User model, what problems do you have?
Thanks!

[jannyHou]

I am closing this issue since I believe the original bug is fixed.
Open a new one to keep track of new bug:https://github.com/strongloop/loopback/issues/2664

Let's keep discussion under the new one.

[dleute]

The original problem appeared to be that models inheriting from User did not inherit ACL's so they had to be copied over.

Now the problem seems to be the ACL's on User are not respecting overrides from the child. If I put a blanket deny all rule on the child model, the specific ACL's from the parent are still overriding.

In my case, the easiest fix is to copy out the source ACL's from the parent (which was a solution to the original problem), paste them into the child, remove any I don't need to change (inheritance seems to work now) and change the ones I need to override with my child models specs.

So, is the intended behavior that parent ACL's are now inherited and that because of the granularity of the rule, may overrule general rules in child models? Is there any way to clear the parent rules and use only the child rules?

Thanks!

[maziarz]

I am experiencing this problem even in version 3.6.0. Is it really not fixed yet?

[ebarault]

hi, i believe this is will be fixed in https://github.com/strongloop/loopback-datasource-juggler/pull/1289

[maziarz]

@ebarault Super awesome! What is the temporary solution for this issue?

[ebarault]

there's currently no clean one i'm afraid, the best i see is to create you own User base model, reusing the code from the built-in model and removing the incriminated ACLs from this custom User base model

[maziarz]

@ebarault I do not want to point fingers, but i just came from Django REST framework and getting a bit cold feet, especially since LB encourages extending the base model to create a custom User model, it does not seem to extend the ACL methodology quite well. I am just crossing my fingers that i do not fall over more weird design defects. Sorry if i sound rude, it was not intentional.

[ebarault]

@maziarz: no offense taken. I'm a community maintainer, doing my best to sort some problems out, one at a time. Community is doing a great job here so does the Strongloop team (now joined by IBM). The framework is nearly extensible to the infinite and everyone expects a tone from it OOTB, which is fine :-) but it just take some time and/or your joined efforts to contribute in fixing the issues that matter most to you.

[maziarz]

@ebarault you are doing a good job as far as i can see! I will eat the default ACL's for now until juggler gets released with new shoes :-)

[kodepareek]

A new version of Datasource juggler seems to have been published 18 hours ago. But doesn't look like it fixed the issue.

Yup the PR above has not been merged yet.

[rashmihunt]

Fix is merged through this PR https://github.com/strongloop/loopback-datasource-juggler/pull/1289 Closing this issue.

[cfjedimaster]

So to be clear, the fix is that all ACLS for models extending others should work now? I just want to be clear on what/how it was corrected.

[rashmihunt]

@bajtos @raymondfeng - since you have reviewed the community PR strongloop/loopback-datasource-juggler#1289, can you please answer @cfjedimaster 's above question?

[bajtos]

I re-read the discussion in strongloop/loopback-datasource-juggler#1289 to refresh my memory. I believe that when a child model is created, ACLs defined by the parent model are merged with ACLs defined by the child model in such way that child model ACL entries take precedence. So yes, all ACLs for models extending others should work now.

[pbourdu]

@bajtos - I'am implementing this case in my project, child model ACL entries have no effects, only parent's ACL are applied. I'm using loopback 3.22.2.

"loopback": "^3.22.2", "loopback-boot": "^2.27.1", "loopback-component-explorer": "^4.3.1", "loopback-component-logger": "^1.0.0", "loopback-connector-mongodb": "^3.6.0",

[bajtos]

@pbourdu I see. Please create a small app reproducing the problem (see Reporting bugs), open a new issue and then post a link to that new issue here.

[eyoma]

@bajtos - I'am implementing this case in my project, child model ACL entries have no effects, only parent's ACL are applied. I'm using loopback 3.22.2.

"loopback": "^3.22.2", "loopback-boot": "^2.27.1", "loopback-component-explorer": "^4.3.1", "loopback-component-logger": "^1.0.0", "loopback-connector-mongodb": "^3.6.0",

Same issue:
STEPS:
Create a new loopback app
Create new Model e.g "Appuser" using "User" as base model
Set ACLs on "Appuser" to grant everyone access
=> ACLs for the child model has not effect