solid-contrib / test-suite

An automated test of Solid specification technical compliance
MIT License
23 stars 10 forks source link

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

Closed michielbdejong closed 2 years ago

michielbdejong commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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).