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

Is create an append operation? #105

Closed
michielbdejong opened this issue Jun 13, 2022 · 8 comments
Closed

Is create an append operation? #105

michielbdejong opened this issue Jun 13, 2022 · 8 comments

Comments

@michielbdejong
Copy link
Contributor

michielbdejong commented Jun 13, 2022

Suppose c/ exists, and c/r does not yet.
An agent creates c/r. Does this require:

  • Option 1: Append or Write on c/ and Write on c/r
    or
  • Option 2: Append or Write on c/ and Append or Write on c/r

Evidence of option 1:

Evidence of option 2:

write operations attempt to create, delete, or modify resources;
append operations attempt to create resources or add information to existing resources

I want to go for option 2 (and update the WAC tests accordingly and file a bug report to CSS), but wanted to check here first.

@michielbdejong
Copy link
Contributor Author

As Martynas mentioned on Gitter, probably option 2.
Let's wait a bit to give others a chance to comment in case there are more perspectives on this.

@michielbdejong
Copy link
Contributor Author

CC @ylebre @bourgeoa @gibsonf1

@csarven
Copy link
Member

csarven commented Jun 17, 2022

The evaluation of an authorization is concerned with finding Authorizations that match the required parameters of an operation. It depends on the HTTP request semantics and conditions. See also https://solid.github.io/web-access-control-spec/#http-method-access-mode-mapping , #85 (comment) .

For example, /C/R would be created given: request PATCH /C/R or PUT /C/R and finding Append (or Write) on /C/ and Write on /C/R; request POST /C/ and finding Append on /C/.

@michielbdejong
Copy link
Contributor Author

Ah, interesting. So then why does it say "append operations attempt to create resources or add information to existing resources"? Does that term "append operations" not refer to "operations for which acl:Append suffices"?

@michielbdejong
Copy link
Contributor Author

@csarven so it's option 1 then, despite that phrase about "append operations attempt to create ..."?

@csarven
Copy link
Member

csarven commented Jun 20, 2022

It is hard for me to answer your question directly because the primary source (HTTP method) of the request semantics that indicates the purpose of the request and the target of the HTTP request (URI) in the scenario is missing. The complete HTTP request semantics needs to be taken into account in order to determine the parameters of an operation and what Authorization rule(s) needs to be matched.

Even without that information, it is still unclear whether the server or the client assigns the name C/R.

What you are quoting is a general description about operations. (But don't run off with that.) We can append to C/ with POST (it is a resource-specific processing resulting in C/R). We can also append or add data to an existing resource C/R (POST, PATCH) but this is not the scenario you're describing.

What I'm saying is that if an agent created C/R, it was probably because there was a PUT or a PATCH targeting C/R or a POST targeting C/.

Append suffices for POST targeting the C/ and that server will name C/R. Append on C/R will not suffice for PUT or PATCH - write is required so that client can name C/R.

If you're satisfied with the answer, feel free to close the issue.

@michielbdejong
Copy link
Contributor Author

Yes, sorry, I forgot to state that I was talking about PUT and PATCH. OK, so option 1 then, thanks! I'll create a PR to edit the confusing text.

michielbdejong added a commit to michielbdejong/web-access-control-spec that referenced this issue Jun 25, 2022
@michielbdejong
Copy link
Contributor Author

As commented in #106, let's leave the spec text as it is but at least we have clarity now.

Thanks!

michielbdejong added a commit to solid-contrib/web-access-control-tests that referenced this issue Jun 29, 2022
michielbdejong added a commit to solid-contrib/web-access-control-tests that referenced this issue Jun 29, 2022
michielbdejong added a commit to michielbdejong/web-access-control-spec that referenced this issue Dec 6, 2023
This updates the text to match what we decided in solid#105 (comment). At the time we decided not to update the spec text, but now that the spec text is more detailed, the current statement is not correctly conveying that access to both the containing folder and the non-existing resource URL is required.

See the confusion that was created by this in solid-contrib/web-access-control-tests#56 which was an (I think incorrect) reaction to solid#122.
csarven added a commit that referenced this issue Dec 15, 2023
)

* Further clarify requirements for PUT-to-create and PATCH-to-create

This updates the text to match what we decided in #105 (comment). At the time we decided not to update the spec text, but now that the spec text is more detailed, the current statement is not correctly conveying that access to both the containing folder and the non-existing resource URL is required.

See the confusion that was created by this in solid-contrib/web-access-control-tests#56 which was an (I think incorrect) reaction to #122.

* Apply suggestions from code review

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

---------

Co-authored-by: Sarven Capadisli <info@csarven.ca>
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
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

2 participants