Skip to content
This repository has been archived by the owner on Dec 15, 2020. It is now read-only.

Commit

Permalink
addressing review items
Browse files Browse the repository at this point in the history
  • Loading branch information
@cicorias
cicorias committed Mar 3, 2016
1 parent ffb2f60 commit f9963ab
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 93 deletions.
50 changes: 27 additions & 23 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,20 @@ var debug = require('debug')('thalisalti:acl');
var fast = require('./indexOf');

var anonymous = 'public';
const NOTFOUND = -1;

function mapIt(subject, key, target) {
var itExists = fast.indexOf(subject, target, key, 0);
if (!~itExists) {
return itExists;
}

return fast.indexOf(subject, target, key, 0);
}
/**
* Acl
* @typedef {Object} Salti.Acl
* TODO:
*/


/**
* Implements an Express middleware Authentication Filter
* that validates an identity provided as a property on the request.connection object
* agasint ACL represented as a json object.
* @param {Object} acl - the acl represented as an array of ACL's - generally they
* are represented as path.role.verb.
* against ACL represented as a json object.
* @param {Salti.Acl} acl - acl represented as an array of path.role.verb
* @returns {Function} returns a handler of (req, res, next) to the middleware pipeline
*/
module.exports = function (acl) {
Expand All @@ -34,38 +31,45 @@ module.exports = function (acl) {
let identity = req.connection.pskIdentity || anonymous;
debug('Identity: %s', identity);

var pathExists = mapIt(acl, 'path', req.path);
//here we're looking for an EXACT match as is - with full path and any resource on the end...
//ie. /foobar/myresource.js
var pathExists = fast.indexOf(acl, req.path, 'path');
debug('req.path: %s', req.path);

//here we fall back to see
if (!~pathExists) {
if ( NOTFOUND === pathExists) {
debug('raw path did not exist now till try with trailing slash');
//lets get the path up to the / but NOT with the slash...
let justPath = req.path.substring(0, req.path.lastIndexOf("/"));

debug('raw path did not exist now till try with trailing slash');
pathExists = mapIt(acl, 'path', justPath + '/');
if (!~pathExists) {
//here we see if it exists up to the last slash (including the slash)
//ie - /foobar/
pathExists = fast.indexOf(acl, justPath + '/', 'path');
if ( NOTFOUND === pathExists) {
//here we fall back to see if the path exists up to the last slash (not including)
//ie - /foobar
debug('now we try with no trailing slash');
pathExists = mapIt(acl, 'path', justPath);
if (!~pathExists) {
pathExists = fast.indexOf(acl, justPath, 'path');
if ( NOTFOUND === pathExists) {
debug('unauthorized1: %s : %s', identity, req.path);
return res.status(401).send(msg401);
}
}
}

//at this point the request PATH has passed...
let roles = acl[pathExists].roles;
let roleExists = mapIt(roles, 'role', identity);
if (!~roleExists) {
let roleExists = fast.indexOf(roles, identity, 'role');
if ( NOTFOUND === roleExists) {
debug('unauthorized2: %s : %s', identity, req.path);
return res.status(401).send(msg401);
}

let verbs = acl[pathExists].roles[roleExists].verbs;
let verbExists = mapIt(verbs, null, req.method.toLowerCase());
let verbExists = fast.indexOf(verbs, req.method);

debug('path/role: %s %s', pathExists, roleExists);

if (!~verbExists) {
if ( NOTFOUND === verbExists) {
debug('unauthorized3: %s : %s', identity, req.path);
return res.status(401).send(msg401);
}
Expand Down
32 changes: 6 additions & 26 deletions lib/indexOf.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,13 @@
* Custom indexOf implementation from fast.js.
* @module indesOf
*/

var secureCompare = require('secure-compare');

var secure = false;

function compareIt(source, target, secure) {
if (secure) {
function compareIt(source, target) {
if (source === target) {
return true;
}
else {
return false;
}
}
else {
return secureCompare(source, target);
}
}

/**
Expand All @@ -57,30 +47,20 @@ function compareIt(source, target, secure) {
* @param {Number} fromIndex - where to start in the subject (offset)
* @returns {Number} the index value where found or -1 if not found
*/
module.exports.indexOf = function fastIndexOf(subject, target, key, fromIndex) {
var length = subject.length,
i = 0;

if (typeof fromIndex === 'number') {
i = fromIndex;
if (i < 0) {
i += length;
if (i < 0) {
i = 0;
}
}
}
module.exports.indexOf = function fastIndexOf(subject, target, key) {
var length = subject.length;
var i = 0;

if (key) {
for (; i < length; i++) {
if (compareIt(subject[i][key], target, secure)) {
if (compareIt(subject[i][key], target)) {
return i;
}
}
}
else {
for (; i < length; i++) {
if (compareIt(subject[i], target, secure)) {
if (compareIt(subject[i], target)) {
return i;
}
}
Expand Down
3 changes: 1 addition & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
},
"homepage": "https://github.com/thaliproject/salti#readme",
"dependencies": {
"debug": "^2.2.0",
"secure-compare": "^3.0.1"
"debug": "^2.2.0"
},
"devDependencies": {
"colors": "^1.1.2",
Expand Down
12 changes: 6 additions & 6 deletions lib/acl-example.js → sample/acl-example.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,27 @@ module.exports = [{
"path": "/",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}, {
"role": "user",
"verbs": ["get"]
"verbs": ["GET"]
}]
}, {
"path": "/foo/",
"roles": [{
"role": "public",
"verbs": ["get", "post"]
"verbs": ["GET", "post"]
}, {
"role": "user",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "put", "post"]
}]
}, {
"path": "/bar",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}, {
"role": "user",
"verbs": ["get"]
"verbs": ["GET"]
}]
}];
28 changes: 14 additions & 14 deletions lib/fauxton.js → sample/fauxton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,81 +4,81 @@ module.exports = [{
"path": "/_utils",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}, {
"role": "user",
"verbs": ["get"]
"verbs": ["GET"]
}]
},
{
"path": "/db",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
},
{
"path": "/_utils/css",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
},
{
"path": "/_utils/js",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
}, {
"path": "/_utils/img",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
}, {
"path": "/_session",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
}, {
"path": "/_utils/fonts",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
}, {
"path": "/foobardb",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
}, {
"path": "/_all_dbs",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
}, {
"path": "/_utils/js/zeroclipboard",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
},
{
"path": "/_uuids",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
},
{
"path": "/favicon.ico",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
}];

Expand All @@ -88,4 +88,4 @@ module.exports = [{
///_all_dbs
// /_utils/js/zeroclipboard/Z
//db_utils
//db/css/
//db/css/
12 changes: 6 additions & 6 deletions lib/pouchdb.js → sample/pouchdb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,33 @@ module.exports = [{
"path": "/_validate",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
},
{
"path": "/foobarrepl",
"roles": [{
"role": "public",
"verbs": ["get", "post", "put"]
"verbs": ["GET", "POST", "PUT"]
}]
},
{
"path": "/foobarrepl/_local",
"roles": [{
"role": "public",
"verbs": ["get", "post", "put"]
"verbs": ["GET", "POST", "PUT"]
}]
}, {
"path": "/_session",
"roles": [{
"role": "public",
"verbs": ["get"]
"verbs": ["GET"]
}]
}, {
"path": "/_all_dbs",
"roles": [{
"role": "public",
"verbs": ["get", "put", "post"]
"verbs": ["GET", "PUT", "POST"]
}]
}];

Expand All @@ -40,4 +40,4 @@ module.exports = [{
///_all_dbs
// /_utils/js/zeroclipboard/Z
//db_utils
//db/css/
//db/css/
4 changes: 2 additions & 2 deletions sample/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var opts = {
}

var acllib = require('../lib/index');
var acl = require('../lib/pouchdb');
var acl = require('./pouchdb');
//Norml middleware usage..
router.all('*', acllib(acl));

Expand Down Expand Up @@ -84,4 +84,4 @@ function onError(error, parent) {
default:
throw error;
}
}
}
10 changes: 5 additions & 5 deletions test/acl-get-multipleusers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ module.exports = [
"path": "/publicall",
"roles": [
{"role": "public",
"verbs": ["get", "post", "put"]},
"verbs": ["GET", "POST", "PUT"]},
{"role": "user",
"verbs": ["get"]}
"verbs": ["GET"]}
]
},
{
Expand All @@ -16,16 +16,16 @@ module.exports = [
{"role": "public",
"verbs": []},
{"role": "user",
"verbs": ["post", "put", "get", "get", "put", "post"]}
"verbs": ["POST", "PUT", "GET", "GET", "PUT", "POST"]}
]
},
{
"path": "/publicget",
"roles": [
{"role": "public",
"verbs": ["get"]},
"verbs": ["GET"]},
{"role": "user",
"verbs": ["get", "post", "put"]}
"verbs": ["GET", "POST", "PUT"]}
]
}
];
Loading

0 comments on commit f9963ab

Please sign in to comment.