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

reading of ACL resource should be reduced to requiring client to have Read rights. #250

Closed
wants to merge 3 commits into from

Conversation

bblfish
Copy link
Member

@bblfish bblfish commented Mar 15, 2021

The ACL ontology states:

  acl:Control     a :Class;
         :comment "Allows read/write access to the ACL for the resource(s)";
         :label "control"@en;
         :subClassOf acl:Access .

This clearly gives read/write access to the controller. But that is different from disallowing read access to anyone else, which is what the removed text stated.

There are use cases for making ACLs readable in order to guarantee privacy of the client. There are ways to extend ACL so that ACLs become readable, see ACLs on ACLs, and this is also needed by ACP.
Having the default be as it currently is specified in WAC that ACLs be only visible to those who control them, when no other information is available, is a default choice that makes sense. But closing doors to valid use cases that would allow such extensions is not justifiable.

acl:Control is about editability of a resource, not readability. 
There are use cases for making ACLs readable in order to guarantee [privacy of the client](https://solid.github.io/authorization-panel/authorization-ucr/#uc-minimalcredentials). There are ways to extend ACL so that ACLs become readable, see [ACLs on ACLs](solid/authorization-panel#189), and this is also needed by ACP. 
Having the default be as it currently is specified in WAC that ACLs be only visible to those who control them, is a default choice that can be defended. But closing doors to valid use cases is not.
create was too hastily removed.
@bblfish
Copy link
Member Author

bblfish commented Mar 15, 2021

To cover discoverability I would be fine with text such as the following:

In order to allow a client that has read access to an ACL to discover it, a server MUST place a Link: <...>; rel="acl" header in the response to the associated resource.

@bblfish bblfish changed the title reduce requirements on acl:Control to edit reduce requirements on acl:Control to edit and create Mar 15, 2021
@gibsonf1
Copy link

My vote is also to have default behavior be that an acl is an acl of itself unless the acl has its own acl. (This is how our current acl algorithm is operating)

@bblfish
Copy link
Member Author

bblfish commented Mar 16, 2021

@gibsonf1 I am in favor of having ACLs of ACLs and of an ACL being its own ACL :-) It would be useful to open a separate PR for that. I am not sure where though.

Here I am just proposing to remove 2 words that could make it seem like the behavior you have implemented was not legal.

@csarven
Copy link
Member

csarven commented Mar 23, 2021

Thanks for highlighting the problematic text and a clarification for it. Closing this PR as agreed in favour of #252

@csarven csarven closed this Mar 23, 2021
@bblfish bblfish changed the title reduce requirements on acl:Control to edit and create reading of ACL resource should be reduced to requiring client to have Read rights. Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants