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

Add WAC-Allow definition and requirements #210

Merged
merged 8 commits into from
Nov 27, 2020
Merged

Conversation

csarven
Copy link
Member

@csarven csarven commented Nov 15, 2020

In response to the following (and possibly other) issues on the WAC-Allow header:

Future considerations:

In order for this HTTP header to be reusable by any access control system, we can consider changing the header field-name as well as the parameters and values.

Introduce new modes eg. "delete", if and when needed.

At the moment, access modes corresponds to the ones defined in the ACL ontology (also kept in context of the Web Access Control section), however, we can relax this with some editing.

There is also the alternative approach to using URIs instead of human-friendly terms for the allowed access modes. While that'd be precise it may be less flexible if access control systems change.

@csarven csarven added this to the ~First Public Working Draft milestone Nov 15, 2020
@csarven csarven requested review from michielbdejong, acoburn and a team November 15, 2020 22:01
@csarven csarven added this to Drafting in Specification via automation Nov 15, 2020
@csarven csarven requested a review from bblfish November 15, 2020 22:01
main/authorization.bs Outdated Show resolved Hide resolved
Copy link
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start, but:

  • not fully in line yet with the original intentions as documented in code (it's fine if the intentions change, just wanted to document this)
  • does not explicitly mention doubts regarding this header

main/authorization.bs Show resolved Hide resolved
main/authorization.bs Outdated Show resolved Hide resolved
main/authorization.bs Outdated Show resolved Hide resolved
main/authorization.bs Outdated Show resolved Hide resolved
@bblfish
Copy link
Member

bblfish commented Nov 16, 2020

If I am not mistaken this is needed because in the current spec one needs Control rights on a resource to see the ACL. If so this should be made clear.

I believe there has been a shift in perception in the Authorization panel in the past 3 months, to acknowledge that allowing ACLs to be readable by a larger group is not necessarily a bad thing, and can in fact be useful. For example it makes sense for resources that can be accessed after proof of payment, proof of age, etc... to be world readable. It is also possible to have publicly visible ACLs that give access to restricted groups without revealing the membership of the groups, reducing concerns regarding confidentiality.

Whatever one's position on this is, the exercise should be quite simple. Would this be needed were access to the ACL visible to clients (or their authentication agents)? If not that should be documented, as that helps motivate the header. If even with such access the header would be needed then that could provide extra support for it, that would also help understand the reason for it.

@RubenVerborgh
Copy link
Contributor

If I am not mistaken this is needed because in the current spec one needs Control rights on a resource to see the ACL. If so this should be made clear.

Correct, and important indeed!

I believe there has been a shift in perception in the Authorization panel in the past 3 months, to acknowledge that allowing ACLs to be readable by a larger group is not necessarily a bad thing

Instead of this header, we could link to (a summarized version of) the ACL.

@acoburn
Copy link
Member

acoburn commented Nov 16, 2020

I believe there has been a shift in perception in the Authorization panel in the past 3 months, to acknowledge that allowing ACLs to be readable by a larger group is not necessarily a bad thing

For example, the Access Policy proposal makes it possible to distinguish between read and write in the context of an ACL. There are certainly cases where the ACL resource should be kept private and other cases where it's OK for it to be readable.

@csarven
Copy link
Member Author

csarven commented Nov 16, 2020

I'm sure we are all on the same page here but just to be clear we had the two (read and write) contexts all along. For clients to be able to write to ACL was there from early on obviously. And for clients to be able read / know about allowed privileges perhaps became more apparent from use cases based on actual applications ( eg. linkeddata/dokieli#137 , circa 2016 ). All of that predates WAC-Allow, ACP, authorization-panel... The functional requirements are more or less known and was agreed. We just didn't agree on the exact solution and carry far with the implementations. WAC-Allow just happens to be one approach to meet the read case - whether that came through by use cases recorded elsewhere doesn't matter at this point. Some aspects of WAC-Allow are attractive (to me) and some not. That's all fine though.. But this is not to say that the alternative approaches have been around for a long time, agreed, and in wide use across different implementations.

While we had rel=acl under WAC for the write case, we didn't "think outside of the box" or at least didn't get to proposing something like rel=readable-acl-for-requesting-agent referring to another ACL document that clients can read. Or bothered - ie. actually implement and propose for spec - to scratch the Link header approach.

There can be 50 different ways of addressing the same functional requirement. We just need one approach. Link header with rel allow is perfectly fine if that seems attractive now.. or having a separate ACL document just to read allowed privileges. That's all in a way "accidental" as far as I can see. Different times.. people.. in the mix.

I'd only suggest that we consider making the solution flexible enough to handle different access control systems. Meaning, along the lines of WAC-Allow's idea of using simple values eg. "read" implying acl:Read but it doesn't need to be acl:. So a bit generic like solid:Read (or whatever) to tell the client that it can do "read operations", or solid:Write for "write operations" and so on that corresponds to the operations as defined (and to be improved) in the Solid ecosystem. In the end something like:

Link: <http://www.w3.org/ns/solid/terms#Read>; rel="http://www.w3.org/ns/solid/terms#allow"

(I don't particularly care which namespace to use or what the term is actually called but probably close enough.)

So, in about 1.61803 years from now when we are reviewing a yet another access control system, we don't flip tables (again).

@bblfish
Copy link
Member

bblfish commented Nov 17, 2020

I am not sure we need different Link headers for read and write.
All I was suggesting is that the reasons for why the proposed feature is needed at this time should be documented. The "WAC-Allow" feature may provide a good stop-gap answer until a deeper one is provided.

@csarven
Copy link
Member Author

csarven commented Nov 27, 2020

WAC-Allow header is now being used by dokieli to determine whether the "Save" button in the menu will be actionable or not (disables the HTML button). Brief detail and screenshots:

linkeddata/dokieli#259 (comment)

main/authorization.bs Outdated Show resolved Hide resolved
main/authorization.bs Show resolved Hide resolved
main/authorization.bs Outdated Show resolved Hide resolved
@csarven
Copy link
Member Author

csarven commented Nov 27, 2020

Thanks all. Let's go for it. Please follow up with issues / concerns / implementation feedback.

@csarven csarven closed this Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Specification
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants