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

findOne ignores $owner dynamic role #238

Closed
Cadrach opened this issue Apr 18, 2014 · 12 comments
Closed

findOne ignores $owner dynamic role #238

Cadrach opened this issue Apr 18, 2014 · 12 comments

Comments

@Cadrach
Copy link

Cadrach commented Apr 18, 2014

Hi,
This is related to #222 and my discussion with @raymondfeng
When using ACL, if you use the role $owner, you cannot access the resource using findOne, but you can using findById.

Here is the definition of the ACLs of "MyModel"

[
            {
                "accessType": "*",
                "permission": "DENY",
                "principalType": "ROLE",
                "principalId": "$everyone"
            },
            {
                "accessType": "*",
                "permission": "ALLOW",
                "principalType": "ROLE",
                "principalId": "$owner"
            }

        ]

I have activated the "loopback:security:role" debug variable, and this is the trace I get when doing a findById:

Fri, 18 Apr 2014 12:36:50 GMT loopback:security:role isInRole(): $everyone {"principals":[{"type":"USER","id":"53144d4e9e79b6f057ffb9c7"}],"modelName":"MyModel","modelId":"530a4696808f657c13e50282","property":"findById","method":"findById","accessType":"READ","accessToken":{"id":"o0Wz1bjD3SEPC7xymUXIBoeDdGGAcliidOhnlQQzHwszBLZHdriDHI1J0oWlqWSS","ttl":1209600,"created":"2014-04-04T14:12:41.424Z","userId":"53144d4e9e79b6f057ffb9c7"}}
Fri, 18 Apr 2014 12:36:50 GMT loopback:security:role Custom resolver found for role $everyone
Fri, 18 Apr 2014 12:36:50 GMT loopback:security:role isInRole(): $owner {"principals":[{"type":"USER","id":"53144d4e9e79b6f057ffb9c7"}],"modelName":"MyModel","modelId":"530a4696808f657c13e50282","property":"findById","method":"findById","accessType":"READ","accessToken":{"id":"o0Wz1bjD3SEPC7xymUXIBoeDdGGAcliidOhnlQQzHwszBLZHdriDHI1J0oWlqWSS","ttl":1209600,"created":"2014-04-04T14:12:41.424Z","userId":"53144d4e9e79b6f057ffb9c7"}}
Fri, 18 Apr 2014 12:36:50 GMT loopback:security:role Custom resolver found for role $owner
Fri, 18 Apr 2014 12:36:50 GMT loopback:security:role isOwner(): MyModel 530a4696808f657c13e50282 userId: 53144d4e9e79b6f057ffb9c7
Fri, 18 Apr 2014 12:36:50 GMT loopback:security:role Model found: {"name":"First MyModel","description":null,"id":"530a4696808f657c13e50282","rulesetId":"530930c5256c45b421682a17","userId":"53144d4e9e79b6f057ffb9c7"}

As you can see, the modelId is correctly filled.
Now when I run a findOne

Fri, 18 Apr 2014 12:38:31 GMT loopback:security:role isInRole(): $everyone {"principals":[{"type":"USER","id":"53144d4e9e79b6f057ffb9c7"}],"modelName":"MyModel","property":"findOne","method":"findOne","accessType":"READ","accessToken":{"id":"o0Wz1bjD3SEPC7xymUXIBoeDdGGAcliidOhnlQQzHwszBLZHdriDHI1J0oWlqWSS","ttl":1209600,"created":"2014-04-04T14:12:41.424Z","userId":"53144d4e9e79b6f057ffb9c7"}}
Fri, 18 Apr 2014 12:38:31 GMT loopback:security:role Custom resolver found for role $everyone
Fri, 18 Apr 2014 12:38:31 GMT loopback:security:role isInRole(): $owner {"principals":[{"type":"USER","id":"53144d4e9e79b6f057ffb9c7"}],"modelName":"MyModel","property":"findOne","method":"findOne","accessType":"READ","accessToken":{"id":"o0Wz1bjD3SEPC7xymUXIBoeDdGGAcliidOhnlQQzHwszBLZHdriDHI1J0oWlqWSS","ttl":1209600,"created":"2014-04-04T14:12:41.424Z","userId":"53144d4e9e79b6f057ffb9c7"}}
Fri, 18 Apr 2014 12:38:31 GMT loopback:security:role Custom resolver found for role $owner

