Refactor ACL to allow for `methodNames` / aliases

lib/application.js

@@ -309,7 +309,7 @@ app.enableAuth = function() {
       Model.checkAccess(
         req.accessToken,
         modelId,
-        method.name,
+        method,
         function(err, allowed) {
           // Emit any cached data events that fired while checking access.
           req.resume();

lib/models/access-context.js

@@ -36,7 +36,15 @@ function AccessContext(context) {
   this.property = context.property || AccessContext.ALL;
 
   this.method = context.method;
-
+  this.sharedMethod = context.sharedMethod;
+  this.sharedClass = this.sharedMethod && this.sharedMethod.sharedClass;
+  if(this.sharedMethod) {
+    this.methodNames = this.sharedMethod.aliases.concat([this.sharedMethod.name]);  
+  } else {
+    this.methodNames = [];
+  }
+  
+  this.accessType = this.model._getAccessTypeForMethod(this.method);
   this.accessType = context.accessType || AccessContext.ALL;
   this.accessToken = context.accessToken || AccessToken.ANONYMOUS;
 
@@ -79,7 +87,6 @@ AccessContext.permissionOrder = {
   DENY: 4
 };
 
-
 /**
  * Add a principal to the context
  * @param {String} principalType The principal type
@@ -96,8 +103,6 @@ AccessContext.prototype.addPrincipal = function (principalType, principalId, pri
     }
   }
   this.principals.push(principal);
-
-  debug('adding principal %j', principal);
   return true;
 };
 
@@ -213,7 +218,7 @@ Principal.prototype.equals = function (p) {
  * @returns {AccessRequest}
  * @class
  */
-function AccessRequest(model, property, accessType, permission) {
+function AccessRequest(model, property, accessType, permission, methodNames) {
   if (!(this instanceof AccessRequest)) {
     return new AccessRequest(model, property, accessType);
   }
@@ -224,20 +229,13 @@ function AccessRequest(model, property, accessType, permission) {
     this.property = obj.property || AccessContext.ALL;
     this.accessType = obj.accessType || AccessContext.ALL;
     this.permission = obj.permission || AccessContext.DEFAULT;
+    this.methodNames = methodNames || [];
   } else {
     this.model = model || AccessContext.ALL;
     this.property = property || AccessContext.ALL;
     this.accessType = accessType || AccessContext.ALL;
     this.permission = permission || AccessContext.DEFAULT;
-  }
+    this.methodNames = methodNames || [];
-
-  if(debug.enabled) {
-    debug('---AccessRequest---');
-    debug(' model %s', this.model);
-    debug(' property %s', this.property);
-    debug(' accessType %s', this.accessType);
-    debug(' permission %s', this.permission);
-    debug(' isWildcard() %s', this.isWildcard());
   }
 }
 
@@ -251,6 +249,52 @@ AccessRequest.prototype.isWildcard = function () {
     this.accessType === AccessContext.ALL;
 };
 
+/**
+ * Does the given `ACL` apply to this `AccessRequest`.
+ *
+ * @param {ACL} acl
+ */
+
+AccessRequest.prototype.exactlyMatches = function(acl) {
+  var matchesModel = acl.model === this.model;
+  var matchesProperty = acl.property === this.property;
+  var matchesMethodName = this.methodNames.indexOf(acl.property) !== -1;
+  var matchesAccessType = (acl.accessType || '*') === this.accessType;
+
+  debug('matchesModel %s === %s %j', acl.model, this.model, acl.model === this.model);
+  debug('matchesProperty %s === %s %j', acl.property, this.property, acl.property === this.property);
+  debug('matchesMethodName %s in %j %j', acl.property, this.methodNames, this.methodNames.indexOf(acl.property) !== -1);
+  debug('matchesAccessType %s === %s %j', acl.accessType, this.accessType, acl.accessType === this.accessType);
+
+  if(matchesModel && matchesAccessType) {
+    return matchesProperty || matchesMethodName;
+  }
+
+  return false;
+}
+
+/**
+ * Is the request for access allowed?
+ * 
+ * @returns {Boolean}
+ */
+
+AccessRequest.prototype.isAllowed = function() {
+  return this.permission !== require('./acl').ACL.DENY;
+}
+
+AccessRequest.prototype.debug = function() {
+  if(debug.enabled) {
+    debug('---AccessRequest---');
+    debug(' model %s', this.model);
+    debug(' property %s', this.property);
+    debug(' accessType %s', this.accessType);
+    debug(' permission %s', this.permission);
+    debug(' isWildcard() %s', this.isWildcard());
+    debug(' isAllowed() %s', this.isAllowed());
+  }
+}
+
 module.exports.AccessContext = AccessContext;
 module.exports.Principal = Principal;
 module.exports.AccessRequest = AccessRequest;

lib/models/acl.js

@@ -119,11 +119,17 @@ ACL.SCOPE = Principal.SCOPE;
 ACL.getMatchingScore = function getMatchingScore(rule, req) {
   var props = ['model', 'property', 'accessType'];
   var score = 0;
+
   for (var i = 0; i < props.length; i++) {
     // Shift the score by 4 for each of the properties as the weight
     score = score * 4;
     var val1 = rule[props[i]] || ACL.ALL;
     var val2 = req[props[i]] || ACL.ALL;
+
+    if(props[i] === 'property' && req.methodNames.indexOf(val1) !== -1) {
+      score += 3;
+    }
+
     if (val1 === val2) {
       // Exact match
       score += 3;
@@ -200,11 +206,10 @@ ACL.resolvePermission = function resolvePermission(acls, req) {
   acls = acls.sort(function (rule1, rule2) {
     return ACL.getMatchingScore(rule2, req) - ACL.getMatchingScore(rule1, req);
   });
-  if(debug.enabled) {
-    debug('ACLs by order: %j', acls);
-  }
   var permission = ACL.DEFAULT;
   var score = 0;
+  var matchingACL;
+
   for (var i = 0; i < acls.length; i++) {
     score = ACL.getMatchingScore(acls[i], req);
     if (score < 0) {
@@ -212,25 +217,41 @@ ACL.resolvePermission = function resolvePermission(acls, req) {
     }
     if (!req.isWildcard()) {
       // We should stop from the first match for non-wildcard
-      permission = acls[i].permission;
+      debug('Found first match (non-wildcard):');
+      matchingACL = acls[i];
+      permission = matchingACL.permission;
       break;
     } else {
-      if(acls[i].model === req.model &&
+      if(req.exactlyMatches(acls[i])) {
-        acls[i].property === req.property &&
+        debug('Exact ACL match:');
-        acls[i].accessType === req.accessType
+        acls[i].debug();
-        ) {
         // We should stop at the exact match
-        permission = acls[i].permission;
+        matchingACL = acls[i];
+        permission = matchingACL.permission;
         break;
       }
       // For wildcard match, find the strongest permission
       if(AccessContext.permissionOrder[acls[i].permission]
         > AccessContext.permissionOrder[permission]) {
-        permission = acls[i].permission;
+        matchingACL = acls[i];
+        permission = matchingACL.permission;
       }
     }
   }
 
+  if(debug.enabled) {
+    if(matchingACL) {
+      debug('Matching ACL:');
+      matchingACL.debug();
+    } else {
+      debug('No matching ACL found!')
+    }
+    debug('The following ACLs were searched: ');
+    acls.forEach(function(acl) {
+      acl.debug();
+    });
+  }
+
   var res = new AccessRequest(req.model, req.property, req.accessType,
     permission || ACL.DEFAULT);
   return res;
@@ -256,8 +277,6 @@ ACL.getStaticACLs = function getStaticACLs(model, property) {
         accessType: acl.accessType,
         permission: acl.permission
       }));
-
-      staticACLs[staticACLs.length - 1].debug('Adding ACL');
     });
   }
   var prop = modelClass &&
@@ -359,6 +378,7 @@ ACL.prototype.debug = function() {
  * @property {String} accessType The access type
  * @param {Function} callback
  */
+
 ACL.checkAccessForContext = function (context, callback) {
   if(!(context instanceof AccessContext)) {
     context = new AccessContext(context);
@@ -367,11 +387,13 @@ ACL.checkAccessForContext = function (context, callback) {
   var model = context.model;
   var property = context.property;
   var accessType = context.accessType;
+  var modelName = context.modelName;
 
-  var propertyQuery = (property === ACL.ALL) ? undefined : {inq: [property, ACL.ALL]};
+  var methodNames = context.methodNames;
+  var propertyQuery = (property === ACL.ALL) ? undefined : {inq: methodNames.concat([ACL.ALL])};
   var accessTypeQuery = (accessType === ACL.ALL) ? undefined : {inq: [accessType, ACL.ALL]};
 
-  var req = new AccessRequest(model.modelName, property, accessType);
+  var req = new AccessRequest(modelName, property, accessType, methodNames);
 
   var effectiveACLs = [];
   var staticACLs = this.getStaticACLs(model.modelName, property);
@@ -404,9 +426,6 @@ ACL.checkAccessForContext = function (context, callback) {
         inRoleTasks.push(function (done) {
           roleModel.isInRole(acl.principalId, context,
             function (err, inRole) {
-              if(debug.enabled) {
-                debug('In role %j: %j', acl.principalId, inRole);
-              }
               if (!err && inRole) {
                 effectiveACLs.push(acl);
               }
@@ -425,13 +444,13 @@ ACL.checkAccessForContext = function (context, callback) {
       if(resolved && resolved.permission === ACL.DEFAULT) {
         resolved.permission = (model && model.settings.defaultPermission) || ACL.ALLOW;
       }
-      debug('checkAccessForContext() returns: %j', resolved);
+      debug('---Resolved---');
+      resolved.debug();
       callback && callback(null, resolved);
     });
   });
 };
 
-
 /**
  * Check if the given access token can invoke the method
  * @param {AccessToken} token The access token
@@ -454,10 +473,6 @@ ACL.checkAccessForToken = function (token, model, modelId, method, callback) {
     modelId: modelId
   });
 
-  context.accessType = context.model._getAccessTypeForMethod(method);
-
-  context.debug();
-
   this.checkAccessForContext(context, function (err, access) {
     if (err) {
       callback && callback(err);

lib/models/model.js

@@ -132,25 +132,34 @@ Model._ACL = function getACL(ACL) {
   return _aclModel;
 };
 
-
 /**
  * Check if the given access token can invoke the method
  *
  * @param {AccessToken} token The access token
  * @param {*} modelId The model id
- * @param {String} method The method name
+ * @param {SharedMethod} sharedMethod
  * @param callback The callback function
  *
  * @callback {Function} callback
  * @param {String|Error} err The error object
  * @param {Boolean} allowed is the request allowed
  */
-Model.checkAccess = function(token, modelId, method, callback) {
+Model.checkAccess = function(token, modelId, sharedMethod, callback) {
   var ANONYMOUS = require('./access-token').ANONYMOUS;
   token = token || ANONYMOUS;
   var aclModel = Model._ACL();
-  var methodName = 'string' === typeof method? method: method && method.name;
+  
-  aclModel.checkAccessForToken(token, this.modelName, modelId, methodName, callback);
+  aclModel.checkAccessForContext({
+    accessToken: token,
+    model: this,
+    property: sharedMethod.name,
+    method: sharedMethod.name,
+    sharedMethod: sharedMethod,
+    modelId: modelId
+  }, function(err, accessRequest) {
+    if(err) return callback(err);
+    callback(null, accessRequest.isAllowed());
+  });
 };
 
 /*!
@@ -163,7 +172,7 @@ Model.checkAccess = function(token, modelId, method, callback) {
 Model._getAccessTypeForMethod = function(method) {
   if(typeof method === 'string') {
     method = {name: method};
-  }
+  }40
   assert(
     typeof method === 'object', 
     'method is a required argument and must be a RemoteMethod object'

lib/models/role.js

@@ -319,7 +319,8 @@ Role.registerResolver(Role.EVERYONE, function (role, context, callback) {
  * @param {Boolean} isInRole
  */
 Role.isInRole = function (role, context, callback) {
-  debug('isInRole(): %s %j', role, context);
+  debug('isInRole(): %s', role);
+  context.debug();
 
   if (!(context instanceof AccessContext)) {
     context = new AccessContext(context);
@@ -409,8 +410,6 @@ Role.isInRole = function (role, context, callback) {
  * @param {String[]} An array of role ids
  */
 Role.getRoles = function (context, callback) {
-  debug('getRoles(): %j', context);
-
   if(!(context instanceof AccessContext)) {
     context = new AccessContext(context);
   }

test/acl.test.js

@@ -296,7 +296,6 @@ describe('security ACLs', function () {
       });
     });
   });
-
 });
 
 

test/app.test.js

@@ -541,7 +541,6 @@ describe('app', function() {
 
     it('adds a camelized alias', function() {
       app.connector('FOO-BAR', loopback.Memory);
-      console.log(app.connectors);
       expect(app.connectors.FOOBAR).to.equal(loopback.Memory);
     });
   });