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

No way to find out if user is authorized to write to a container #171

Closed
rhiaro opened this issue Jan 27, 2016 · 10 comments
Closed

No way to find out if user is authorized to write to a container #171

rhiaro opened this issue Jan 27, 2016 · 10 comments

Comments

@rhiaro
Copy link

rhiaro commented Jan 27, 2016

No way to find out if user is authorized to write to (/delete from) a container without actually trying to write (/delete) something.

@dmitrizagidulin
Copy link
Member

As Henry indirectly alluded to in the solid-spec/#54, there's a way to solve this using Access-Control-Request-Method headers.

Specifically, by sending a "Preflighted request", an "exploratory" HTTP OPTIONS request where you pass in your credentials and the verb you actually want to perform on the resource in the Access-Control-Request-Method header.
Something like:

OPTIONS /documents/doc1 HTTP/1.1
...
Access-Control-Request-Method: DELETE

The server would then return the verbs you're authorized for in Access-Control-Allow-Methods: PUT, GET, OPTIONS, DELETE etc.

@rhiaro
Copy link
Author

rhiaro commented Jan 28, 2016

Sounds good, +1 to implement

@bblfish
Copy link
Member

bblfish commented Jan 30, 2016

I am not sure I alluded to using "Access-Control-Request-Method" in issue 54.

In the CORS specification I think this won't actually work, as you are not meant to be authenticated when sending the preflight request (please verify). The browser will pass the Origin header but that is the identity of the JS, not the identity of the user. CORS is about identifying the agent in the browser, rather than identifying the JS agent itself.

I have written up a wiki page for this issue Client-Authentication Discovery. Interested in feedback.

@dmitrizagidulin
Copy link
Member

you are not meant to be authenticated when sending the preflight request (please verify).

I double-checked the spec, there's no requirement prohibiting the passing of credentials to preflight requests.

@dmitrizagidulin
Copy link
Member

The Client-Authentication Discovery proposal that you link to sounds like decent capability to have on the client library side.

It won't address this particular issue here, since the client won't always have read-access to a resource's ACLs.

@bblfish
Copy link
Member

bblfish commented Jan 31, 2016

you are not meant to be authenticated when sending the preflight request (please verify).

I double-checked the spec, there's no requirement prohibiting the passing of credentials to preflight requests.

The spec is really not easy to read. I know this from experience. I think the relevant part of the spec is 7.1.5 Cross-Origin Request with Preflight. It says in point 1 there

Exclude user credentials.

Concerning your second point:

The Client-Authentication Discovery proposal that you link to sounds like decent capability to have on the client library side.

It won't address this particular issue here, since the client won't always have read-access to a resource's ACLs.

That wiki page also considers what one can do when the ACL is not made visible, in the first section Allow Headers

@solid solid deleted a comment from Mitzi-Laszlo Apr 23, 2020
@csarven
Copy link
Member

csarven commented Apr 23, 2020

Transferring this issue to solid/specification for further consideration. See also #170 as one solution.

@csarven
Copy link
Member

csarven commented May 11, 2020

@bblfish Thanks for writing up the Client-Authentication Discovery for this issue. Still relevant today.

As @dmitrizagidulin mentioned, the client may not be able to read the ACL or even be able to discover the location of the ACL. This is reflected in the current state of the specification:

To discover, read, create, or modify an ACL auxiliary resource, an acl:agent MUST have acl:Control privileges per the ACL inheritance algorithm on the resource directly associated with it.

Even if all clients were to read the ACL, it raises the question of whether the policy they are granted to read is a subset of the information in the ACL. I don't think this flexible or at least requires more from servers - changing the representation based on agent. Alternatively, it is unclear if the link relation should refer to a different ACL resource describing only their readable bits. This duplicates the information and whether the resource is ephemeral or not is unknown.

While Allow includes HTTP methods as value, it is mapped from ACL. The WAC-Allow approach is also based on ACL, and so I presume the cost to calculate per request to be about the same. For an approach based on Link header (using URIs from the ACL vocab) would be in the same category.

Do we have other approaches documented? What implementation experiences can we take into account to make a decision here?

@acoburn
Copy link
Member

acoburn commented May 29, 2020

Trellis does not currently implement support for the WAC-Allow header (though it would be easy to do so). It does support Allow, but the value of the Allow response header is a function of the LDP interaction model (e.g. you can POST to a container but not an RDFSource). It would be an interesting idea to combine those two concepts, though, and filter the list of Allow headers by the operations that are supported. The downside of that approach, though, is that it wouldn't distinguish between the current user and a "public" user, which is what WAC-Allow does.

OTOH, one significant operational downside of the WAC-Allow header (and why Trellis doesn't support it) is that ACL lookups are expensive and WAC-Allow requires two separate lookups for each request: one for the current user and one for an anonymous user. (TBH, I'm not entirely convinced of the necessity of the extra lookup for an anonymous user)

@csarven
Copy link
Member

csarven commented Nov 29, 2020

Proposal/requirements based on the WAC-Allow HTTP header is merged via PR #210 . See further details/discussion on concerns of this header, as well as use cases for different permission groups. Implementation feedback is always useful/welcome. Please follow up with new issues/proposals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Specification
  
Done
Development

No branches or pull requests

5 participants