Skip to content

Commit 3861c5b

Browse files
RubenVerborghRubenVerborgh
committed
Clean up ACLChecker.
1 parent a086afd commit 3861c5b

File tree

5 files changed

+79
-123
lines changed

5 files changed

+79
-123
lines changed

‎lib/acl-checker.js‎

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,17 @@ const path = require('path')
44
const PermissionSet = require('solid-permissions').PermissionSet
55
const rdf = require('rdflib')
66
const url = require('url')
7+
const debug = require('./debug').ACL
78
const HTTPError = require('./http-error')
89

910
const DEFAULT_ACL_SUFFIX = '.acl'
1011

12+
// An ACLChecker exposes the permissions on a specific resource
1113
class ACLChecker {
1214
constructor (resource, options = {}) {
1315
this.resource = resource
1416
this.host = options.host
1517
this.origin = options.origin
16-
this.debug = options.debug || console.log.bind(console)
1718
this.fetch = options.fetch
1819
this.strictOrigin = options.strictOrigin
1920
this.suffix = options.suffix || DEFAULT_ACL_SUFFIX
@@ -22,7 +23,7 @@ class ACLChecker {
2223
// Returns a fulfilled promise when the user can access the resource
2324
// in the given mode, or a rejected promise otherwise
2425
can (user, mode) {
25-
this.debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
26+
debug(`Can ${user || 'an agent'} ${mode} ${this.resource}?`)
2627
// If this is an ACL, Control mode must be present for any operations
2728
if (this.isAcl(this.resource)) {
2829
mode = 'Control'
@@ -34,16 +35,16 @@ class ACLChecker {
3435
.then(acl => this.getPermissionSet(acl))
3536
}
3637

37-
// Check the permissions
38+
// Check the resource's permissions
3839
return this._permissionSet.then(acls => this.checkAccess(acls, user, mode))
39-
.then(() => { this.debug('ACL policy found') })
40+
.then(() => { debug('ACL policy found') })
4041
.catch(err => {
41-
this.debug(`Error: ${err.message}`)
42+
debug(`Error: ${err.message}`)
4243
if (!user) {
43-
this.debug('Authentication required')
44+
debug('Authentication required')
4445
throw new HTTPError(401, `Access to ${this.resource} requires authorization`)
4546
} else {
46-
this.debug(`${mode} access denied for ${user}`)
47+
debug(`${mode} access denied for ${user}`)
4748
throw new HTTPError(403, `Access to ${this.resource} denied for ${user}`)
4849
}
4950
})
@@ -52,13 +53,14 @@ class ACLChecker {
5253
// Gets the ACL that applies to the resource
5354
getNearestACL () {
5455
let isContainer = false
56+
// Create a cascade of reject handlers (one for each possible ACL)
5557
let nearestACL = Promise.reject()
5658
for (const acl of this.getPossibleACLs()) {
5759
nearestACL = nearestACL.catch(() => new Promise((resolve, reject) => {
58-
this.debug(`Check if ACL exists: ${acl}`)
60+
debug(`Check if ACL exists: ${acl}`)
5961
this.fetch(acl, (err, graph) => {
6062
if (err || !graph || !graph.length) {
61-
if (err) this.debug(`Error reading ${acl}: ${err}`)
63+
if (err) debug(`Error reading ${acl}: ${err}`)
6264
isContainer = true
6365
reject(err)
6466
} else {
@@ -70,26 +72,26 @@ class ACLChecker {
7072
return nearestACL.catch(e => { throw new Error('No ACL resource found') })
7173
}
7274

73-
// Get all possible ACL paths that apply to the resource
75+
// Gets all possible ACL paths that apply to the resource
7476
getPossibleACLs () {
75-
var uri = this.resource
76-
var suffix = this.suffix
77-
var first = uri.endsWith(suffix) ? uri : uri + suffix
78-
var urls = [first]
79-
var parsedUri = url.parse(uri)
80-
var baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
77+
let uri = this.resource
78+
const suffix = this.suffix
79+
const first = uri.endsWith(suffix) ? uri : uri + suffix
80+
const urls = [first]
81+
const parsedUri = url.parse(uri)
82+
const baseUrl = (parsedUri.protocol ? parsedUri.protocol + '//' : '') +
8183
(parsedUri.host || '')
8284
if (baseUrl + '/' === uri) {
8385
return urls
8486
}
8587

86-
var times = parsedUri.pathname.split('/').length
88+
let times = parsedUri.pathname.split('/').length
8789
// TODO: improve temporary solution to stop recursive path walking above root
8890
if (parsedUri.pathname.endsWith('/')) {
8991
times--
9092
}
9193

92-
for (var i = 0; i < times - 1; i++) {
94+
for (let i = 0; i < times - 1; i++) {
9395
uri = path.dirname(uri)
9496
urls.push(uri + (uri[uri.length - 1] === '/' ? suffix : '/' + suffix))
9597
}
@@ -101,23 +103,22 @@ class ACLChecker {
101103
return permissionSet.checkAccess(this.resource, user, mode)
102104
.then(hasAccess => {
103105
if (hasAccess) {
104-
this.debug(`${mode} access permitted to ${user}`)
106+
debug(`${mode} access permitted to ${user}`)
105107
return true
106108
} else {
107-
this.debug(`${mode} access NOT permitted to ${user}`)
109+
debug(`${mode} access NOT permitted to ${user}`)
108110
throw new Error('ACL file found but no matching policy found')
109111
}
110112
})
111113
.catch(err => {
112-
this.debug(`${mode} access denied to ${user}`)
113-
this.debug(err)
114+
debug(`${mode} access denied to ${user}`)
115+
debug(err)
114116
throw err
115117
})
116118
}
117119

118120
// Gets the permission set for the given ACL
119121
getPermissionSet ({ acl, graph, isContainer }) {
120-
const debug = this.debug
121122
if (!graph || graph.length === 0) {
122123
debug('ACL ' + acl + ' is empty')
123124
throw new Error('No policy found - empty ACL')
@@ -136,19 +137,11 @@ class ACLChecker {
136137
}
137138

138139
aclUrlFor (uri) {
139-
if (this.isAcl(uri)) {
140-
return uri
141-
} else {
142-
return uri + this.suffix
143-
}
140+
return this.isAcl(uri) ? uri : uri + this.suffix
144141
}
145142

146143
isAcl (resource) {
147-
if (typeof resource === 'string') {
148-
return resource.endsWith(this.suffix)
149-
} else {
150-
return false
151-
}
144+
return resource.endsWith(this.suffix)
152145
}
153146
}
154147

‎lib/handlers/allow.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ var ACL = require('../acl-checker')
44
var $rdf = require('rdflib')
55
var url = require('url')
66
var async = require('async')
7-
var debug = require('../debug').ACL
87
var utils = require('../utils')
98

109
function allow (mode) {
@@ -29,7 +28,6 @@ function allow (mode) {
2928
var acl = new ACL(baseUri + reqPath, {
3029
origin: req.get('origin'),
3130
host: req.protocol + '://' + req.get('host'),
32-
debug: debug,
3331
fetch: fetchDocument(req.hostname, ldp, baseUri),
3432
suffix: ldp.suffixAcl,
3533
strictOrigin: ldp.strictOrigin

‎package-lock.json‎

Lines changed: 0 additions & 41 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@
9090
"nock": "^9.0.14",
9191
"node-mocks-http": "^1.5.6",
9292
"nyc": "^10.1.2",
93-
"proxyquire": "^1.7.10",
9493
"sinon": "^2.1.0",
9594
"sinon-chai": "^2.8.0",
9695
"solid-auth-oidc": "^0.2.0",

‎test/unit/acl-checker-test.js‎

Lines changed: 53 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,65 @@
11
'use strict'
2-
const proxyquire = require('proxyquire')
3-
const debug = require('../../lib/debug').ACL
2+
const ACLChecker = require('../../lib/acl-checker')
43
const chai = require('chai')
54
const { expect } = chai
65
chai.use(require('chai-as-promised'))
76

8-
class PermissionSetAlwaysGrant {
9-
checkAccess () {
10-
return Promise.resolve(true)
11-
}
12-
}
13-
class PermissionSetNeverGrant {
14-
checkAccess () {
15-
return Promise.resolve(false)
16-
}
17-
}
18-
class PermissionSetAlwaysError {
19-
checkAccess () {
20-
return Promise.reject(new Error('Error thrown during checkAccess()'))
21-
}
22-
}
23-
247
describe('ACLChecker unit test', () => {
25-
it('should callback with null on grant success', () => {
26-
let ACLChecker = proxyquire('../../lib/acl-checker', {
27-
'solid-permissions': { PermissionSet: PermissionSetAlwaysGrant }
8+
describe('checkAccess', () => {
9+
it('should callback with null on grant success', () => {
10+
let acl = new ACLChecker()
11+
let acls = { checkAccess: () => Promise.resolve(true) }
12+
return expect(acl.checkAccess(acls)).to.eventually.be.true
2813
})
29-
let graph = {}
30-
let user, mode, resource, aclUrl
31-
let acl = new ACLChecker(resource, { debug })
32-
let acls = acl.getPermissionSet({ graph, acl: aclUrl })
33-
return expect(acl.checkAccess(acls, user, mode))
34-
.to.eventually.be.true
35-
})
36-
it('should callback with error on grant failure', () => {
37-
let ACLChecker = proxyquire('../../lib/acl-checker', {
38-
'solid-permissions': { PermissionSet: PermissionSetNeverGrant }
14+
it('should callback with error on grant failure', () => {
15+
let acl = new ACLChecker()
16+
let acls = { checkAccess: () => Promise.resolve(false) }
17+
return expect(acl.checkAccess(acls))
18+
.to.be.rejectedWith('ACL file found but no matching policy found')
19+
})
20+
it('should callback with error on grant error', () => {
21+
let acl = new ACLChecker()
22+
let acls = { checkAccess: () => Promise.reject(new Error('my error')) }
23+
return expect(acl.checkAccess(acls)).to.be.rejectedWith('my error')
3924
})
40-
let graph = {}
41-
let user, mode, resource, aclUrl
42-
let acl = new ACLChecker(resource, { debug })
43-
let acls = acl.getPermissionSet({ graph, acl: aclUrl })
44-
return expect(acl.checkAccess(acls, user, mode))
45-
.to.be.rejectedWith('ACL file found but no matching policy found')
4625
})
47-
it('should callback with error on grant error', () => {
48-
let ACLChecker = proxyquire('../../lib/acl-checker', {
49-
'solid-permissions': { PermissionSet: PermissionSetAlwaysError }
26+
27+
describe('getPossibleACLs', () => {
28+
it('returns all possible ACLs of the root', () => {
29+
const aclChecker = new ACLChecker('http://ex.org/')
30+
expect(aclChecker.getPossibleACLs()).to.deep.equal([
31+
'http://ex.org/.acl'
32+
])
33+
})
34+
35+
it('returns all possible ACLs of a regular file', () => {
36+
const aclChecker = new ACLChecker('http://ex.org/abc/def/ghi')
37+
expect(aclChecker.getPossibleACLs()).to.deep.equal([
38+
'http://ex.org/abc/def/ghi.acl',
39+
'http://ex.org/abc/def/.acl',
40+
'http://ex.org/abc/.acl',
41+
'http://ex.org/.acl'
42+
])
43+
})
44+
45+
it('returns all possible ACLs of an ACL file', () => {
46+
const aclChecker = new ACLChecker('http://ex.org/abc/def/ghi.acl')
47+
expect(aclChecker.getPossibleACLs()).to.deep.equal([
48+
'http://ex.org/abc/def/ghi.acl',
49+
'http://ex.org/abc/def/.acl',
50+
'http://ex.org/abc/.acl',
51+
'http://ex.org/.acl'
52+
])
53+
})
54+
55+
it('returns all possible ACLs of a directory', () => {
56+
const aclChecker = new ACLChecker('http://ex.org/abc/def/ghi/')
57+
expect(aclChecker.getPossibleACLs()).to.deep.equal([
58+
'http://ex.org/abc/def/ghi/.acl',
59+
'http://ex.org/abc/def/.acl',
60+
'http://ex.org/abc/.acl',
61+
'http://ex.org/.acl'
62+
])
5063
})
51-
let graph = {}
52-
let user, mode, resource, aclUrl
53-
let acl = new ACLChecker(resource, { debug })
54-
let acls = acl.getPermissionSet({ graph, acl: aclUrl })
55-
return expect(acl.checkAccess(acls, user, mode))
56-
.to.be.rejectedWith('Error thrown during checkAccess()')
5764
})
5865
})

0 commit comments

Comments
 (0)