Further Code Review Items

.travis.yml

@@ -4,6 +4,7 @@ node_js:
   - "4.1"
   - "0.12"
   - "0.11"
+  - "0.10"
 os:
   - linux
 script: "npm run-script test-travis"

lib/index.js

@@ -4,7 +4,7 @@ var debug = require('debug')('thalisalti:acl');
 var fast = require('./indexOf');
 
 var anonymous = 'public';
-const NOTFOUND = -1;
+var NOTFOUND = -1;
 
 /**
  * Acl
@@ -22,54 +22,46 @@ const NOTFOUND = -1;
  */
 module.exports = function (acl) {
   //path.role.verb
-  let msg401 = { success: false, message: 'Unauthorized' };
+  var msg401 = { success: false, message: 'Unauthorized' };
 
   return function (req, res, next) {
     debug('method: %s url: %s path: %s', req.method, req.url, req.path);
 
     //using headers for validation of the acl engine    
-    let identity = req.connection.pskIdentity || anonymous;
+    var identity = req.connection.pskIdentity || anonymous;
     debug('Identity: %s', identity);
 
-    //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);
-
-    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("/"));
+    //if the path is just '/' then use that, otherwise, find the path
+    //without the trailing '/'
+    var path = req.path === '/' ? '/' 
+          : req.path.slice(-1) === '/' ? req.path.slice(0, -1) 
+              : req.path;
+
+    var pathExists = fast.indexOf(acl, path, 'path');
 
-      //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 = fast.indexOf(acl, justPath, 'path');
-        if ( NOTFOUND === pathExists) {
-          debug('unauthorized1: %s : %s', identity, req.path);
-          return res.status(401).send(msg401);
-        }
-      }
+    //here we're looking for an EXACT match as is - with full path and NO resource on the end...
+    //ie.  /foobar/myresource.js  - this will fail if /foobar is in the ACL's
+
+    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 = fast.indexOf(roles, identity, 'role');
-    if ( NOTFOUND === roleExists) {
+    //roles are an array of objects with properyt of 'role'
+    var roles = acl[pathExists].roles;
+    var 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 = fast.indexOf(verbs, req.method);
-
+    //verbs are just an array of strings
+    var verbs = roles[roleExists].verbs;
+    var verbExists = fast.indexOf(verbs, req.method);
+
     debug('path/role:  %s  %s', pathExists, roleExists);
 
-    if ( NOTFOUND === verbExists) {
+    if (NOTFOUND === verbExists) {
       debug('unauthorized3: %s : %s', identity, req.path);
       return res.status(401).send(msg401);
     }

newtests.txt

@@ -0,0 +1,29 @@
+
+ * | Path | Method | PouchDB API |
+ * |------|--------|-------------|
+ * | / | GET | NA [3] |
+ * | /:db | GET | info |
+ * | /:db/_all_docs | GET, HEAD, POST [1] | allDocs |
+ * | /:db/_bulk_get | POST | bulkGet |
+ * | /:db/_changes | GET, POST [2] | changes |
+ * | /:db/_local/thali_:id | GET, PUT, DELETE | get, put, remove |
+ * | /:db/_local/:id |
+ * | /:db/_revs_diff | POST | revsDiff |
+ * | /:db/:id | GET | get |
+
+ review the detailed spec
+ https://github.com/thaliproject/Thali_CordovaPlugin/blob/vNext/thali/NextGeneration/security/thaliACLLayer.js
+
+
+
+ if no identity, verify that it's loopback address calling.
+
+  * If req.connection.pskIdentity is null/undefined then we MUST:
+ * - check req.ip to make sure that the IP address is set to
+ * "127.0.0.1"
+ * - make a case sensitive string compare and make sure that
+ * the value of the authorization header after the "CLEAR" keyword matches
+ * the value set in this object.
+ * If both checks pass then we MUST call next(). Otherwise we MUST immediately
+ * return a 401 Unauthorized and close the connection as defined below. For now
+ * we won't worry about also returning a www-authenticate header.

package.json

@@ -1,6 +1,6 @@
 {
   "name": "salti",
-  "version": "0.1.4",
+  "version": "0.1.5",
   "description": "Simple Authentication and Authorization for Thali IoT",
   "main": "lib/index.js",
   "scripts": {

test/acl-path-types.js

@@ -9,7 +9,7 @@ module.exports =  [
     ]
   },
     {
-    "path": '/foo/',
+    "path": '/foo',
     "roles": [
       {"role": 'public',
         "verbs": ['GET', 'PUT', 'POST']}
@@ -23,7 +23,7 @@ module.exports =  [
     ]
   },
     {
-    "path": '/fiz/baz/',
+    "path": '/fiz/baz',
     "roles": [
       {"role": 'public',
         "verbs": ['GET', 'PUT', 'POST']}