-
Notifications
You must be signed in to change notification settings - Fork 20
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
Effective ACR discovery #244
Conversation
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: elf Pavlik <elf-pavlik@hackers4peace.net>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to clarify a bit more how WAC works here from the client's perspective.
> 2. Let aclResource be the ACL resource of resource. | ||
> 3. If resource has an associated aclResource with a representation, return aclResource. | ||
> 4. Otherwise, repeat the steps using the container resource of resource. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the algorithm for determining the ACL of a resource in the spec is relevant to the client.
In the previous wiki version of the spec on which the current spec is based it was stated quite clearly that one should not try to guess.
So I think that text is only aimed at the server, to allow the server to determine which ACL to add to the header. Am I right @csarven ?
btw. I think that means the text "To determine the effective ACL..." should be: "For a server to determine..." the effective ACL of a resource...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is aimed at any anything that wants to determine the ACL of a resource, including servers, clients..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous version of the document that was the official one just a little over a month ago, stated that
It is considered an anti-pattern for a client to perform those steps, however. A client may discover and load a document's individual resource (for example, for the purposes of editing its permissions). If such a resource does not exist, a client SHOULD NOT search for, or interact with, the inherited ACLs from an upstream container.
Also the algorithm given for finding the container is very vague, because you are trying to specify WAC without assuming LDP, which would make finding the container a lot simpler.
So should we instead write up an example where the client has to try to guess its way back up the hierarchy to find the default ACL? I guess we can make our lives a bit simpler to ourselves if we also have a
Link: <.>; rev="http://www.w3.org/ns/ldp#contains">
on the resource, so that the client can find the default.
Which do you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous document did say that but we also had an issue on solid/specification#106 to change that. WAC ED's #effective-acl-resource addressed that issue.
As for WAC without LDP's containment.. that makes the spec more flexible and combined with other specs. Servers implementing LDP/Solid can already work with it.
|
||
### WAC | ||
|
||
In WAC, the effective ACR of a resource might be the ACR of a parent container of the resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the client's point of view the only thing that matters should be the Link: <....acl>; rel="acl">
relation in the header.
The text about the effective ACL process (see my other comment) is aimed at the server I think.
But that means that the Link: </.acl>, rel="acl">
pointing to the default only allows the client to edit the default ACL document, as the client cannot just create ACL resources by guessing them.
So that means that: a server that allows the client to also edit acls for a resource must have 2 Link:
headers: one for the default and that could be created with a PUT, in order to override the default. I think @csarven agreed with that in the last meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recall saying that. I probably said that (UN)LINK methods can be used or extensions can be introduced to do those things - as mentioned in the current WAC ED.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I thought we had come to the conclusion that having two acl links could solve the problem. But it is not in the meeting notes.
Does the spec explicitly state that one cannot have two link headers?
Anway, so if I understand you correctly, the right way to do things is for the acl link header to point to the non-existent acl for that resource, and if the client gets a 404 Page not found then it should follow the links down the hierarchy container by container to find the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a full description of what needs to be done in WAC with a default three levels deep in this commit. 893dc36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superb, thank you Henry.
```HTTP | ||
200 Ok | ||
Link: </foo/bar/baz/x.acr>; rel="acl" | ||
Link: <.>; rev="http://www.w3.org/ns/ldp#contains" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is rev ldp contains in any spec? Any implementation? Why is this part of the evaluation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the WAC spec says
WAC has the property of being recursive with respect to container hierarchy, meaning that a member resource inherits Authorizations from the closest container resource (heading towards the root container).
For ldp the way to find the container is by following the reverse of ldp:contains
.
As I understand you don't specify how to find the container. So I don't see any other way for ldp of finding the parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right in that it is unspecified, and that can be a reasonable way for LDP clients to find the container of a resource.
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
A simple description of how effective access control resources are discovered.
That mechanism will hold for all Use Case, so in order to avoid unnecessary repetition, it is here factored out.