Various ACL fixes

lib/application.js

@@ -162,12 +162,7 @@ app.enableAuth = function() {
     var req = ctx.req;
     var Model = method.ctor;
     var modelInstance = ctx.instance;
-    var modelId = modelInstance && modelInstance.id;
-
-    // TODO(ritch) - this fallback could be less express dependent
-    if(modelInstance && !modelId) {
-      modelId = req.param('id');
-    }
+    var modelId = modelInstance && modelInstance.id || req.param('id');
 
     Model.checkAccess(
       req.accessToken,

lib/models/access-token.js

@@ -95,7 +95,7 @@ AccessToken.findForRequest = function(req, options, cb) {
     this.findById(id, function(err, token) {
       if(err) {
         cb(err);
-      } else {
+      } else if(token) {
         token.validate(function(err, isValid) {
           if(err) {
             cb(err);
@@ -105,6 +105,8 @@ AccessToken.findForRequest = function(req, options, cb) {
             cb(new Error('Invalid Access Token'));
           }
         });
+      } else {
+        cb();
       }
     });
   } else {

lib/models/acl.js

@@ -34,6 +34,7 @@
 var loopback = require('../loopback');
 var async = require('async');
 var assert = require('assert');
+var debug = require('debug')('acl');
 
 var role = require('./role');
 var Role = role.Role;
@@ -190,6 +191,7 @@ function resolvePermission(acls, req) {
       }
     }
   }
+
   return {
     model: req.model,
     property: req.property,
@@ -208,7 +210,7 @@ function resolvePermission(acls, req) {
  *
  * @return {Object[]} An array of ACLs
  */
-function getStaticACLs(principalType, principalId, model, property, accessType) {
+function getStaticACLs(model, property, accessType) {
   var modelClass = loopback.getModel(model);
   var staticACLs = [];
   if (modelClass && modelClass.settings.acls) {
@@ -269,7 +271,7 @@ ACL.checkPermission = function (principalType, principalId, model, property, acc
     accessType: accessType
   };
 
-  var acls = getStaticACLs(principalType, principalId, model, property, accessType);
+  var acls = getStaticACLs(model, property, accessType);
 
   var resolved = resolvePermission(acls, req);
 
@@ -355,21 +357,7 @@ ACL.checkAccess = function (context, callback) {
   };
 
   var effectiveACLs = [];
-
-  // Check the LDL ACLs
-  principals.forEach(function(p) {
-    effectiveACLs = effectiveACLs.concat(getStaticACLs(p.principalType, p.principalId, model.modelName, property, accessType));
-  });
-
-  var resolved = resolvePermission(effectiveACLs, req);
-
-  if(resolved && resolved.permission === ACL.DENY) {
-    // Fail fast
-    process.nextTick(function() {
-      callback && callback(null, resolved);
-    });
-    return;
-  }
+  var staticACLs = getStaticACLs(model.modelName, property, accessType);
 
   ACL.find({where: {model: model.modelName, property: propertyQuery, accessType: accessTypeQuery}}, function (err, acls) {
     if (err) {
@@ -377,6 +365,9 @@ ACL.checkAccess = function (context, callback) {
       return;
     }
     var inRoleTasks = [];
+
+    acls = acls.concat(staticACLs);
+
     acls.forEach(function (acl) {
       principals.forEach(function (principal) {
         if (principal.principalType === acl.pricipalType && principal.principalId === acl.principalId) {
@@ -384,11 +375,12 @@ ACL.checkAccess = function (context, callback) {
         } else if (acl.principalType === ACL.ROLE) {
           inRoleTasks.push(function (done) {
             Role.isInRole(acl.principalId,
-              {principalType: principal.principalType, principalId: acl.principalId, model: model, id: id, property: property},
+              {principalType: principal.principalType, userId: context.userId, principalId: acl.principalId, model: model, id: id, property: property},
               function (err, inRole) {
-                if(!err) {
+                if(!err && inRole) {
                   effectiveACLs.push(acl);
                 }
+
                 done(err, acl);
               });
           });
@@ -429,12 +421,14 @@ ACL.checkAccessForToken = function(token, model, modelId, method, callback) {
   var modelCtor = loopback.getModel(model);
 
   var context = {
+    userId: token.userId,
     principals: principals,
     model: model,
     property: method,
     accessType: modelCtor._getAccessTypeForMethod(method),
     id: modelId
   };
+
   ACL.checkAccess(context, function(err, access) {
     if(err) {
       callback && callback(err);

lib/models/role.js

@@ -161,11 +161,28 @@ Role.registerResolver(Role.OWNER, function(role, context, callback) {
   }
   var modelClass = context.model;
   var id = context.id;
-  var userId = context.principalId;
+  var userId = context.userId;
   isOwner(modelClass, id, userId, callback);
 });
 
-function isOwner(modelClass, id, userId, callback) {
+function isOwner(modelClass, id, userId, callback) {  
+  if(!userId) {
+    process.nextTick(function() {
+      callback(null, false);
+    });
+    return;
+  }
+  
+  var modelClassIsUserClass = modelClass.prototype instanceof loopback.User
+                            || modelClass === loopback.User;
+
+  if(modelClassIsUserClass) {
+    process.nextTick(function() {
+      callback(null, id === userId);
+    });
+    return;
+  }
+
   modelClass.findById(id, function(err, inst) {
     if(err) {
       callback && callback(err);