Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 55 additions & 113 deletions lib/acl-checker.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

const async = require('async')
const path = require('path')
const PermissionSet = require('solid-permissions').PermissionSet
const rdf = require('rdflib')
const url = require('url')

const DEFAULT_ACL_SUFFIX = '.acl'
Expand Down Expand Up @@ -33,18 +35,11 @@ class ACLChecker {
// Let's see if there is a file..
self.fetch(acl, function (err, graph) {
if (err || !graph || graph.length === 0) {
// TODO
// If no file is found and we want to Control,
// we should not be able to do that!
// Control is only to Read and Write the current file!
// if (mode === 'Control') {
// return next(new Error('You can\'t Control an unexisting file'))
// }
if (err) debug('Error: ' + err)
accessType = 'defaultForNew'
return next()
}
self.findRule(
self.checkAccess(
graph, // The ACL graph
user, // The webId of the user
mode, // Read/Write/Append
Expand Down Expand Up @@ -81,125 +76,72 @@ class ACLChecker {
})
}

findAgentClass (graph, user, mode, resource, acl, callback) {
const debug = this.debug
// Agent class statement
var agentClassStatements = graph.match(acl,
'http://www.w3.org/ns/auth/acl#agentClass')
if (agentClassStatements.length === 0) {
return callback(false)
}
async.some(
agentClassStatements,
function (agentClassTriple, found) {
// Check for FOAF groups
debug('Found agentClass policy')
if (agentClassTriple.object.uri === 'http://xmlns.com/foaf/0.1/Agent') {
debug(mode + ' allowed access as FOAF agent')
return found(true)
}
return found(false)
},
callback)
}

findRule (graph, user, mode, resource, accessType, acl, callback, options) {
/**
* 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 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
if (!graph || graph.length === 0) {
debug('ACL ' + acl + ' is empty')
return callback(new Error('No policy found'))
return callback(new Error('No policy found - empty ACL'))
}
debug('Found policies in ' + acl)
// Check for mode
var statements = this.getMode(graph, mode)
if (mode === 'Append') {
statements = statements.concat(this.getMode(graph, 'Write'))
let isContainer = accessType.startsWith('default')
let aclOptions = {
aclSuffix: this.suffix,
graph: graph,
host: options.host,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this used?
where is host set and what is this set to?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

host gets set in the allow handler, here.
It's used by the 'enforce strict origin' code in the permissions lib.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect! works with me!

origin: options.origin,
rdf: rdf,
strictOrigin: this.strictOrigin,
isAcl: (uri) => { return this.isAcl(uri) },
aclUrlFor: (uri) => { return this.aclUrlFor(uri) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at this point, can we not pass the entire ACL instance?
(maybe not)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could? I'd rather be explicit, though..

}
var self = this
async.some(
statements,
function (statement, done) {
var statementSubject = statement.subject.uri
// Check for origin
var matchOrigin = self.matchOrigin(graph, statementSubject,
options.origin, options.host)
if (!matchOrigin) {
debug('The request does not match the origin')
return done(false)
}
// Check for accessTo/defaultForNew
if (!self.isAcl(resource) || accessType === 'defaultForNew') {
debug('Checking for accessType:' + accessType)
var accesses = self.matchAccessType(graph, statementSubject, accessType,
resource)
if (!accesses) {
debug('Cannot find accessType ' + accessType)
return done(false)
}
}
// Check for Agent
var agentStatements = []
if (user) {
agentStatements = graph.match(
statementSubject,
'http://www.w3.org/ns/auth/acl#agent',
user)
}
if (agentStatements.length) {
debug(mode + ' access allowed (as agent) for: ' + user)
return done(true)
}
debug('Inspect agentClass')
// Check for AgentClass
return self.findAgentClass(graph, user, mode, resource, statementSubject,
done)
},
function (found) {
if (!found) {
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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this method is called findRule, I think I assumed that the callback function would get passed the access rule/policy for the given user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, now that it's refactored the findRule name doesn't make as much sense. I'll rename it to something like checkAccess.

} else {
debug(`${mode} access not permitted to ${user}`)
return callback(new Error('Acl found but policy not found'))
}
return callback(null)
})
.catch(err => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you clarify the distinction between a rejected promise and a resolved promise with a falsy value in checkAccess?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. checkAccess() resolves with either a truthy value (which means the user has access), or a falsy value (which means the user does not have access). A rejected promise means an error (parsing error, argument error, network errors, etc).

debug(`${mode} access denied to ${user}`)
debug(err)
return callback(err)
})
}

getMode (graph, mode) {
return graph.match(
null,
'http://www.w3.org/ns/auth/acl#mode',
'http://www.w3.org/ns/auth/acl#' + mode
)
aclUrlFor (uri) {
if (this.isAcl(uri)) {
return uri
} else {
return uri + this.suffix
}
}

isAcl (resource) {
return resource.endsWith(this.suffix)
}

matchAccessType (graph, rule, accessType, uri) {
var matches = graph.match(
rule,
'http://www.w3.org/ns/auth/acl#' + accessType
)
return matches.some(function (match) {
return uri.startsWith(match.object.uri)
})
}

matchOrigin (graph, rule, origin, host) {
// if there is no origin, then the host is the origin
if (this.strictOrigin && !origin) {
return true
}
var origins = graph.match(
rule,
'http://www.w3.org/ns/auth/acl#origin'
)
if (origins.length) {
return origins.some(function (triple) {
return triple.object.uri === (origin || host)
})
if (typeof resource === 'string') {
return resource.endsWith(this.suffix)
} else {
return false
}
// return true if origin is not enforced
return !this.strictOrigin
}

static possibleACLs (uri, suffix) {
Expand Down
6 changes: 2 additions & 4 deletions lib/handlers/allow.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ function allow (mode) {
var reqPath = res && res.locals && res.locals.path
? res.locals.path
: req.path
ldp.exists(req.hostname, reqPath, function (err, stat) {
if (reqPath[reqPath.length - 1] !== '/' &&
!err &&
stat.isDirectory()) {
ldp.exists(req.hostname, reqPath, (err, stat) => {
if (!reqPath.endsWith('/') && !err && stat.isDirectory()) {
reqPath += '/'
}
var options = {
Expand Down
1 change: 0 additions & 1 deletion lib/ldp.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ LDP.prototype.graph = function (host, reqPath, baseUri, contentType, callback) {

var root = ldp.idp ? ldp.root + host + '/' : ldp.root
var filename = utils.uriToFilename(reqPath, root)
console.log(filename)

async.waterfall([
// Read file
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"rimraf": "^2.5.0",
"run-waterfall": "^1.1.3",
"solid-namespace": "^0.1.0",
"solid-permissions": "^0.3.0",
"solid-ws": "^0.2.2",
"string": "^3.3.0",
"uid-safe": "^2.1.1",
Expand All @@ -75,6 +76,7 @@
"hippie": "^0.5.0",
"mocha": "^2.2.5",
"nock": "^7.0.2",
"proxyquire": "^1.7.10",
"rsvp": "^3.1.0",
"run-waterfall": "^1.1.3",
"sinon": "^1.17.4",
Expand Down
65 changes: 65 additions & 0 deletions test/acl-checker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
'use strict'
const proxyquire = require('proxyquire')
const assert = require('chai').assert
const debug = require('../lib/debug').ACL

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', done => {
let ACLChecker = proxyquire('../lib/acl-checker', {
'solid-permissions': { PermissionSet: PermissionSetAlwaysGrant }
})
let graph = {}
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()
})
})
it('should callback with error on grant failure', done => {
let ACLChecker = proxyquire('../lib/acl-checker', {
'solid-permissions': { PermissionSet: PermissionSetNeverGrant }
})
let graph = {}
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()
})
})
it('should callback with error on grant error', done => {
let ACLChecker = proxyquire('../lib/acl-checker', {
'solid-permissions': { PermissionSet: PermissionSetAlwaysError }
})
let graph = {}
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()
})
})
})