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

Folder create permissions for "mkdir -p" not enforced? #145

Closed
michielbdejong opened this issue Jun 16, 2022 · 3 comments
Closed

Folder create permissions for "mkdir -p" not enforced? #145

michielbdejong opened this issue Jun 16, 2022 · 3 comments

Comments

@michielbdejong
Copy link
Collaborator

Environment

CSS v4.0.1, node v12.19.1, npm v6.14.8

Description

Save this as acl.ttl which gives any agent read-only access to the server root, and read-write access to any contained resources:

@prefix acl: <http://www.w3.org/ns/auth/acl#>.
@prefix foaf: <http://xmlns.com/foaf/0.1/>.

<#access-to-read> a acl:Authorization;
  acl:agentClass foaf:Agent;
  acl:accessTo <http://localhost:3000/>;
  acl:mode acl:Read.

<#default-read-write> a acl:Authorization;
  acl:agentClass foaf:Agent;
  acl:default <http://localhost:3000/>;
  acl:mode acl:Read, acl:Write.

And upload it to a newly started CSS v4.0.1 instance using:
curl -v -X PUT -H 'Content-Type: text/turtle' -T acl.ttl http://localhost:3000/.acl

Now try these commands:

curl -v -X PUT  -H 'Content-Type: text/plain' -d hello http://localhost:3000/test.txt
curl -v -X PUT  -H 'Content-Type: text/plain' -d hello http://localhost:3000/nested/test.txt

The first will give a 401, the second a 201. And indeed, if you then run curl http://localhost:3000/ you will see that although the creation of /test.txt was blocked correctly, the creation of a /nested folder in the pod root was not prevented:

@prefix dc: <http://purl.org/dc/terms/>.
@prefix ldp: <http://www.w3.org/ns/ldp#>.
@prefix posix: <http://www.w3.org/ns/posix/stat#>.
@prefix xsd: <http://www.w3.org/2001/XMLSchema#>.

<> a <http://www.w3.org/ns/pim/space#Storage>, ldp:Container, ldp:BasicContainer, ldp:Resource;
    dc:modified "2022-06-13T13:51:47.000Z"^^xsd:dateTime;
    <http://www.w3.org/ns/auth/acl#accessControl> <.acl>;
    ldp:contains <index.html>, <nested/>.

However, the spec says that creating that nested/ folder should have require Write or Append on /. Is WAC not enforced for the "mkdir -p" behaviour of creating nested folders?

@michielbdejong
Copy link
Collaborator Author

I had a look in the code, and when trying to create c1/c2/c3/c4/r, there is a check whether c1/c2/c3/c4/r exists, and if not, the "parentAcl" (that of c1/c2/c3/c4/) is retrieved and correctly checked for Append-or-Write access, as it is required for the creation of c1/c2/c3/c4/r itself.
What is not checked is whether the client has permissions to create c1/, c1/c2/, c1/c2/c3/ and c1/c2/c3/c4/.
Working on a fix for this in my fork of CSS.

michielbdejong added a commit to pdsinterop/community-server that referenced this issue Jun 29, 2022
@michielbdejong
Copy link
Collaborator Author

The PermissionBasedAuthorizer has access to a resourceSet where it can check if a resource exists.
Maybe it should also get an identifierStrategy so it can determine parent paths
But it can't return anything other than Promise<void>.
I tried injecting an identifierStrategy and a resourceSet into the AuthorizingHttpHandler but that led to strange errors. Even if I just add them in the componentsjs config and not use them, it thinks all requests come from an unauthenticated user and rejects them.
Will maybe try again some other day. If I get it working I'll publish it as a spec-compliant fork of CSS (unfortunately we can't fix it in CSS itself since team has asked not be contacted, so we can't create PRs there).

@TallTed
Copy link

TallTed commented Jun 29, 2022

can't fix it in CSS itself since team has asked not be contacted, so we can't create PRs there

That's absurd. If they haven't archived the repo, it's entirely legitimate to create PRs. If they won't merge code fixes, it would be reasonable to submit PR against their README that point to more lively fork(s).

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