Merge pull request #298 from strongloop/fix/aliased-shared-methods

Fix/aliased shared methods

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,18 @@ 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 = [];
+  }
+  
+  if(this.sharedMethod) {
+    this.accessType = this.model._getAccessTypeForMethod(this.sharedMethod);  
+  }
+  
   this.accessType = context.accessType || AccessContext.ALL;
   this.accessToken = context.accessToken || AccessToken.ANONYMOUS;
 
@@ -79,7 +90,6 @@ AccessContext.permissionOrder = {
   DENY: 4
 };
 
-
 /**
  * Add a principal to the context
  * @param {String} principalType The principal type
@@ -96,8 +106,6 @@ AccessContext.prototype.addPrincipal = function (principalType, principalId, pri
     }
   }
   this.principals.push(principal);
-
-  debug('adding principal %j', principal);
   return true;
 };
 
@@ -213,7 +221,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,26 +232,20 @@ 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());
   }
 }
 
 /**
- * Is the request a wildcard
+ * Does the request contain any wildcards?
- * @returns {boolean}
+ *
+ * @returns {Boolean}
  */
 AccessRequest.prototype.isWildcard = function () {
   return this.model === AccessContext.ALL ||
@@ -251,6 +253,47 @@ 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;
+
+  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,12 +119,15 @@ 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 (val1 === val2) {
+    var isMatchingMethodName = props[i] === 'property' && req.methodNames.indexOf(val1) !== -1;
+
+    if (val1 === val2 || isMatchingMethodName) {
       // Exact match
       score += 3;
     } else if (val1 === ACL.ALL) {
@@ -186,6 +189,16 @@ ACL.getMatchingScore = function getMatchingScore(rule, req) {
   return score;
 };
 
+/**
+ * Get matching score for the given `AccessRequest`.
+ * @param {AccessRequest} req The request
+ * @returns {Number} score
+ */
+
+ACL.prototype.score = function(req) {
+  return this.constructor.getMatchingScore(this, req);
+}
+
 /*!
  * Resolve permission from the ACLs
  * @param {Object[]) acls The list of ACLs
@@ -200,14 +213,13 @@ 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;
+
   for (var i = 0; i < acls.length; i++) {
     score = ACL.getMatchingScore(acls[i], req);
     if (score < 0) {
+      // the highest scored ACL did not match
       break;
     }
     if (!req.isWildcard()) {
@@ -215,11 +227,7 @@ ACL.resolvePermission = function resolvePermission(acls, req) {
       permission = acls[i].permission;
       break;
     } else {
-      if(acls[i].model === req.model &&
+      if(req.exactlyMatches(acls[i])) {
-        acls[i].property === req.property &&
-        acls[i].accessType === req.accessType
-        ) {
-        // We should stop at the exact match
         permission = acls[i].permission;
         break;
       }
@@ -231,6 +239,14 @@ ACL.resolvePermission = function resolvePermission(acls, req) {
     }
   }
 
+  if(debug.enabled) {
+    debug('The following ACLs were searched: ');
+    acls.forEach(function(acl) {
+      acl.debug();
+      debug('with score:', acl.score(req));
+    });
+  }
+
   var res = new AccessRequest(req.model, req.property, req.accessType,
     permission || ACL.DEFAULT);
   return res;
@@ -253,11 +269,9 @@ ACL.getStaticACLs = function getStaticACLs(model, property) {
         property: acl.property || ACL.ALL,
         principalType: acl.principalType,
         principalId: acl.principalId, // TODO: Should it be a name?
-        accessType: acl.accessType,
+        accessType: acl.accessType || ACL.ALL,
         permission: acl.permission
       }));
-
-      staticACLs[staticACLs.length - 1].debug('Adding ACL');
     });
   }
   var prop = modelClass &&
@@ -359,6 +373,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 +382,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, ACL.DEFAULT, methodNames);
 
   var effectiveACLs = [];
   var staticACLs = this.getStaticACLs(model.modelName, property);
@@ -404,9 +421,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 +439,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 +468,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,35 @@ 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,
+    accessType: this._getAccessTypeForMethod(sharedMethod)
+  }, function(err, accessRequest) {
+    if(err) return callback(err);
+    callback(null, accessRequest.isAllowed());
+  });
 };
 
 /*!

lib/models/role.js

@@ -319,12 +319,13 @@ Role.registerResolver(Role.EVERYONE, function (role, context, callback) {
  * @param {Boolean} isInRole
  */
 Role.isInRole = function (role, context, callback) {
-  debug('isInRole(): %s %j', role, context);
-
   if (!(context instanceof AccessContext)) {
     context = new AccessContext(context);
   }
 
+  debug('isInRole(): %s', role);
+  context.debug();
+
   var resolver = Role.resolvers[role];
   if (resolver) {
     debug('Custom resolver found for role %s', role);
@@ -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);
   }

package.json

@@ -33,7 +33,7 @@
   "dependencies": {
     "debug": "~0.8.1",
     "express": "~3.5.0",
-    "strong-remoting": "~1.4.0",
+    "strong-remoting": "~1.5.0",
     "inflection": "~1.3.5",
     "passport": "~0.2.0",
     "passport-local": "~1.0.0",

test/acl.test.js

@@ -101,11 +101,15 @@ describe('security ACLs', function () {
       property: 'find',
       accessType: 'WRITE'
     };
+
+    acls = acls.map(function(a) { return new ACL(a)});
+
     var perm = ACL.resolvePermission(acls, req);
     assert.deepEqual(perm, { model: 'account',
       property: 'find',
       accessType: 'WRITE',
-      permission: 'ALLOW' });
+      permission: 'ALLOW',
+      methodNames: []});
   });
 
   it("should allow access to models for the given principal by wildcard", function () {
@@ -296,7 +300,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);
     });
   });