add filesystem/classpath backstop #39

acoburn · 2015-10-28T04:49:48Z

See: https://jira.duraspace.org/browse/FCREPO-1792

awoods · 2015-10-29T14:13:27Z

src/main/java/org/fcrepo/auth/webac/WebACRolesProvider.java

@@ -138,7 +147,7 @@ public void deleteRoles(final Node node) throws RepositoryException {
        // Read the effective Acl and return a list of acl:Authorization statements
        final List<WebACAuthorization> authorizations = effectiveAcl
                .map(uncheck(x -> getAuthorizations(x.getLeft().toString())))
-                .orElse(new ArrayList<>());
+                .orElseGet(() -> getDefaultAuthorizations());


Instead of tying in the default ACL here, I would have thought the backstop ACL would logically be returned when searching for getEffectiveAcl:
https://github.com/acoburn/fcrepo-module-auth-webac/blob/fcrepo-1792/src/main/java/org/fcrepo/auth/webac/WebACRolesProvider.java#L121

Was there a reason you did not go that direction?

Because getEffectiveAcl returns a Pair<URI, FedoraResource> which than gets loaded and the children (e.g. acl:Authorization nodes) are read -- and all of that is irrelevant for a default acl (which is already an acl:Authorization).

That is, I don't know what a filesystem-based FedoraResource would be (?). If we were to go that direction, the getAuthorizations method would need to be completely rewritten. This route is extremely light-weight -- the default is loaded only if an in-domain acl resource is not found.

Is it correct then, that the algorithm of determining the effective policy is now different than documented here:
https://wiki.duraspace.org/display/FEDORA4x/WebAC+Authorization+Delegate#WebACAuthorizationDelegate-Stepsindeterminingtheeffectiveauthorization

The default is not "Deny Access", but rather, "whatever is in the root-authorization.rdf"?
...or rather, "Deny Access" preceded by a search for "root-authorization.rdf".

That is correct, which is why we should discuss the contents of that file.

The default acl:Authorization will now deny access to all non-admin users, but the wiki should be updated as you describe.

awoods · 2015-10-30T00:55:29Z

Something odd. When I use the following root authorization:

@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
@prefix acl: <http://www.w3.org/ns/auth/acl#> .
@prefix foaf: <http://xmlns.com/foaf/0.1/> .
@prefix fedora: <http://fedora.info/definitions/v4/repository#> .

<> a acl:Authorization ;
   acl:agent "user1" ;
   acl:mode acl:Write, acl:Read ;
   acl:accessTo <http://localhost:8080/rest/> .

And try to read the root of the repository with:

curl -i -uuser1:password1 localhost:8080/rest/

403 Forbidden

If I create an ACL and Authorization identical to the authorization above within the repository and link to it from the root with acl:accessControl, and execute the same curl request:

404

If I use an admin user to create a resource /rest/collection, and execute:

curl -i -uuser1:password1  localhost:8080/rest/collection

403

If I change the root authorization to:

@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
@prefix acl: <http://www.w3.org/ns/auth/acl#> .
@prefix foaf: <http://xmlns.com/foaf/0.1/> .
@prefix fedora: <http://fedora.info/definitions/v4/repository#> .

<> a acl:Authorization ;
   acl:agent "user1" ;
   acl:mode acl:Write, acl:Read ;
   acl:accessTo <http://localhost:8080/rest/collection> .

and execute:

curl -i -uuser1:password1  localhost:8080/rest/collection

403

But if I create an ACL and Authorization as above (pointing to /rest/collection) within the repository, and link it to the /rest/collection resource with acl:accessControl and execute:

curl -i -uuser1:password1  localhost:8080/rest/collection

Success

It seems that something is not working as expected.

acoburn · 2015-10-30T04:08:03Z

@awoods the previous code did not properly support acl:accessTo properties in the file-based Authorization. Part of the issue was that the resource path did not map to what acl:accessTo might have expected (this is now fixed and an integration test is in place for this).

The second issue relates to internal/external translations of the path. I left the implementation as-is, but I am open to changing it. The webac code operates on internal URIs: <info:fedora/> as opposed to external URIs: <http://localhost:8080/fcrepo/rest>. I see certain advantages of the current setup (using internal paths), primarily that the system configuration is indifferent to servlet changes to the context, port or host changes. Of course, that may be confusing to administrators.

Thoughts?

awoods · 2015-10-31T22:01:32Z

src/main/java/org/fcrepo/auth/webac/WebACRolesProvider.java

@@ -132,6 +145,16 @@ public void deleteRoles(final Node node) throws RepositoryException {
                rdfTypes.addAll(x.getTypes());
            });

