From 8148887dee7459120ab20b5700dd8c106b4eb9c9 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Thu, 17 Aug 2017 23:08:28 -0400
Subject: [PATCH 01/12] Convert checkAccess to promise.

---
 lib/acl-checker.js            | 46 ++++++++++++++++-------------------
 test/unit/acl-checker-test.js | 37 +++++++++-------------------
 2 files changed, 33 insertions(+), 50 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 8cda9d396..83bcb5c1f 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -46,9 +46,8 @@ class ACLChecker {
             resource, // The resource we want to access
             accessType, // accessTo or defaultForNew
             acl, // The current Acl file!
-            (err) => { return next(!err || err) },
             options
-          )
+          ).then(next, next)
         })
       },
       function handleNoAccess (err) {
@@ -87,17 +86,14 @@ class ACLChecker {
    * @param resource {String} URI of the resource being accessed
    * @param accessType {String} One of `accessTo`, or `default`
    * @param acl {String} URI of this current .acl resource
-   * @param callback {Function}
    * @param options {Object} Options hashmap
    * @param [options.origin] Request's `Origin:` header
    * @param [options.host] Request's host URI (with protocol)
    */
-  checkAccess (graph, user, mode, resource, accessType, acl, callback,
-               options = {}) {
-    const debug = this.debug
+  checkAccess (graph, user, mode, resource, accessType, acl, options = {}) {
     if (!graph || graph.length === 0) {
       debug('ACL ' + acl + ' is empty')
-      return callback(new Error('No policy found - empty ACL'))
+      return Promise.reject(new Error('No policy found - empty ACL'))
     }
     let isContainer = accessType.startsWith('default')
     let aclOptions = {
@@ -107,26 +103,26 @@ class ACLChecker {
       origin: options.origin,
       rdf: rdf,
       strictOrigin: this.strictOrigin,
-      isAcl: (uri) => { return this.isAcl(uri) },
-      aclUrlFor: (uri) => { return this.aclUrlFor(uri) }
+      isAcl: uri => this.isAcl(uri),
+      aclUrlFor: uri => this.aclUrlFor(uri)
     }
     let acls = new PermissionSet(resource, acl, isContainer, aclOptions)
-    acls.checkAccess(resource, user, mode)
-      .then(hasAccess => {
-        if (hasAccess) {
-          debug(`${mode} access permitted to ${user}`)
-          return callback()
-        } else {
-          debug(`${mode} access NOT permitted to ${user}` +
-            aclOptions.strictOrigin ? ` and origin ${options.origin}` : '')
-          return callback(new Error('ACL file found but no matching policy found'))
-        }
-      })
-      .catch(err => {
-        debug(`${mode} access denied to ${user}`)
-        debug(err)
-        return callback(err)
-      })
+    return acls.checkAccess(resource, user, mode)
+    .then(hasAccess => {
+      if (hasAccess) {
+        this.debug(`${mode} access permitted to ${user}`)
+        return true
+      } else {
+        this.debug(`${mode} access NOT permitted to ${user}` +
+          aclOptions.strictOrigin ? ` and origin ${options.origin}` : '')
+        throw new Error('ACL file found but no matching policy found')
+      }
+    })
+    .catch(err => {
+      this.debug(`${mode} access denied to ${user}`)
+      this.debug(err)
+      throw err
+    })
   }
 
   aclUrlFor (uri) {
diff --git a/test/unit/acl-checker-test.js b/test/unit/acl-checker-test.js
index a4569e5b8..3626c1daf 100644
--- a/test/unit/acl-checker-test.js
+++ b/test/unit/acl-checker-test.js
@@ -1,13 +1,9 @@
 'use strict'
 const proxyquire = require('proxyquire')
-const chai = require('chai')
-const { assert } = chai
-const dirtyChai = require('dirty-chai')
-chai.use(dirtyChai)
-const sinonChai = require('sinon-chai')
-chai.use(sinonChai)
-chai.should()
 const debug = require('../../lib/debug').ACL
+const chai = require('chai')
+const { expect } = chai
+chai.use(require('chai-as-promised'))
 
 class PermissionSetAlwaysGrant {
   checkAccess () {
@@ -26,7 +22,7 @@ class PermissionSetAlwaysError {
 }
 
 describe('ACLChecker unit test', () => {
-  it('should callback with null on grant success', done => {
+  it('should callback with null on grant success', () => {
     let ACLChecker = proxyquire('../../lib/acl-checker', {
       'solid-permissions': { PermissionSet: PermissionSetAlwaysGrant }
     })
@@ -34,13 +30,10 @@ describe('ACLChecker unit test', () => {
     let accessType = ''
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    acl.checkAccess(graph, user, mode, resource, accessType, aclUrl, (err) => {
-      assert.isUndefined(err,
-        'Granted permission should result in an empty callback!')
-      done()
-    })
+    return expect(acl.checkAccess(graph, user, mode, resource, accessType, aclUrl))
+    .to.eventually.be.true
   })
-  it('should callback with error on grant failure', done => {
+  it('should callback with error on grant failure', () => {
     let ACLChecker = proxyquire('../../lib/acl-checker', {
       'solid-permissions': { PermissionSet: PermissionSetNeverGrant }
     })
@@ -48,13 +41,10 @@ describe('ACLChecker unit test', () => {
     let accessType = ''
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    acl.checkAccess(graph, user, mode, resource, accessType, aclUrl, (err) => {
-      assert.ok(err instanceof Error,
-        'Denied permission should result in an error callback!')
-      done()
-    })
+    return expect(acl.checkAccess(graph, user, mode, resource, accessType, aclUrl))
+    .to.be.rejectedWith('ACL file found but no matching policy found')
   })
-  it('should callback with error on grant error', done => {
+  it('should callback with error on grant error', () => {
     let ACLChecker = proxyquire('../../lib/acl-checker', {
       'solid-permissions': { PermissionSet: PermissionSetAlwaysError }
     })
@@ -62,10 +52,7 @@ describe('ACLChecker unit test', () => {
     let accessType = ''
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    acl.checkAccess(graph, user, mode, resource, accessType, aclUrl, (err) => {
-      assert.ok(err instanceof Error,
-        'Error during checkAccess should result in an error callback!')
-      done()
-    })
+    return expect(acl.checkAccess(graph, user, mode, resource, accessType, aclUrl))
+    .to.be.rejectedWith('Error thrown during checkAccess()')
   })
 })

From e6ac576fec8067fd7378b3322b917b99d2c5429f Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Thu, 17 Aug 2017 23:54:54 -0400
Subject: [PATCH 02/12] Convert can to promise.

---
 lib/acl-checker.js    | 89 ++++++++++++++++++++-----------------------
 lib/handlers/allow.js |  3 +-
 2 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 83bcb5c1f..21caa2388 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -1,10 +1,10 @@
 'use strict'
 
-const async = require('async')
 const path = require('path')
 const PermissionSet = require('solid-permissions').PermissionSet
 const rdf = require('rdflib')
 const url = require('url')
+const HTTPError = require('./http-error')
 
 const DEFAULT_ACL_SUFFIX = '.acl'
 
@@ -16,63 +16,58 @@ class ACLChecker {
     this.suffix = options.suffix || DEFAULT_ACL_SUFFIX
   }
 
-  can (user, mode, resource, callback, options = {}) {
+  can (user, mode, resource, options = {}) {
     const debug = this.debug
     debug('Can ' + (user || 'an agent') + ' ' + mode + ' ' + resource + '?')
-    var accessType = 'accessTo'
-    var possibleACLs = ACLChecker.possibleACLs(resource, this.suffix)
     // If this is an ACL, Control mode must be present for any operations
     if (this.isAcl(resource)) {
       mode = 'Control'
     }
-    var self = this
-    async.eachSeries(
-      possibleACLs,
 
-      // Looks for ACL, if found, looks for a rule
-      function tryAcl (acl, next) {
+    // Find nearest ACL
+    let accessType = 'accessTo'
+    let nearestACL = Promise.reject()
+    for (const acl of ACLChecker.possibleACLs(resource, this.suffix)) {
+      nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
         debug('Check if acl exist: ' + acl)
-        // Let's see if there is a file..
-        self.fetch(acl, function (err, graph) {
-          if (err || !graph || graph.length === 0) {
-            if (err) debug('Error: ' + err)
+        this.fetch(acl, function (err, graph) {
+          if (err || !graph || !graph.length) {
+            if (err) debug(`Error reading ${acl}: ${err}`)
             accessType = 'defaultForNew'
-            return next()
-          }
-          self.checkAccess(
-            graph, // The ACL graph
-            user, // The webId of the user
-            mode, // Read/Write/Append
-            resource, // The resource we want to access
-            accessType, // accessTo or defaultForNew
-            acl, // The current Acl file!
-            options
-          ).then(next, next)
-        })
-      },
-      function handleNoAccess (err) {
-        if (err === false || err === null) {
-          debug('No ACL resource found - access not allowed')
-          err = new Error('No Access Control Policy found')
-        }
-        if (err === true) {
-          debug('ACL policy found')
-          err = null
-        }
-        if (err) {
-          debug('Error: ' + err.message)
-          if (!user || user.length === 0) {
-            debug('Authentication required')
-            err.status = 401
-            err.message = 'Access to ' + resource + ' requires authorization'
+            reject(err)
           } else {
-            debug(mode + ' access denied for: ' + user)
-            err.status = 403
-            err.message = 'Access denied for ' + user
+            resolve({ acl, graph })
           }
-        }
-        return callback(err)
-      })
+        })
+      }))
+    }
+    nearestACL = nearestACL.catch(() => {
+      throw new Error('No ACL resource found')
+    })
+
+    // Check the permissions within the ACL
+    return nearestACL.then(({ acl, graph }) =>
+      this.checkAccess(
+        graph, // The ACL graph
+        user, // The webId of the user
+        mode, // Read/Write/Append
+        resource, // The resource we want to access
+        accessType, // accessTo or defaultForNew
+        acl, // The current Acl file!
+        options
+      )
+    )
+    .then(() => { debug('ACL policy found') })
+    .catch(err => {
+      debug(`Error: ${err.message}`)
+      if (!user) {
+        debug('Authentication required')
+        throw new HTTPError(401, `Access to ${resource} requires authorization`)
+      } else {
+        debug(`${mode} access denied for ${user}`)
+        throw new HTTPError(403, `Access denied for ${user}`)
+      }
+    })
   }
 
   /**
diff --git a/lib/handlers/allow.js b/lib/handlers/allow.js
index 6046453a8..d8be7727d 100644
--- a/lib/handlers/allow.js
+++ b/lib/handlers/allow.js
@@ -37,7 +37,8 @@ function allow (mode) {
         origin: req.get('origin'),
         host: req.protocol + '://' + req.get('host')
       }
-      return acl.can(req.session.userId, mode, baseUri + reqPath, next, options)
+      acl.can(req.session.userId, mode, baseUri + reqPath, options)
+      .then(next, next)
     })
   }
 }

From 4104c07b13c0c56080daf2b162f8d5b96285cb7e Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 10:12:35 -0400
Subject: [PATCH 03/12] Move getNearestACL into separate method.

---
 lib/acl-checker.js | 94 +++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 46 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 21caa2388..ee673aca1 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -24,29 +24,9 @@ class ACLChecker {
       mode = 'Control'
     }
 
-    // Find nearest ACL
-    let accessType = 'accessTo'
-    let nearestACL = Promise.reject()
-    for (const acl of ACLChecker.possibleACLs(resource, this.suffix)) {
-      nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
-        debug('Check if acl exist: ' + acl)
-        this.fetch(acl, function (err, graph) {
-          if (err || !graph || !graph.length) {
-            if (err) debug(`Error reading ${acl}: ${err}`)
-            accessType = 'defaultForNew'
-            reject(err)
-          } else {
-            resolve({ acl, graph })
-          }
-        })
-      }))
-    }
-    nearestACL = nearestACL.catch(() => {
-      throw new Error('No ACL resource found')
-    })
-
-    // Check the permissions within the ACL
-    return nearestACL.then(({ acl, graph }) =>
+    // Check the permissions within the nearest ACL
+    return this.getNearestACL(resource)
+    .then(({ acl, graph, accessType }) =>
       this.checkAccess(
         graph, // The ACL graph
         user, // The webId of the user
@@ -70,6 +50,51 @@ class ACLChecker {
     })
   }
 
+  // Gets the ACL that applies to the resource
+  getNearestACL (uri) {
+    let accessType = 'accessTo'
+    let nearestACL = Promise.reject()
+    for (const acl of this.getPossibleACLs(uri, this.suffix)) {
+      nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
+        this.debug(`Check if ACL exists: ${acl}`)
+        this.fetch(acl, (err, graph) => {
+          if (err || !graph || !graph.length) {
+            if (err) this.debug(`Error reading ${acl}: ${err}`)
+            accessType = 'defaultForNew'
+            reject(err)
+          } else {
+            resolve({ acl, graph, accessType })
+          }
+        })
+      }))
+    }
+    return nearestACL.catch(e => { throw new Error('No ACL resource found') })
+  }
+
+  // Get all possible ACL paths that apply to the resource
+  getPossibleACLs (uri, suffix) {
+    var first = uri.endsWith(suffix) ? uri : uri + suffix
+    var urls = [first]
+    var parsedUri = url.parse(uri)
+    var baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
+      (parsedUri.host || '')
+    if (baseUrl + '/' === uri) {
+      return urls
+    }
+
+    var times = parsedUri.pathname.split('/').length
+    // TODO: improve temporary solution to stop recursive path walking above root
+    if (parsedUri.pathname.endsWith('/')) {
+      times--
+    }
+
+    for (var i = 0; i < times - 1; i++) {
+      uri = path.dirname(uri)
+      urls.push(uri + (uri[uri.length - 1] === '/' ? suffix : '/' + suffix))
+    }
+    return urls
+  }
+
   /**
    * Tests whether a graph (parsed .acl resource) allows a given operation
    * for a given user. Calls the provided callback with `null` if the user
@@ -135,29 +160,6 @@ class ACLChecker {
       return false
     }
   }
-
-  static possibleACLs (uri, suffix) {
-    var first = uri.endsWith(suffix) ? uri : uri + suffix
-    var urls = [first]
-    var parsedUri = url.parse(uri)
-    var baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
-      (parsedUri.host || '')
-    if (baseUrl + '/' === uri) {
-      return urls
-    }
-
-    var times = parsedUri.pathname.split('/').length
-    // TODO: improve temporary solution to stop recursive path walking above root
-    if (parsedUri.pathname.endsWith('/')) {
-      times--
-    }
-
-    for (var i = 0; i < times - 1; i++) {
-      uri = path.dirname(uri)
-      urls.push(uri + (uri[uri.length - 1] === '/' ? suffix : '/' + suffix))
-    }
-    return urls
-  }
 }
 
 module.exports = ACLChecker

From b169010a5fafdc388e81edc55842e59c8f80a7ab Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 10:23:48 -0400
Subject: [PATCH 04/12] Move getPermissionSet into separate method.

---
 lib/acl-checker.js | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index ee673aca1..ac48aa798 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -111,22 +111,9 @@ class ACLChecker {
    * @param [options.host] Request's host URI (with protocol)
    */
   checkAccess (graph, user, mode, resource, accessType, acl, options = {}) {
-    if (!graph || graph.length === 0) {
-      debug('ACL ' + acl + ' is empty')
-      return Promise.reject(new Error('No policy found - empty ACL'))
-    }
-    let isContainer = accessType.startsWith('default')
-    let aclOptions = {
-      aclSuffix: this.suffix,
-      graph: graph,
-      host: options.host,
-      origin: options.origin,
-      rdf: rdf,
-      strictOrigin: this.strictOrigin,
-      isAcl: uri => this.isAcl(uri),
-      aclUrlFor: uri => this.aclUrlFor(uri)
-    }
-    let acls = new PermissionSet(resource, acl, isContainer, aclOptions)
+    const isContainer = accessType.startsWith('default')
+    const acls = this.getPermissionSet(graph, resource, isContainer, acl, options)
+
     return acls.checkAccess(resource, user, mode)
     .then(hasAccess => {
       if (hasAccess) {
@@ -134,7 +121,7 @@ class ACLChecker {
         return true
       } else {
         this.debug(`${mode} access NOT permitted to ${user}` +
-          aclOptions.strictOrigin ? ` and origin ${options.origin}` : '')
+          this.strictOrigin ? ` and origin ${options.origin}` : '')
         throw new Error('ACL file found but no matching policy found')
       }
     })
@@ -145,6 +132,26 @@ class ACLChecker {
     })
   }
 
+  // Gets the permission set for the given resource
+  getPermissionSet (graph, resource, isContainer, acl, options = {}) {
+    const debug = this.debug
+    if (!graph || graph.length === 0) {
+      debug('ACL ' + acl + ' is empty')
+      throw new Error('No policy found - empty ACL')
+    }
+    const aclOptions = {
+      aclSuffix: this.suffix,
+      graph: graph,
+      host: options.host,
+      origin: options.origin,
+      rdf: rdf,
+      strictOrigin: this.strictOrigin,
+      isAcl: uri => this.isAcl(uri),
+      aclUrlFor: uri => this.aclUrlFor(uri)
+    }
+    return new PermissionSet(resource, acl, isContainer, aclOptions)
+  }
+
   aclUrlFor (uri) {
     if (this.isAcl(uri)) {
       return uri

From fa23c82c89a50c0dadf4f13a254f052f141764a2 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 10:28:42 -0400
Subject: [PATCH 05/12] Change accessType into isContainer.

---
 lib/acl-checker.js            | 15 +++++++--------
 test/unit/acl-checker-test.js |  9 +++------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index ac48aa798..b5f32fad9 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -26,13 +26,13 @@ class ACLChecker {
 
     // Check the permissions within the nearest ACL
     return this.getNearestACL(resource)
-    .then(({ acl, graph, accessType }) =>
+    .then(({ acl, graph, isContainer }) =>
       this.checkAccess(
         graph, // The ACL graph
         user, // The webId of the user
         mode, // Read/Write/Append
         resource, // The resource we want to access
-        accessType, // accessTo or defaultForNew
+        isContainer,
         acl, // The current Acl file!
         options
       )
@@ -52,7 +52,7 @@ class ACLChecker {
 
   // Gets the ACL that applies to the resource
   getNearestACL (uri) {
-    let accessType = 'accessTo'
+    let isContainer = false
     let nearestACL = Promise.reject()
     for (const acl of this.getPossibleACLs(uri, this.suffix)) {
       nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
@@ -60,10 +60,10 @@ class ACLChecker {
         this.fetch(acl, (err, graph) => {
           if (err || !graph || !graph.length) {
             if (err) this.debug(`Error reading ${acl}: ${err}`)
-            accessType = 'defaultForNew'
+            isContainer = true
             reject(err)
           } else {
-            resolve({ acl, graph, accessType })
+            resolve({ acl, graph, isContainer })
           }
         })
       }))
@@ -104,14 +104,13 @@ class ACLChecker {
    * @param user {String} WebID URI of the user accessing the resource
    * @param mode {String} Access mode, e.g. 'Read', 'Write', etc.
    * @param resource {String} URI of the resource being accessed
-   * @param accessType {String} One of `accessTo`, or `default`
+   * @param isContainer {boolean}
    * @param acl {String} URI of this current .acl resource
    * @param options {Object} Options hashmap
    * @param [options.origin] Request's `Origin:` header
    * @param [options.host] Request's host URI (with protocol)
    */
-  checkAccess (graph, user, mode, resource, accessType, acl, options = {}) {
-    const isContainer = accessType.startsWith('default')
+  checkAccess (graph, user, mode, resource, isContainer, acl, options = {}) {
     const acls = this.getPermissionSet(graph, resource, isContainer, acl, options)
 
     return acls.checkAccess(resource, user, mode)
diff --git a/test/unit/acl-checker-test.js b/test/unit/acl-checker-test.js
index 3626c1daf..74c25b63f 100644
--- a/test/unit/acl-checker-test.js
+++ b/test/unit/acl-checker-test.js
@@ -27,10 +27,9 @@ describe('ACLChecker unit test', () => {
       'solid-permissions': { PermissionSet: PermissionSetAlwaysGrant }
     })
     let graph = {}
-    let accessType = ''
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    return expect(acl.checkAccess(graph, user, mode, resource, accessType, aclUrl))
+    return expect(acl.checkAccess(graph, user, mode, resource, true, aclUrl))
     .to.eventually.be.true
   })
   it('should callback with error on grant failure', () => {
@@ -38,10 +37,9 @@ describe('ACLChecker unit test', () => {
       'solid-permissions': { PermissionSet: PermissionSetNeverGrant }
     })
     let graph = {}
-    let accessType = ''
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    return expect(acl.checkAccess(graph, user, mode, resource, accessType, aclUrl))
+    return expect(acl.checkAccess(graph, user, mode, resource, true, aclUrl))
     .to.be.rejectedWith('ACL file found but no matching policy found')
   })
   it('should callback with error on grant error', () => {
@@ -49,10 +47,9 @@ describe('ACLChecker unit test', () => {
       'solid-permissions': { PermissionSet: PermissionSetAlwaysError }
     })
     let graph = {}
-    let accessType = ''
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    return expect(acl.checkAccess(graph, user, mode, resource, accessType, aclUrl))
+    return expect(acl.checkAccess(graph, user, mode, resource, true, aclUrl))
     .to.be.rejectedWith('Error thrown during checkAccess()')
   })
 })

From 4bb1b8f56a45257cd7d26518067dd272bb87f3c7 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 10:46:46 -0400
Subject: [PATCH 06/12] Pass permission set to checkAccess.

---
 lib/acl-checker.js            | 42 ++++++++---------------------------
 test/unit/acl-checker-test.js |  9 +++++---
 2 files changed, 15 insertions(+), 36 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index b5f32fad9..c090ce813 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -26,17 +26,10 @@ class ACLChecker {
 
     // Check the permissions within the nearest ACL
     return this.getNearestACL(resource)
-    .then(({ acl, graph, isContainer }) =>
-      this.checkAccess(
-        graph, // The ACL graph
-        user, // The webId of the user
-        mode, // Read/Write/Append
-        resource, // The resource we want to access
-        isContainer,
-        acl, // The current Acl file!
-        options
-      )
-    )
+    .then(nearestAcl => {
+      const acls = this.getPermissionSet(nearestAcl, resource, options)
+      return this.checkAccess(acls, user, mode, resource)
+    })
     .then(() => { debug('ACL policy found') })
     .catch(err => {
       debug(`Error: ${err.message}`)
@@ -95,32 +88,15 @@ class ACLChecker {
     return urls
   }
 
-  /**
-   * Tests whether a graph (parsed .acl resource) allows a given operation
-   * for a given user. Calls the provided callback with `null` if the user
-   * has access, otherwise calls it with an error.
-   * @method checkAccess
-   * @param graph {Graph} Parsed RDF graph of current .acl resource
-   * @param user {String} WebID URI of the user accessing the resource
-   * @param mode {String} Access mode, e.g. 'Read', 'Write', etc.
-   * @param resource {String} URI of the resource being accessed
-   * @param isContainer {boolean}
-   * @param acl {String} URI of this current .acl resource
-   * @param options {Object} Options hashmap
-   * @param [options.origin] Request's `Origin:` header
-   * @param [options.host] Request's host URI (with protocol)
-   */
-  checkAccess (graph, user, mode, resource, isContainer, acl, options = {}) {
-    const acls = this.getPermissionSet(graph, resource, isContainer, acl, options)
-
-    return acls.checkAccess(resource, user, mode)
+  // Tests whether the permissions allow a given operation
+  checkAccess (permissionSet, user, mode, resource) {
+    return permissionSet.checkAccess(resource, user, mode)
     .then(hasAccess => {
       if (hasAccess) {
         this.debug(`${mode} access permitted to ${user}`)
         return true
       } else {
-        this.debug(`${mode} access NOT permitted to ${user}` +
-          this.strictOrigin ? ` and origin ${options.origin}` : '')
+        this.debug(`${mode} access NOT permitted to ${user}`)
         throw new Error('ACL file found but no matching policy found')
       }
     })
@@ -132,7 +108,7 @@ class ACLChecker {
   }
 
   // Gets the permission set for the given resource
-  getPermissionSet (graph, resource, isContainer, acl, options = {}) {
+  getPermissionSet ({ acl, graph, isContainer }, resource, options = {}) {
     const debug = this.debug
     if (!graph || graph.length === 0) {
       debug('ACL ' + acl + ' is empty')
diff --git a/test/unit/acl-checker-test.js b/test/unit/acl-checker-test.js
index 74c25b63f..2bc67015e 100644
--- a/test/unit/acl-checker-test.js
+++ b/test/unit/acl-checker-test.js
@@ -29,7 +29,8 @@ describe('ACLChecker unit test', () => {
     let graph = {}
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    return expect(acl.checkAccess(graph, user, mode, resource, true, aclUrl))
+    let acls = acl.getPermissionSet({ graph, acl: aclUrl }, resource)
+    return expect(acl.checkAccess(acls, user, mode, resource))
     .to.eventually.be.true
   })
   it('should callback with error on grant failure', () => {
@@ -39,7 +40,8 @@ describe('ACLChecker unit test', () => {
     let graph = {}
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    return expect(acl.checkAccess(graph, user, mode, resource, true, aclUrl))
+    let acls = acl.getPermissionSet({ graph, acl: aclUrl }, resource)
+    return expect(acl.checkAccess(acls, user, mode, resource))
     .to.be.rejectedWith('ACL file found but no matching policy found')
   })
   it('should callback with error on grant error', () => {
@@ -49,7 +51,8 @@ describe('ACLChecker unit test', () => {
     let graph = {}
     let user, mode, resource, aclUrl
     let acl = new ACLChecker({ debug })
-    return expect(acl.checkAccess(graph, user, mode, resource, true, aclUrl))
+    let acls = acl.getPermissionSet({ graph, acl: aclUrl }, resource)
+    return expect(acl.checkAccess(acls, user, mode, resource))
     .to.be.rejectedWith('Error thrown during checkAccess()')
   })
 })

From 9a6bdf337c748d74ab79eca9e1028035d8386f7c Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 10:58:25 -0400
Subject: [PATCH 07/12] Move resource parameter to constructor.

---
 lib/acl-checker.js            | 33 ++++++++++++++++++---------------
 lib/handlers/allow.js         | 22 +++++++++++-----------
 test/unit/acl-checker-test.js | 18 +++++++++---------
 3 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index c090ce813..c27af6a1c 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -9,33 +9,34 @@ const HTTPError = require('./http-error')
 const DEFAULT_ACL_SUFFIX = '.acl'
 
 class ACLChecker {
-  constructor (options = {}) {
+  constructor (resource, options = {}) {
+    this.resource = resource
     this.debug = options.debug || console.log.bind(console)
     this.fetch = options.fetch
     this.strictOrigin = options.strictOrigin
     this.suffix = options.suffix || DEFAULT_ACL_SUFFIX
   }
 
-  can (user, mode, resource, options = {}) {
+  can (user, mode, options = {}) {
     const debug = this.debug
-    debug('Can ' + (user || 'an agent') + ' ' + mode + ' ' + resource + '?')
+    this.debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
     // If this is an ACL, Control mode must be present for any operations
-    if (this.isAcl(resource)) {
+    if (this.isAcl(this.resource)) {
       mode = 'Control'
     }
 
     // Check the permissions within the nearest ACL
-    return this.getNearestACL(resource)
+    return this.getNearestACL(this.resource)
     .then(nearestAcl => {
-      const acls = this.getPermissionSet(nearestAcl, resource, options)
-      return this.checkAccess(acls, user, mode, resource)
+      const acls = this.getPermissionSet(nearestAcl, options)
+      return this.checkAccess(acls, user, mode, this.resource)
     })
     .then(() => { debug('ACL policy found') })
     .catch(err => {
       debug(`Error: ${err.message}`)
       if (!user) {
         debug('Authentication required')
-        throw new HTTPError(401, `Access to ${resource} requires authorization`)
+        throw new HTTPError(401, `Access to ${this.resource} requires authorization`)
       } else {
         debug(`${mode} access denied for ${user}`)
         throw new HTTPError(403, `Access denied for ${user}`)
@@ -44,10 +45,10 @@ class ACLChecker {
   }
 
   // Gets the ACL that applies to the resource
-  getNearestACL (uri) {
+  getNearestACL () {
     let isContainer = false
     let nearestACL = Promise.reject()
-    for (const acl of this.getPossibleACLs(uri, this.suffix)) {
+    for (const acl of this.getPossibleACLs()) {
       nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
         this.debug(`Check if ACL exists: ${acl}`)
         this.fetch(acl, (err, graph) => {
@@ -65,7 +66,9 @@ class ACLChecker {
   }
 
   // Get all possible ACL paths that apply to the resource
-  getPossibleACLs (uri, suffix) {
+  getPossibleACLs () {
+    var uri = this.resource
+    var suffix = this.suffix
     var first = uri.endsWith(suffix) ? uri : uri + suffix
     var urls = [first]
     var parsedUri = url.parse(uri)
@@ -89,8 +92,8 @@ class ACLChecker {
   }
 
   // Tests whether the permissions allow a given operation
-  checkAccess (permissionSet, user, mode, resource) {
-    return permissionSet.checkAccess(resource, user, mode)
+  checkAccess (permissionSet, user, mode) {
+    return permissionSet.checkAccess(this.resource, user, mode)
     .then(hasAccess => {
       if (hasAccess) {
         this.debug(`${mode} access permitted to ${user}`)
@@ -108,7 +111,7 @@ class ACLChecker {
   }
 
   // Gets the permission set for the given resource
-  getPermissionSet ({ acl, graph, isContainer }, resource, options = {}) {
+  getPermissionSet ({ acl, graph, isContainer }, options = {}) {
     const debug = this.debug
     if (!graph || graph.length === 0) {
       debug('ACL ' + acl + ' is empty')
@@ -124,7 +127,7 @@ class ACLChecker {
       isAcl: uri => this.isAcl(uri),
       aclUrlFor: uri => this.aclUrlFor(uri)
     }
-    return new PermissionSet(resource, acl, isContainer, aclOptions)
+    return new PermissionSet(this.resource, acl, isContainer, aclOptions)
   }
 
   aclUrlFor (uri) {
diff --git a/lib/handlers/allow.js b/lib/handlers/allow.js
index d8be7727d..046c0e5ed 100644
--- a/lib/handlers/allow.js
+++ b/lib/handlers/allow.js
@@ -13,15 +13,6 @@ function allow (mode) {
     if (!ldp.webid) {
       return next()
     }
-    var baseUri = utils.uriBase(req)
-
-    var acl = new ACL({
-      debug: debug,
-      fetch: fetchDocument(req.hostname, ldp, baseUri),
-      suffix: ldp.suffixAcl,
-      strictOrigin: ldp.strictOrigin
-    })
-    req.acl = acl
 
     var reqPath = res && res.locals && res.locals.path
       ? res.locals.path
@@ -37,8 +28,17 @@ function allow (mode) {
         origin: req.get('origin'),
         host: req.protocol + '://' + req.get('host')
       }
-      acl.can(req.session.userId, mode, baseUri + reqPath, options)
-      .then(next, next)
+
+      var baseUri = utils.uriBase(req)
+      var acl = new ACL(baseUri + reqPath, {
+        debug: debug,
+        fetch: fetchDocument(req.hostname, ldp, baseUri),
+        suffix: ldp.suffixAcl,
+        strictOrigin: ldp.strictOrigin
+      })
+      req.acl = acl
+
+      acl.can(req.session.userId, mode, options).then(next, next)
     })
   }
 }
diff --git a/test/unit/acl-checker-test.js b/test/unit/acl-checker-test.js
index 2bc67015e..dc4cc4e4e 100644
--- a/test/unit/acl-checker-test.js
+++ b/test/unit/acl-checker-test.js
@@ -28,9 +28,9 @@ describe('ACLChecker unit test', () => {
     })
     let graph = {}
     let user, mode, resource, aclUrl
-    let acl = new ACLChecker({ debug })
-    let acls = acl.getPermissionSet({ graph, acl: aclUrl }, resource)
-    return expect(acl.checkAccess(acls, user, mode, resource))
+    let acl = new ACLChecker(resource, { debug })
+    let acls = acl.getPermissionSet({ graph, acl: aclUrl })
+    return expect(acl.checkAccess(acls, user, mode))
     .to.eventually.be.true
   })
   it('should callback with error on grant failure', () => {
@@ -39,9 +39,9 @@ describe('ACLChecker unit test', () => {
     })
     let graph = {}
     let user, mode, resource, aclUrl
-    let acl = new ACLChecker({ debug })
-    let acls = acl.getPermissionSet({ graph, acl: aclUrl }, resource)
-    return expect(acl.checkAccess(acls, user, mode, resource))
+    let acl = new ACLChecker(resource, { debug })
+    let acls = acl.getPermissionSet({ graph, acl: aclUrl })
+    return expect(acl.checkAccess(acls, user, mode))
     .to.be.rejectedWith('ACL file found but no matching policy found')
   })
   it('should callback with error on grant error', () => {
@@ -50,9 +50,9 @@ describe('ACLChecker unit test', () => {
     })
     let graph = {}
     let user, mode, resource, aclUrl
-    let acl = new ACLChecker({ debug })
-    let acls = acl.getPermissionSet({ graph, acl: aclUrl }, resource)
-    return expect(acl.checkAccess(acls, user, mode, resource))
+    let acl = new ACLChecker(resource, { debug })
+    let acls = acl.getPermissionSet({ graph, acl: aclUrl })
+    return expect(acl.checkAccess(acls, user, mode))
     .to.be.rejectedWith('Error thrown during checkAccess()')
   })
 })

From 4b2ac7a5a92e10cd4215cfb1d40359b73313e738 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 11:08:04 -0400
Subject: [PATCH 08/12] Move all options to constructor.

---
 lib/acl-checker.js    | 12 +++++++-----
 lib/handlers/allow.js |  8 +++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index c27af6a1c..8eb2bcbea 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -11,13 +11,15 @@ const DEFAULT_ACL_SUFFIX = '.acl'
 class ACLChecker {
   constructor (resource, options = {}) {
     this.resource = resource
+    this.host = options.host
+    this.origin = options.origin
     this.debug = options.debug || console.log.bind(console)
     this.fetch = options.fetch
     this.strictOrigin = options.strictOrigin
     this.suffix = options.suffix || DEFAULT_ACL_SUFFIX
   }
 
-  can (user, mode, options = {}) {
+  can (user, mode) {
     const debug = this.debug
     this.debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
     // If this is an ACL, Control mode must be present for any operations
@@ -28,7 +30,7 @@ class ACLChecker {
     // Check the permissions within the nearest ACL
     return this.getNearestACL(this.resource)
     .then(nearestAcl => {
-      const acls = this.getPermissionSet(nearestAcl, options)
+      const acls = this.getPermissionSet(nearestAcl)
       return this.checkAccess(acls, user, mode, this.resource)
     })
     .then(() => { debug('ACL policy found') })
@@ -111,7 +113,7 @@ class ACLChecker {
   }
 
   // Gets the permission set for the given resource
-  getPermissionSet ({ acl, graph, isContainer }, options = {}) {
+  getPermissionSet ({ acl, graph, isContainer }) {
     const debug = this.debug
     if (!graph || graph.length === 0) {
       debug('ACL ' + acl + ' is empty')
@@ -120,8 +122,8 @@ class ACLChecker {
     const aclOptions = {
       aclSuffix: this.suffix,
       graph: graph,
-      host: options.host,
-      origin: options.origin,
+      host: this.host,
+      origin: this.origin,
       rdf: rdf,
       strictOrigin: this.strictOrigin,
       isAcl: uri => this.isAcl(uri),
diff --git a/lib/handlers/allow.js b/lib/handlers/allow.js
index 046c0e5ed..c1bfa7468 100644
--- a/lib/handlers/allow.js
+++ b/lib/handlers/allow.js
@@ -24,13 +24,11 @@ function allow (mode) {
       if (!reqPath.endsWith('/') && !err && stat.isDirectory()) {
         reqPath += '/'
       }
-      var options = {
-        origin: req.get('origin'),
-        host: req.protocol + '://' + req.get('host')
-      }
 
       var baseUri = utils.uriBase(req)
       var acl = new ACL(baseUri + reqPath, {
+        origin: req.get('origin'),
+        host: req.protocol + '://' + req.get('host'),
         debug: debug,
         fetch: fetchDocument(req.hostname, ldp, baseUri),
         suffix: ldp.suffixAcl,
@@ -38,7 +36,7 @@ function allow (mode) {
       })
       req.acl = acl
 
-      acl.can(req.session.userId, mode, options).then(next, next)
+      acl.can(req.session.userId, mode).then(next, next)
     })
   }
 }

From a086afd933337045ef44863c8e07c585643cefa3 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 11:18:41 -0400
Subject: [PATCH 09/12] Cache the permission set.

---
 lib/acl-checker.js | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 8eb2bcbea..90b1164b7 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -19,29 +19,32 @@ class ACLChecker {
     this.suffix = options.suffix || DEFAULT_ACL_SUFFIX
   }
 
+  // Returns a fulfilled promise when the user can access the resource
+  // in the given mode, or a rejected promise otherwise
   can (user, mode) {
-    const debug = this.debug
     this.debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
     // If this is an ACL, Control mode must be present for any operations
     if (this.isAcl(this.resource)) {
       mode = 'Control'
     }
 
-    // Check the permissions within the nearest ACL
-    return this.getNearestACL(this.resource)
-    .then(nearestAcl => {
-      const acls = this.getPermissionSet(nearestAcl)
-      return this.checkAccess(acls, user, mode, this.resource)
-    })
-    .then(() => { debug('ACL policy found') })
+    // Obtain the permission set for the resource
+    if (!this._permissionSet) {
+      this._permissionSet = this.getNearestACL()
+        .then(acl => this.getPermissionSet(acl))
+    }
+
+    // Check the permissions
+    return this._permissionSet.then(acls => this.checkAccess(acls, user, mode))
+    .then(() => { this.debug('ACL policy found') })
     .catch(err => {
-      debug(`Error: ${err.message}`)
+      this.debug(`Error: ${err.message}`)
       if (!user) {
-        debug('Authentication required')
+        this.debug('Authentication required')
         throw new HTTPError(401, `Access to ${this.resource} requires authorization`)
       } else {
-        debug(`${mode} access denied for ${user}`)
-        throw new HTTPError(403, `Access denied for ${user}`)
+        this.debug(`${mode} access denied for ${user}`)
+        throw new HTTPError(403, `Access to ${this.resource} denied for ${user}`)
       }
     })
   }
@@ -112,7 +115,7 @@ class ACLChecker {
     })
   }
 
-  // Gets the permission set for the given resource
+  // Gets the permission set for the given ACL
   getPermissionSet ({ acl, graph, isContainer }) {
     const debug = this.debug
     if (!graph || graph.length === 0) {

From 5adfedb4123d999587e614c1b20bee77b25c5415 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 13:13:19 -0400
Subject: [PATCH 10/12] Clean up ACLChecker.

---
 lib/acl-checker.js            | 60 +++++++++------------
 lib/handlers/allow.js         | 22 ++++----
 package-lock.json             | 41 ---------------
 package.json                  |  1 -
 test/unit/acl-checker-test.js | 99 +++++++++++++++++++----------------
 5 files changed, 91 insertions(+), 132 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 90b1164b7..2dab03aa6 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -4,25 +4,26 @@ const path = require('path')
 const PermissionSet = require('solid-permissions').PermissionSet
 const rdf = require('rdflib')
 const url = require('url')
+const debug = require('./debug').ACL
 const HTTPError = require('./http-error')
 
 const DEFAULT_ACL_SUFFIX = '.acl'
 
+// An ACLChecker exposes the permissions on a specific resource
 class ACLChecker {
   constructor (resource, options = {}) {
     this.resource = resource
     this.host = options.host
     this.origin = options.origin
-    this.debug = options.debug || console.log.bind(console)
     this.fetch = options.fetch
     this.strictOrigin = options.strictOrigin
     this.suffix = options.suffix || DEFAULT_ACL_SUFFIX
   }
 
   // Returns a fulfilled promise when the user can access the resource
-  // in the given mode, or a rejected promise otherwise
+  // in the given mode, or rejects with an HTTP error otherwise
   can (user, mode) {
-    this.debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
+    debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
     // If this is an ACL, Control mode must be present for any operations
     if (this.isAcl(this.resource)) {
       mode = 'Control'
@@ -34,16 +35,15 @@ class ACLChecker {
         .then(acl => this.getPermissionSet(acl))
     }
 
-    // Check the permissions
+    // Check the resource's permissions
     return this._permissionSet.then(acls => this.checkAccess(acls, user, mode))
-    .then(() => { this.debug('ACL policy found') })
     .catch(err => {
-      this.debug(`Error: ${err.message}`)
+      debug(`Error: ${err.message}`)
       if (!user) {
-        this.debug('Authentication required')
+        debug('Authentication required')
         throw new HTTPError(401, `Access to ${this.resource} requires authorization`)
       } else {
-        this.debug(`${mode} access denied for ${user}`)
+        debug(`${mode} access denied for ${user}`)
         throw new HTTPError(403, `Access to ${this.resource} denied for ${user}`)
       }
     })
@@ -52,13 +52,14 @@ class ACLChecker {
   // Gets the ACL that applies to the resource
   getNearestACL () {
     let isContainer = false
+    // Create a cascade of reject handlers (one for each possible ACL)
     let nearestACL = Promise.reject()
     for (const acl of this.getPossibleACLs()) {
       nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
-        this.debug(`Check if ACL exists: ${acl}`)
+        debug(`Check if ACL exists: ${acl}`)
         this.fetch(acl, (err, graph) => {
           if (err || !graph || !graph.length) {
-            if (err) this.debug(`Error reading ${acl}: ${err}`)
+            if (err) debug(`Error reading ${acl}: ${err}`)
             isContainer = true
             reject(err)
           } else {
@@ -70,26 +71,26 @@ class ACLChecker {
     return nearestACL.catch(e => { throw new Error('No ACL resource found') })
   }
 
-  // Get all possible ACL paths that apply to the resource
+  // Gets all possible ACL paths that apply to the resource
   getPossibleACLs () {
-    var uri = this.resource
-    var suffix = this.suffix
-    var first = uri.endsWith(suffix) ? uri : uri + suffix
-    var urls = [first]
-    var parsedUri = url.parse(uri)
-    var baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
+    let uri = this.resource
+    const suffix = this.suffix
+    const first = uri.endsWith(suffix) ? uri : uri + suffix
+    const urls = [first]
+    const parsedUri = url.parse(uri)
+    const baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
       (parsedUri.host || '')
     if (baseUrl + '/' === uri) {
       return urls
     }
 
-    var times = parsedUri.pathname.split('/').length
+    let times = parsedUri.pathname.split('/').length
     // TODO: improve temporary solution to stop recursive path walking above root
     if (parsedUri.pathname.endsWith('/')) {
       times--
     }
 
-    for (var i = 0; i < times - 1; i++) {
+    for (let i = 0; i < times - 1; i++) {
       uri = path.dirname(uri)
       urls.push(uri + (uri[uri.length - 1] === '/' ? suffix : '/' + suffix))
     }
@@ -101,23 +102,22 @@ class ACLChecker {
     return permissionSet.checkAccess(this.resource, user, mode)
     .then(hasAccess => {
       if (hasAccess) {
-        this.debug(`${mode} access permitted to ${user}`)
+        debug(`${mode} access permitted to ${user}`)
         return true
       } else {
-        this.debug(`${mode} access NOT permitted to ${user}`)
+        debug(`${mode} access NOT permitted to ${user}`)
         throw new Error('ACL file found but no matching policy found')
       }
     })
     .catch(err => {
-      this.debug(`${mode} access denied to ${user}`)
-      this.debug(err)
+      debug(`${mode} access denied to ${user}`)
+      debug(err)
       throw err
     })
   }
 
   // Gets the permission set for the given ACL
   getPermissionSet ({ acl, graph, isContainer }) {
-    const debug = this.debug
     if (!graph || graph.length === 0) {
       debug('ACL ' + acl + ' is empty')
       throw new Error('No policy found - empty ACL')
@@ -136,19 +136,11 @@ class ACLChecker {
   }
 
   aclUrlFor (uri) {
-    if (this.isAcl(uri)) {
-      return uri
-    } else {
-      return uri + this.suffix
-    }
+    return this.isAcl(uri) ? uri : uri + this.suffix
   }
 
   isAcl (resource) {
-    if (typeof resource === 'string') {
-      return resource.endsWith(this.suffix)
-    } else {
-      return false
-    }
+    return resource.endsWith(this.suffix)
   }
 }
 
diff --git a/lib/handlers/allow.js b/lib/handlers/allow.js
index c1bfa7468..9dec5f4d5 100644
--- a/lib/handlers/allow.js
+++ b/lib/handlers/allow.js
@@ -4,7 +4,6 @@ var ACL = require('../acl-checker')
 var $rdf = require('rdflib')
 var url = require('url')
 var async = require('async')
-var debug = require('../debug').ACL
 var utils = require('../utils')
 
 function allow (mode) {
@@ -14,29 +13,32 @@ function allow (mode) {
       return next()
     }
 
+    // Determine the actual path of the request
     var reqPath = res && res.locals && res.locals.path
       ? res.locals.path
       : req.path
+
+    // Check whether the resource exists
     ldp.exists(req.hostname, reqPath, (err, ret) => {
-      if (ret) {
-        var stat = ret.stream
-      }
-      if (!reqPath.endsWith('/') && !err && stat.isDirectory()) {
+      // Ensure directories always end in a slash
+      const stat = err ? null : ret.stream
+      if (!reqPath.endsWith('/') && stat && stat.isDirectory()) {
         reqPath += '/'
       }
 
-      var baseUri = utils.uriBase(req)
-      var acl = new ACL(baseUri + reqPath, {
+      // Obtain and store the ACL of the requested resource
+      const baseUri = utils.uriBase(req)
+      req.acl = new ACL(baseUri + reqPath, {
         origin: req.get('origin'),
         host: req.protocol + '://' + req.get('host'),
-        debug: debug,
         fetch: fetchDocument(req.hostname, ldp, baseUri),
         suffix: ldp.suffixAcl,
         strictOrigin: ldp.strictOrigin
       })
-      req.acl = acl
 
-      acl.can(req.session.userId, mode).then(next, next)
+      // Ensure the user has the required permission
+      req.acl.can(req.session.userId, mode)
+             .then(() => next(), next)
     })
   }
 }
diff --git a/package-lock.json b/package-lock.json
index 6df92ea1d..4e6f44dff 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1864,16 +1864,6 @@
       "resolved": "https://registry.npmjs.org/filename-regex/-/filename-regex-2.0.1.tgz",
       "integrity": "sha1-wcS5vuPglyXdsQa3XB4wH+LxiyY="
     },
-    "fill-keys": {
-      "version": "1.0.2",
-      "resolved": "https://registry.npmjs.org/fill-keys/-/fill-keys-1.0.2.tgz",
-      "integrity": "sha1-mo+jb06K1jTjv2tPPIiCVRRS6yA=",
-      "dev": true,
-      "requires": {
-        "is-object": "1.0.1",
-        "merge-descriptors": "1.0.1"
-      }
-    },
     "fill-range": {
       "version": "2.2.3",
       "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-2.2.3.tgz",
@@ -2678,12 +2668,6 @@
         "kind-of": "3.2.2"
       }
     },
-    "is-object": {
-      "version": "1.0.1",
-      "resolved": "https://registry.npmjs.org/is-object/-/is-object-1.0.1.tgz",
-      "integrity": "sha1-iVJojF7C/9awPsyF52ngKQMINHA=",
-      "dev": true
-    },
     "is-path-cwd": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/is-path-cwd/-/is-path-cwd-1.0.0.tgz",
@@ -3282,12 +3266,6 @@
         }
       }
     },
-    "module-not-found-error": {
-      "version": "1.0.1",
-      "resolved": "https://registry.npmjs.org/module-not-found-error/-/module-not-found-error-1.0.1.tgz",
-      "integrity": "sha1-z4tP9PKWQGdNbN0CsOO8UjwrvcA=",
-      "dev": true
-    },
     "moment": {
       "version": "2.18.1",
       "resolved": "https://registry.npmjs.org/moment/-/moment-2.18.1.tgz",
@@ -5258,25 +5236,6 @@
         "ipaddr.js": "1.4.0"
       }
     },
-    "proxyquire": {
-      "version": "1.8.0",
-      "resolved": "https://registry.npmjs.org/proxyquire/-/proxyquire-1.8.0.tgz",
-      "integrity": "sha1-AtUUpb7ZhvBMuyCTrxZ0FTX3ntw=",
-      "dev": true,
-      "requires": {
-        "fill-keys": "1.0.2",
-        "module-not-found-error": "1.0.1",
-        "resolve": "1.1.7"
-      },
-      "dependencies": {
-        "resolve": {
-          "version": "1.1.7",
-          "resolved": "https://registry.npmjs.org/resolve/-/resolve-1.1.7.tgz",
-          "integrity": "sha1-IDEU2CrSxe2ejgQRs5ModeiJ6Xs=",
-          "dev": true
-        }
-      }
-    },
     "public-encrypt": {
       "version": "4.0.0",
       "resolved": "https://registry.npmjs.org/public-encrypt/-/public-encrypt-4.0.0.tgz",
diff --git a/package.json b/package.json
index 2a5a2ba8b..e4ea0d143 100644
--- a/package.json
+++ b/package.json
@@ -90,7 +90,6 @@
     "nock": "^9.0.14",
     "node-mocks-http": "^1.5.6",
     "nyc": "^10.1.2",
-    "proxyquire": "^1.7.10",
     "sinon": "^2.1.0",
     "sinon-chai": "^2.8.0",
     "solid-auth-oidc": "^0.2.0",
diff --git a/test/unit/acl-checker-test.js b/test/unit/acl-checker-test.js
index dc4cc4e4e..316227f2d 100644
--- a/test/unit/acl-checker-test.js
+++ b/test/unit/acl-checker-test.js
@@ -1,58 +1,65 @@
 'use strict'
-const proxyquire = require('proxyquire')
-const debug = require('../../lib/debug').ACL
+const ACLChecker = require('../../lib/acl-checker')
 const chai = require('chai')
 const { expect } = chai
 chai.use(require('chai-as-promised'))
 
-class PermissionSetAlwaysGrant {
-  checkAccess () {
-    return Promise.resolve(true)
-  }
-}
-class PermissionSetNeverGrant {
-  checkAccess () {
-    return Promise.resolve(false)
-  }
-}
-class PermissionSetAlwaysError {
-  checkAccess () {
-    return Promise.reject(new Error('Error thrown during checkAccess()'))
-  }
-}
-
 describe('ACLChecker unit test', () => {
-  it('should callback with null on grant success', () => {
-    let ACLChecker = proxyquire('../../lib/acl-checker', {
-      'solid-permissions': { PermissionSet: PermissionSetAlwaysGrant }
+  describe('checkAccess', () => {
+    it('should callback with null on grant success', () => {
+      let acl = new ACLChecker()
+      let acls = { checkAccess: () => Promise.resolve(true) }
+      return expect(acl.checkAccess(acls)).to.eventually.be.true
     })
-    let graph = {}
-    let user, mode, resource, aclUrl
-    let acl = new ACLChecker(resource, { debug })
-    let acls = acl.getPermissionSet({ graph, acl: aclUrl })
-    return expect(acl.checkAccess(acls, user, mode))
-    .to.eventually.be.true
-  })
-  it('should callback with error on grant failure', () => {
-    let ACLChecker = proxyquire('../../lib/acl-checker', {
-      'solid-permissions': { PermissionSet: PermissionSetNeverGrant }
+    it('should callback with error on grant failure', () => {
+      let acl = new ACLChecker()
+      let acls = { checkAccess: () => Promise.resolve(false) }
+      return expect(acl.checkAccess(acls))
+      .to.be.rejectedWith('ACL file found but no matching policy found')
+    })
+    it('should callback with error on grant error', () => {
+      let acl = new ACLChecker()
+      let acls = { checkAccess: () => Promise.reject(new Error('my error')) }
+      return expect(acl.checkAccess(acls)).to.be.rejectedWith('my error')
     })
-    let graph = {}
-    let user, mode, resource, aclUrl
-    let acl = new ACLChecker(resource, { debug })
-    let acls = acl.getPermissionSet({ graph, acl: aclUrl })
-    return expect(acl.checkAccess(acls, user, mode))
-    .to.be.rejectedWith('ACL file found but no matching policy found')
   })
-  it('should callback with error on grant error', () => {
-    let ACLChecker = proxyquire('../../lib/acl-checker', {
-      'solid-permissions': { PermissionSet: PermissionSetAlwaysError }
+
+  describe('getPossibleACLs', () => {
+    it('returns all possible ACLs of the root', () => {
+      const aclChecker = new ACLChecker('http://ex.org/')
+      expect(aclChecker.getPossibleACLs()).to.deep.equal([
+        'http://ex.org/.acl'
+      ])
+    })
+
+    it('returns all possible ACLs of a regular file', () => {
+      const aclChecker = new ACLChecker('http://ex.org/abc/def/ghi')
+      expect(aclChecker.getPossibleACLs()).to.deep.equal([
+        'http://ex.org/abc/def/ghi.acl',
+        'http://ex.org/abc/def/.acl',
+        'http://ex.org/abc/.acl',
+        'http://ex.org/.acl'
+      ])
+    })
+
+    it('returns all possible ACLs of an ACL file', () => {
+      const aclChecker = new ACLChecker('http://ex.org/abc/def/ghi.acl')
+      expect(aclChecker.getPossibleACLs()).to.deep.equal([
+        'http://ex.org/abc/def/ghi.acl',
+        'http://ex.org/abc/def/.acl',
+        'http://ex.org/abc/.acl',
+        'http://ex.org/.acl'
+      ])
+    })
+
+    it('returns all possible ACLs of a directory', () => {
+      const aclChecker = new ACLChecker('http://ex.org/abc/def/ghi/')
+      expect(aclChecker.getPossibleACLs()).to.deep.equal([
+        'http://ex.org/abc/def/ghi/.acl',
+        'http://ex.org/abc/def/.acl',
+        'http://ex.org/abc/.acl',
+        'http://ex.org/.acl'
+      ])
     })
-    let graph = {}
-    let user, mode, resource, aclUrl
-    let acl = new ACLChecker(resource, { debug })
-    let acls = acl.getPermissionSet({ graph, acl: aclUrl })
-    return expect(acl.checkAccess(acls, user, mode))
-    .to.be.rejectedWith('Error thrown during checkAccess()')
   })
 })

From 2e81b07cc605842f4f5ad5ce90df4a8424ecd616 Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 13:46:17 -0400
Subject: [PATCH 11/12] Simplify ACL path algorithm.

---
 lib/acl-checker.js | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index 2dab03aa6..da221d04d 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -1,9 +1,7 @@
 'use strict'
 
-const path = require('path')
 const PermissionSet = require('solid-permissions').PermissionSet
 const rdf = require('rdflib')
-const url = require('url')
 const debug = require('./debug').ACL
 const HTTPError = require('./http-error')
 
@@ -73,28 +71,21 @@ class ACLChecker {
 
   // Gets all possible ACL paths that apply to the resource
   getPossibleACLs () {
-    let uri = this.resource
-    const suffix = this.suffix
-    const first = uri.endsWith(suffix) ? uri : uri + suffix
-    const urls = [first]
-    const parsedUri = url.parse(uri)
-    const baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
-      (parsedUri.host || '')
-    if (baseUrl + '/' === uri) {
-      return urls
-    }
+    // Obtain the resource URI and the length of its base
+    let { resource: uri, suffix } = this
+    const [ { length: base } ] = uri.match(/^[^:]+:\/*[^/]+/)
 
-    let times = parsedUri.pathname.split('/').length
-    // TODO: improve temporary solution to stop recursive path walking above root
-    if (parsedUri.pathname.endsWith('/')) {
-      times--
+    // If the URI points to a file, append the file's ACL
+    const possibleAcls = []
+    if (!uri.endsWith('/')) {
+      possibleAcls.push(uri.endsWith(suffix) ? uri : uri + suffix)
     }
 
-    for (let i = 0; i < times - 1; i++) {
-      uri = path.dirname(uri)
-      urls.push(uri + (uri[uri.length - 1] === '/' ? suffix : '/' + suffix))
+    // Append the ACLs of all parent directories
+    for (let i = lastSlash(uri); i >= base; i = lastSlash(uri, i - 1)) {
+      possibleAcls.push(uri.substr(0, i + 1) + suffix)
     }
-    return urls
+    return possibleAcls
   }
 
   // Tests whether the permissions allow a given operation
@@ -144,5 +135,10 @@ class ACLChecker {
   }
 }
 
+// Returns the index of the last slash before the given position
+function lastSlash (string, pos = string.length) {
+  return string.lastIndexOf('/', pos)
+}
+
 module.exports = ACLChecker
 module.exports.DEFAULT_ACL_SUFFIX = DEFAULT_ACL_SUFFIX

From bda742636e346fff02f6f082b10d37bbd524013b Mon Sep 17 00:00:00 2001
From: Ruben Verborgh <ruben@verborgh.org>
Date: Fri, 18 Aug 2017 14:47:20 -0400
Subject: [PATCH 12/12] Indent then and catch.

---
 lib/acl-checker.js | 51 +++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/lib/acl-checker.js b/lib/acl-checker.js
index da221d04d..4aef8dfa8 100644
--- a/lib/acl-checker.js
+++ b/lib/acl-checker.js
@@ -34,17 +34,18 @@ class ACLChecker {
     }
 
     // Check the resource's permissions
-    return this._permissionSet.then(acls => this.checkAccess(acls, user, mode))
-    .catch(err => {
-      debug(`Error: ${err.message}`)
-      if (!user) {
-        debug('Authentication required')
-        throw new HTTPError(401, `Access to ${this.resource} requires authorization`)
-      } else {
-        debug(`${mode} access denied for ${user}`)
-        throw new HTTPError(403, `Access to ${this.resource} denied for ${user}`)
-      }
-    })
+    return this._permissionSet
+      .then(acls => this.checkAccess(acls, user, mode))
+      .catch(err => {
+        debug(`Error: ${err.message}`)
+        if (!user) {
+          debug('Authentication required')
+          throw new HTTPError(401, `Access to ${this.resource} requires authorization`)
+        } else {
+          debug(`${mode} access denied for ${user}`)
+          throw new HTTPError(403, `Access to ${this.resource} denied for ${user}`)
+        }
+      })
   }
 
   // Gets the ACL that applies to the resource
@@ -91,20 +92,20 @@ class ACLChecker {
   // Tests whether the permissions allow a given operation
   checkAccess (permissionSet, user, mode) {
     return permissionSet.checkAccess(this.resource, user, mode)
-    .then(hasAccess => {
-      if (hasAccess) {
-        debug(`${mode} access permitted to ${user}`)
-        return true
-      } else {
-        debug(`${mode} access NOT permitted to ${user}`)
-        throw new Error('ACL file found but no matching policy found')
-      }
-    })
-    .catch(err => {
-      debug(`${mode} access denied to ${user}`)
-      debug(err)
-      throw err
-    })
+      .then(hasAccess => {
+        if (hasAccess) {
+          debug(`${mode} access permitted to ${user}`)
+          return true
+        } else {
+          debug(`${mode} access NOT permitted to ${user}`)
+          throw new Error('ACL file found but no matching policy found')
+        }
+      })
+      .catch(err => {
+        debug(`${mode} access denied to ${user}`)
+        debug(err)
+        throw err
+      })
   }
 
   // Gets the permission set for the given ACL