Issue looking up remote group acl

[justinwb]

## Please describe what you did in reproducible steps
@timbl had a correct ACL rule that was referencing a group hosted on a different server. My webid was included in this group. When trying to access a file on tim's server covered by this acl, i was getting a 404. In the server log, it was reporting that it was unable to find the group, but the path it specified for the group was local to tim's server.

How did it work with 4.x series servers?

Correctly 😄

What happened when you tried the same with the 5.0.0 beta?

404

Any material that will help, logs, error messages, etc.

/Library/Server/Documents/group-file +0ms solid:server Error page because of: { [HTTPError: HTTPError: File not found: /Library/Server/Documents/group-file] name: 'HTTPError', message: { [HTTPError: File not found: /Library/Server/Documents/group-file] name: 'HTTPError', message: 'File not found: /Library/Server/Documents/group-file', status: 404 }

[timbl]

Looks like a problem that a local file read function is being used to fetch the group resource.

[timbl]

Quick fix seems to work as follows.

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 53c41f8..fbc6b17 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -1,6 +1,8 @@
 'use strict'
 
 const rdf = require('rdflib')
+const fetch = require('node-fetch')
+
 const debug = require('./debug').ACL
 const HTTPError = require('./http-error')
 const aclCheck = require('@solid/acl-check')
@@ -113,10 +115,13 @@ class ACLChecker {
     if (!returnAcl) {
       throw new HTTPError(500, `No ACL found for ${resource}, searched in \n- ${acls.join('\n- ')}`)
     }
+    var graph = returnAcl.graph
+    if (!graph.fetcher) rdf.fetcher(graph)
     const groupNodes = returnAcl.graph.statementsMatching(null, ACL('agentGroup'), null)
     const groupUrls = groupNodes.map(node => node.object.value.split('#')[0])
     await Promise.all(groupUrls.map(groupUrl => {
-      this.requests[groupUrl] = this.requests[groupUrl] || this.fetch(groupUrl, returnAcl.graph)
+      this.requests[groupUrl] = this.requests[groupUrl] || graph.fetcher.load(groupUrl)
+      console.log('@@@ fetching ' + groupUrl)
       return this.requests[groupUrl]
     }))
 

[timbl]

actually the +const fetch = require('node-fetch') is unnecessary -- that was another way I was going to go. But the function it need (and misleadlngly called fetch) is load - fetch uit parse it and stick it in the store.

[michielbdejong]

Thanks! @jaxoncreed do you want to take a look at this one? I see it's already assigned to you, we can discuss in today's triage meeting if anything's unclear.

[michielbdejong]

Ah, just saw you already created #1160. Let's discuss in the triage meeting today!

[jaxoncreed]

@michielbdejong Sounds good

[timbl]

The reason for fetching the group file via a file get rather than an http get was that it avoids the access control, and therefore allows people to use non-public groups in ACLs, though only for resources on the same server. Doing this of course is against the solid philosophy that all pods are equivalent, and any group can be given access to anything.

[kjetilk]

So, @timbl, you're saying that we should not allow filesystem access at all, not even as an optimization? In which case, #1160 is not the one we'd merge.