+        // If we get the default Authorization and it contains acl:accessTo properties,


It is unclear to me what you are achieving with this block... particularly since the comment is somewhat misleading by referring to the "default Authorization", which is not found until line:169 further down.

This block handles the following case:

Say you make a request for /rest/foo/bar/baz

If there is no acl:accessControl triple on that resource, it searches up the hierarchy to the root. If no acl:accessControl triple is found (effectiveAcl.isPresent() == false), the filesystem backstop is checked. Now, say that filesystem backstop contains an acl:accessTo triple pointing to the root: acl:accessTo <info:fedora/> (or, alternately: acl:accessTo <info:fedora/foo>). This is where this block comes in.

Later on, when the code checks the List<WebACAuthorization>, it needs to make sure that the path of the requested resource matches the resource pointed to by acl:accessTo, but if the filesystem backstop is used, we need a way to line up the path of the original resource and whatever is listed in the acl:Authorization. In the more traditional case (where there is an acl:accessControl triple somewhere in the hierarchy), we simply use that path, but we don't have that information in this case. Thus, we need to provide all the possible paths: /rest/, /rest/foo, /rest/foo/bar and /rest/foo/baz. That is what this block generates. With that list in place, it can be compared to the acl:accessControl triple(s) in the filesystem backstop. If any of those match, then the ACL modes are applied. If none of them match (and there is no relevant acl:accessToClass), then the request is denied.

The block is located here because resourcePaths is needed in the next line when the checkAccessTo Predicate is created. (The accessTo function forms a closure over the value of resourcePaths).

Thanks, @acoburn, that makes sense... however, the block itself and associated comment did not shine much light in that direction for me. Would you please update the comment or pull this into its own private method to make the logic and intent more clear?

awoods · 2015-11-01T02:22:35Z

Resolved with: 7d9cf0d

awoods reviewed Oct 29, 2015
View reviewed changes

awoods mentioned this pull request Oct 29, 2015

Implement support for acl:agentClass triples #40

Closed

awoods reviewed Oct 31, 2015
View reviewed changes

acoburn added 5 commits October 31, 2015 22:07

add filesystem/classpath backstop

c406eae

updates from code review

3bc6306

rename root-authorization file

88b71fe

more updates from code review

483fdf0

clarified intention of code

78c8b4f

awoods closed this Nov 1, 2015

acoburn deleted the fcrepo-1792 branch July 21, 2016 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add filesystem/classpath backstop #39

add filesystem/classpath backstop #39

acoburn commented Oct 28, 2015

awoods Oct 29, 2015

acoburn Oct 29, 2015

awoods Oct 29, 2015

acoburn Oct 29, 2015

acoburn Oct 29, 2015

awoods commented Oct 30, 2015

acoburn commented Oct 30, 2015

awoods Oct 31, 2015

acoburn Oct 31, 2015

awoods Oct 31, 2015

awoods commented Nov 1, 2015

add filesystem/classpath backstop #39

add filesystem/classpath backstop #39

Conversation

acoburn commented Oct 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awoods commented Oct 30, 2015

acoburn commented Oct 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awoods commented Nov 1, 2015