No modelId is present this time. This is what breaks the $owner role, and in fact it will break any dynamic role that need to use the current model instance for findOne.

@bajtos bajtos added the bug label Apr 18, 2014
@Cadrach
Copy link
Author

Cadrach commented Apr 24, 2014

Any update on this? Will dynamic role work as specified in the documentation (with "modelId" always available regardless of the method accessed), or will it be reworked with "less" features?

@ritch
Copy link
Member

ritch commented Apr 24, 2014

In order to solve this we need to implement "post-check" for non-destructive operations.

You can always use hooks to do whatever access control you want in the meantime.

/cc @raymondfeng @bajtos

@coodoo
Copy link

coodoo commented Aug 8, 2014

Related: #450

@venkatperi
Copy link

Is this fixed? or can you post a workaround with hooks?

@dv3000
Copy link

dv3000 commented Mar 29, 2015

I think I found a bug, but I'm not 100% sure if it is my mistake ...

according to this line of code https://github.com/strongloop/loopback/blob/master/lib/application.js#L301-L306

I can do something like this:
http://localhost:8001/v1/collection/findOne?access_token=mycurrenttoken&filter[where][id]=id-2&id=id-1
or
http://localhost:8001/v1/collection?access_token=mycurrenttoken&filter[where][id]=id-2&id=id-1
and get a list
That makes the "findOne" validate the id 1 and the "filters" I seek the id 2

the model has this ACL:
{
"accessType": "*",
"principalType": "ROLE",
"principalId": "$everyone",
"permission": "DENY"
},
{
"accessType": "EXECUTE",
"principalType": "ROLE",
"principalId": "$owner",
"permission": "ALLOW"
}

someone else can reproduce the error??

sorry my english is verry poor

@fabien
Copy link
Contributor

fabien commented Mar 29, 2015

This is not really a bug, it's a limitation of how ACL's currently handled (pre- instead of post find).

You have a few options when restricting access like this:

  • expose all data access through a relation, for example /api/users/:id/things which will scope all operations correctly, making sure one cannot access anything outside of their ownership. You'd usually need to deny all access to /api/things though.
  • use an access operation hook or implement the defaultScope methods to limit access using a more restricted filter/scope. You can use loopback.getCurrentContext and subsequently get the current user token and the userId to build such query. This way, all the api endpoints stay unchanged, they just transparently pre-filter using a more restricted query.

@diaswrd
Copy link

diaswrd commented Aug 7, 2015

+1 to this.

@bajtos
Copy link
Member

bajtos commented Apr 7, 2016

Hello, I am reviewing old issues to check which of them are still relevant. I am little bit confused about what's the actual problem reported here. Could somebody explain 1) what requests to invoke, 2) what is the actual result and 3) was was the expectation? A full app reproducing the problem would be even better, see https://github.com/strongloop/loopback/wiki/Reporting-issues#bug-report

@Cadrach
Copy link
Author

Cadrach commented Apr 7, 2016

I have not been developping on loopback for sometimes now, but the initial report is about an inconsistency of the security system when requesting with findOne & findById

@bajtos
Copy link
Member

bajtos commented Mar 23, 2017

As I was rereading the discussion here, I think the problem is in the fact that LoopBack's ACL system works at method level (a method is allowed or denied) and does not support model-instance restrictions (user X can access only instances 1,2,3 when querying a database).

I'm relabelling this issue as feature. Related: #1427

@stale
Copy link

stale bot commented Aug 23, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this as completed Sep 6, 2017
@stale
Copy link

stale bot commented Sep 6, 2017

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.

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