Merge pull request #94 from strongloop/feature/access-context

Refactor to the code use wrapper classes

lib/models/access-context.js

@@ -0,0 +1,208 @@
+var loopback = require('../loopback');
+var AccessToken = require('./access-token');
+
+/**
+ * Access context represents the context for a request to access protected
+ * resources
+ *
+ * The AccessContext instance contains the following properties:
+ * @property {Principal[]} principals An array of principals
+ * @property {Function} model The model class
+ * @property {String} modelName The model name
+ * @property {String} modelId The model id
+ * @property {String} property The model property/method/relation name
+ * @property {String} method The model method to be invoked
+ * @property {String} accessType The access type
+ * @property {AccessToken} accessToken The access token
+ *
+ * @param {Object} context The context object
+ * @returns {AccessContext}
+ * @constructor
+ */
+function AccessContext(context) {
+  if (!(this instanceof AccessContext)) {
+    return new AccessContext(context);
+  }
+  context = context || {};
+
+  this.principals = context.principals || [];
+  var model = context.model;
+  model = ('string' === typeof model) ? loopback.getModel(model) : model;
+  this.model = model;
+  this.modelName = model && model.modelName;
+
+  this.modelId = context.id || context.modelId;
+  this.property = context.property || AccessContext.ALL;
+
+  this.method = context.method;
+
+  this.accessType = context.accessType || AccessContext.ALL;
+  this.accessToken = context.accessToken || AccessToken.ANONYMOUS;
+
+  var principalType = context.principalType || Principal.USER;
+  var principalId = context.principalId || undefined;
+  var principalName = context.principalName || undefined;
+  if (principalId) {
+    this.addPrincipal(principalType, principalId, principalName);
+  }
+
+  var token = this.accessToken || {};
+
+  if (token.userId) {
+    this.addPrincipal(Principal.USER, token.userId);
+  }
+  if (token.appId) {
+    this.addPrincipal(Principal.APPLICATION, token.appId);
+  }
+}
+
+// Define constant for the wildcard
+AccessContext.ALL = '*';
+
+// Define constants for access types
+AccessContext.READ = 'READ'; // Read operation
+AccessContext.WRITE = 'WRITE'; // Write operation
+AccessContext.EXECUTE = 'EXECUTE'; // Execute operation
+
+AccessContext.DEFAULT = 'DEFAULT'; // Not specified
+AccessContext.ALLOW = 'ALLOW'; // Allow
+AccessContext.ALARM = 'ALARM'; // Warn - send an alarm
+AccessContext.AUDIT = 'AUDIT'; // Audit - record the access
+AccessContext.DENY = 'DENY'; // Deny
+
+AccessContext.permissionOrder = {
+  DEFAULT: 0,
+  ALLOW: 1,
+  ALARM: 2,
+  AUDIT: 3,
+  DENY: 4
+};
+
+
+/**
+ * Add a principal to the context
+ * @param {String} principalType The principal type
+ * @param {*} principalId The principal id
+ * @param {String} [principalName] The principal name
+ * @returns {boolean}
+ */
+AccessContext.prototype.addPrincipal = function (principalType, principalId, principalName) {
+  var principal = new Principal(principalType, principalId, principalName);
+  for (var i = 0; i < this.principals.length; i++) {
+    var p = this.principals[i];
+    if (p.equals(principal)) {
+      return false;
+    }
+  }
+  this.principals.push(principal);
+  return true;
+};
+
+/**
+ * Get the user id
+ * @returns {*}
+ */
+AccessContext.prototype.getUserId = function() {
+  for (var i = 0; i < this.principals.length; i++) {
+    var p = this.principals[i];
+    if (p.type === Principal.USER) {
+      return p.id;
+    }
+  }
+  return null;
+};
+
+
+/**
+ * Get the application id
+ * @returns {*}
+ */
+AccessContext.prototype.getAppId = function() {
+  for (var i = 0; i < this.principals.length; i++) {
+    var p = this.principals[i];
+    if (p.type === Principal.APPLICATION) {
+      return p.id;
+    }
+  }
+  return null;
+};
+
+/**
+ * Check if the access context has authenticated principals
+ * @returns {boolean}
+ */
+AccessContext.prototype.isAuthenticated = function() {
+  return !!(this.getUserId() || this.getAppId());
+};
+
+/**
+ * This class represents the abstract notion of a principal, which can be used
+ * to represent any entity, such as an individual, a corporation, and a login id
+ * @param {String} type The principal type
+ * @param {*} id The princiapl id
+ * @param {String} [name] The principal name
+ * @returns {Principal}
+ * @constructor
+ */
+function Principal(type, id, name) {
+  if (!(this instanceof Principal)) {
+    return new Principal(type, id, name);
+  }
+  this.type = type;
+  this.id = id;
+  this.name = name;
+}
+
+// Define constants for principal types
+Principal.USER = 'USER';
+Principal.APP = Principal.APPLICATION = 'APP';
+Principal.ROLE = 'ROLE';
+Principal.SCOPE = 'SCOPE';
+
+/**
+ * Compare if two principals are equal
+ * @param p The other principal
+ * @returns {boolean}
+ */
+Principal.prototype.equals = function (p) {
+  if (p instanceof Principal) {
+    return this.type === p.type && String(this.id) === String(p.id);
+  }
+  return false;
+};
+
+/**
+ * A request to access protected resources
+ * @param {String} model The model name
+ * @param {String} property
+ * @param {String} accessType The access type
+ * @param {String} permission The permission
+ * @returns {AccessRequest}
+ * @constructor
+ */
+function AccessRequest(model, property, accessType, permission) {
+  if (!(this instanceof AccessRequest)) {
+    return new AccessRequest(model, property, accessType);
+  }
+  this.model = model || AccessContext.ALL;
+  this.property = property || AccessContext.ALL;
+  this.accessType = accessType || AccessContext.ALL;
+  this.permission = permission || AccessContext.DEFAULT;
+}
+
+/**
+ * Is the request a wildcard
+ * @returns {boolean}
+ */
+AccessRequest.prototype.isWildcard = function () {
+  return this.model === AccessContext.ALL ||
+    this.property === AccessContext.ALL ||
+    this.accessType === AccessContext.ALL;
+};
+
+module.exports.AccessContext = AccessContext;
+module.exports.Principal = Principal;
+module.exports.AccessRequest = AccessRequest;
+
+
+

lib/models/acl.js

@@ -36,6 +36,11 @@ var async = require('async');
 var assert = require('assert');
 var debug = require('debug')('acl');
 
+var ctx = require('./access-context');
+var AccessContext = ctx.AccessContext;
+var Principal = ctx.Principal;
+var AccessRequest = ctx.AccessRequest;
+
 var role = require('./role');
 var Role = role.Role;
 
@@ -102,38 +107,30 @@ var ACLSchema = {
 
 var ACL = loopback.createModel('ACL', ACLSchema);
 
-ACL.ALL = '*';
+ACL.ALL = AccessContext.ALL;
 
-ACL.DEFAULT = 'DEFAULT'; // Not specified
+ACL.DEFAULT = AccessContext.DEFAULT; // Not specified
-ACL.ALLOW = 'ALLOW'; // Allow
+ACL.ALLOW = AccessContext.ALLOW; // Allow
-ACL.ALARM = 'ALARM'; // Warn - send an alarm
+ACL.ALARM = AccessContext.ALARM; // Warn - send an alarm
-ACL.AUDIT = 'AUDIT'; // Audit - record the access
+ACL.AUDIT = AccessContext.AUDIT; // Audit - record the access
-ACL.DENY = 'DENY'; // Deny
+ACL.DENY = AccessContext.DENY; // Deny
 
-ACL.READ = 'READ'; // Read operation
+ACL.READ = AccessContext.READ; // Read operation
-ACL.WRITE = 'WRITE'; // Write operation
+ACL.WRITE = AccessContext.WRITE; // Write operation
-ACL.EXECUTE = 'EXECUTE'; // Execute operation
+ACL.EXECUTE = AccessContext.EXECUTE; // Execute operation
 
-ACL.USER = 'USER';
+ACL.USER = Principal.USER;
-ACL.APP = ACL.APPLICATION = 'APP';
+ACL.APP = ACL.APPLICATION = Principal.APPLICATION;
-ACL.ROLE = 'ROLE';
+ACL.ROLE = Principal.ROLE;
-ACL.SCOPE = 'SCOPE';
+ACL.SCOPE = Principal.SCOPE;
-
-var permissionOrder = {
-  DEFAULT: 0,
-  ALLOW: 1,
-  ALARM: 2,
-  AUDIT: 3,
-  DENY: 4
-};
 
 /**
  * Calculate the matching score for the given rule and request
- * @param {Object} rule The ACL entry
+ * @param {ACL} rule The ACL entry
- * @param {Object} req The request
+ * @param {AccessRequest} req The request
  * @returns {number}
  */
-function getMatchingScore(rule, req) {
+ACL.getMatchingScore = function getMatchingScore(rule, req) {
   var props = ['model', 'property', 'accessType'];
   var score = 0;
   for (var i = 0; i < props.length; i++) {
@@ -155,9 +152,9 @@ function getMatchingScore(rule, req) {
     }
   }
   score = score * 4;
-  score += permissionOrder[rule.permission || ACL.ALLOW] - 1;
+  score += AccessContext.permissionOrder[rule.permission || ACL.ALLOW] - 1;
   return score;
-}
+};
 
 /*!
  * Resolve permission from the ACLs
@@ -165,21 +162,20 @@ function getMatchingScore(rule, req) {
  * @param {Object} req The request
  * @returns {Object} The effective ACL
  */
-function resolvePermission(acls, req) {
+ACL.resolvePermission = function resolvePermission(acls, req) {
+  debug('resolvePermission(): %j %j', acls, req);
   // Sort by the matching score in descending order
   acls = acls.sort(function (rule1, rule2) {
-    return getMatchingScore(rule2, req) - getMatchingScore(rule1, req);
+    return ACL.getMatchingScore(rule2, req) - ACL.getMatchingScore(rule1, req);
   });
   var permission = ACL.DEFAULT;
   var score = 0;
   for (var i = 0; i < acls.length; i++) {
-    score = getMatchingScore(acls[i], req);
+    score = ACL.getMatchingScore(acls[i], req);
     if (score < 0) {
       break;
     }
-    if (req.model !== ACL.ALL &&
+    if (!req.isWildcard()) {
-      req.property !== ACL.ALL &&
-      req.accessType !== ACL.ALL) {
       // We should stop from the first match for non-wildcard
       permission = acls[i].permission;
       break;
@@ -193,19 +189,18 @@ function resolvePermission(acls, req) {
         break;
       }
       // For wildcard match, find the strongest permission
-      if(permissionOrder[acls[i].permission] > permissionOrder[permission]) {
+      if(AccessContext.permissionOrder[acls[i].permission]
+        > AccessContext.permissionOrder[permission]) {
         permission = acls[i].permission;
       }
     }
   }
 
-  return {
+  var res = new AccessRequest(req.model, req.property, req.accessType,
-    model: req.model,
+    permission || ACL.DEFAULT);
-    property: req.property,
+  debug('resolvePermission() returns: %j', res);
-    accessType: req.accessType,
+  return res;
-    permission: permission || ACL.DEFAULT
+};
-  };
-}
 
 /*!
  * Get the static ACLs from the model definition
@@ -214,19 +209,20 @@ function resolvePermission(acls, req) {
  *
  * @return {Object[]} An array of ACLs
  */
-function getStaticACLs(model, property) {
+ACL.getStaticACLs = function getStaticACLs(model, property) {
+  debug('getStaticACLs(): %s %s', model, property);
   var modelClass = loopback.getModel(model);
   var staticACLs = [];
   if (modelClass && modelClass.settings.acls) {
     modelClass.settings.acls.forEach(function (acl) {
-      staticACLs.push({
+      staticACLs.push(new ACL({
         model: model,
         property: acl.property || ACL.ALL,
         principalType: acl.principalType,
         principalId: acl.principalId, // TODO: Should it be a name?
         accessType: acl.accessType,
         permission: acl.permission
-      });
+      }));
     });
   }
   var prop = modelClass &&
@@ -236,19 +232,19 @@ function getStaticACLs(model, property) {
       || modelClass.prototype[property]); // prototype method
   if (prop && prop.acls) {
     prop.acls.forEach(function (acl) {
-      staticACLs.push({
+      staticACLs.push(new ACL({
         model: modelClass.modelName,
         property: property,
         principalType: acl.principalType,
         principalId: acl.principalId,
         accessType: acl.accessType,
         permission: acl.permission
-      });
+      }));
     });
   }
-
+  debug('getStaticACLs() returns: %s', staticACLs);
   return staticACLs;
-}
+};
 
 /**
  * Check if the given principal is allowed to access the model/property
@@ -263,24 +259,25 @@ function getStaticACLs(model, property) {
  * @param {String|Error} err The error object
  * @param {Object} the access permission
  */
-ACL.checkPermission = function (principalType, principalId, model, property, accessType, callback) {
+ACL.checkPermission = function checkPermission(principalType, principalId,
+                                               model, property, accessType,
+                                               callback) {
+  debug('checkPermission(): %s %s %s %s %s', principalType, principalId, model,
+    property, accessType);
   property = property || ACL.ALL;
   var propertyQuery = (property === ACL.ALL) ? undefined : {inq: [property, ACL.ALL]};
   accessType = accessType || ACL.ALL;
   var accessTypeQuery = (accessType === ACL.ALL) ? undefined : {inq: [accessType, ACL.ALL]};
 
-  var req = {
+  var req = new AccessRequest(model, property, accessType);
-    model: model,
-    property: property,
-    accessType: accessType
-  };
 
-  var acls = getStaticACLs(model, property);
+  var acls = ACL.getStaticACLs(model, property);
 
-  var resolved = resolvePermission(acls, req);
+  var resolved = ACL.resolvePermission(acls, req);
 
   if(resolved && resolved.permission === ACL.DENY) {
     // Fail fast
+    debug('checkPermission(): %j', resolved);
     process.nextTick(function() {
       callback && callback(null, resolved);
     });
@@ -295,11 +292,12 @@ ACL.checkPermission = function (principalType, principalId, model, property, acc
         return;
       }
       acls = acls.concat(dynACLs);
-      resolved = resolvePermission(acls, req);
+      resolved = ACL.resolvePermission(acls, req);
       if(resolved && resolved.permission === ACL.DEFAULT) {
         var modelClass = loopback.getModel(model);
         resolved.permission = (modelClass && modelClass.settings.defaultPermission) || ACL.ALLOW;
       }
+      debug('checkPermission(): %j', resolved);
       callback && callback(null, resolved);
     });
 };
@@ -337,31 +335,23 @@ Scope.checkPermission = function (scope, model, property, accessType, callback)
  * @param {Function} callback
  */
 ACL.checkAccess = function (context, callback) {
-  context = context || {};
+  debug('checkAccess(): %j', context);
-  var principals = context.principals || [];
 
-  // add ROLE.EVERYONE
+  if(!(context instanceof AccessContext)) {
-  principals.unshift({principalType: ACL.ROLE, principalId: Role.EVERYONE});
+    context = new AccessContext(context);
+  }
 
   var model = context.model;
-  model = ('string' === typeof model) ? loopback.getModel(model) : model;
-  var id = context.id;
   var property = context.property;
   var accessType = context.accessType;
 
-  property = property || ACL.ALL;
   var propertyQuery = (property === ACL.ALL) ? undefined : {inq: [property, ACL.ALL]};
-  accessType = accessType || ACL.ALL;
   var accessTypeQuery = (accessType === ACL.ALL) ? undefined : {inq: [accessType, ACL.ALL]};
 
-  var req = {
+  var req = new AccessRequest(model.modelName, property, accessType);
-    model: model.modelName,
-    property: property,
-    accessType: accessType
-  };
 
   var effectiveACLs = [];
-  var staticACLs = getStaticACLs(model.modelName, property);
+  var staticACLs = ACL.getStaticACLs(model.modelName, property);
 
   ACL.find({where: {model: model.modelName, property: propertyQuery,
     accessType: accessTypeQuery}}, function (err, acls) {
@@ -374,30 +364,37 @@ ACL.checkAccess = function (context, callback) {
     acls = acls.concat(staticACLs);
 
     acls.forEach(function (acl) {
-      principals.forEach(function (principal) {
+      // Check exact matches
-        if (principal.principalType === acl.principalType
+      for (var i = 0; i < context.principals.length; i++) {
-          && String(principal.principalId) === String(acl.principalId)) {
+        var p = context.principals[i];
+        if (p.type === acl.principalType
+          && String(p.id) === String(acl.principalId)) {
           effectiveACLs.push(acl);
-        } else if (acl.principalType === ACL.ROLE) {
+          return;
-          inRoleTasks.push(function (done) {
-            Role.isInRole(acl.principalId,
-               {principalType: principal.principalType,
-                principalId: principal.principalId,
-                userId: context.userId,
-                model: model, id: id, property: property},
-              function (err, inRole) {
-                if(!err && inRole) {
-                  effectiveACLs.push(acl);
-                }
-                done(err, acl);
-              });
-          });
         }
-      });
+      }
+
+      // Check role matches
+      if (acl.principalType === ACL.ROLE) {
+        inRoleTasks.push(function (done) {
+          Role.isInRole(acl.principalId, context,
+            function (err, inRole) {
+              if (!err && inRole) {
+                effectiveACLs.push(acl);
+              }
+              done(err, acl);
+            });
+        });
+      }
     });
 
-    async.parallel(inRoleTasks, function(err, results) {
+    async.parallel(inRoleTasks, function (err, results) {
-      resolved = resolvePermission(effectiveACLs, req);
+      if(err) {
+        callback && callback(err, null);
+        return;
+      }
+      var resolved = ACL.resolvePermission(effectiveACLs, req);
+      debug('checkAccess() returns: %j', resolved);
       callback && callback(null, resolved);
     });
   });
@@ -416,38 +413,30 @@ ACL.checkAccess = function (context, callback) {
  * @param {String|Error} err The error object
  * @param {Boolean} allowed is the request allowed
  */
-ACL.checkAccessForToken = function(token, model, modelId, method, callback) {
+ACL.checkAccessForToken = function (token, model, modelId, method, callback) {
+  debug('checkAccessForToken(): %j %s %s %s', token, model, modelId, method);
   assert(token, 'Access token is required');
-  var principals = [];
-  if(token.userId) {
-    principals.push({principalType: ACL.USER, principalId: token.userId});
-  }
-  if(token.appId) {
-    principals.push({principalType: ACL.APPLICATION, principalId: token.appId});
-  }
 
-  var modelCtor = loopback.getModel(model);
+  var context = new AccessContext({
-
+    accessToken: token,
-  var context = {
-    userId: token.userId,
-    principals: principals,
     model: model,
     property: method,
-    accessType: modelCtor._getAccessTypeForMethod(method),
+    method: method,
-    id: modelId
+    modelId: modelId
-  };
+  });
 
-  ACL.checkAccess(context, function(err, access) {
+  context.accessType = context.model._getAccessTypeForMethod(method);
-    if(err) {
+
+  ACL.checkAccess(context, function (err, access) {
+    if (err) {
       callback && callback(err);
       return;
     }
+    debug('checkAccessForToken(): %j', access);
     callback && callback(null, access.permission !== ACL.DENY);
   });
 };
 
 
-module.exports = {
+module.exports.ACL = ACL;
-  ACL: ACL,
+module.exports.Scope = Scope;
-  Scope: Scope
-};

lib/models/role.js

@@ -3,6 +3,8 @@ var debug = require('debug')('role');
 var assert = require('assert');
 var async = require('async');
 
+var AccessContext = require('./access-context').AccessContext;
+
 // Role model
 var RoleSchema = {
   id: {type: String, id: true}, // Id
@@ -160,16 +162,16 @@ Role.registerResolver = function(role, resolver) {
 };
 
 Role.registerResolver(Role.OWNER, function(role, context, callback) {
-  if(!context || !context.model || !context.id) {
+  if(!context || !context.model || !context.modelId) {
     process.nextTick(function() {
       callback && callback(null, false);
     });
     return;
   }
   var modelClass = context.model;
-  var id = context.id;
+  var modelId = context.modelId;
-  var userId = context.userId || context.principalId;
+  var userId = context.getUserId();
-  Role.isOwner(modelClass, id, userId, callback);
+  Role.isOwner(modelClass, modelId, userId, callback);
 });
 
 function isUserClass(modelClass) {
@@ -251,13 +253,13 @@ Role.registerResolver(Role.AUTHENTICATED, function(role, context, callback) {
  */
 Role.isAuthenticated = function isAuthenticated(context, callback) {
   process.nextTick(function() {
-    callback && callback(null, !!context.principalId);
+    callback && callback(null, context.isAuthenticated());
   });
 };
 
 Role.registerResolver(Role.UNAUTHENTICATED, function(role, context, callback) {
   process.nextTick(function() {
-    callback && callback(null, !context || !context.principalId);
+    callback && callback(null, !context || !context.isAuthenticated());
   });
 });
 
@@ -276,19 +278,38 @@ Role.registerResolver(Role.EVERYONE, function (role, context, callback) {
  */
 Role.isInRole = function (role, context, callback) {
   debug('isInRole(): %s %j', role, context);
+
+  if (!(context instanceof AccessContext)) {
+    context = new AccessContext(context);
+  }
+
   var resolver = Role.resolvers[role];
-  if(resolver) {
+  if (resolver) {
     debug('Custom resolver found for role %s', role);
     resolver(role, context, callback);
     return;
   }
 
-  var principalType = context.principalType;
+  if (context.principals.length === 0) {
-  var principalId = context.principalId;
+    debug('isInRole() returns: false');
+    process.nextTick(function () {
+      callback && callback(null, false);
+    });
+    return;
+  }
 
-  // Check if it's the same role
+  var inRole = context.principals.some(function (p) {
-  if(principalType === RoleMapping.ROLE && principalId === role) {
+
-    process.nextTick(function() {
+    var principalType = p.type || undefined;
+    var principalId = p.id || undefined;
+
+    // Check if it's the same role
+    return principalType === RoleMapping.ROLE && principalId === role;
+  });
+
+  if (inRole) {
+    debug('isInRole() returns: %j', inRole);
+    process.nextTick(function () {
       callback && callback(null, true);
     });
     return;
@@ -299,21 +320,34 @@ Role.isInRole = function (role, context, callback) {
       callback && callback(err);
       return;
     }
-    if(!result) {
+    if (!result) {
       callback && callback(null, false);
       return;
     }
     debug('Role found: %j', result);
-    RoleMapping.findOne({where: {roleId: result.id, principalType: principalType, principalId: principalId}},
+
-      function (err, result) {
+    // Iterate through the list of principals
-        if (err) {
+    async.some(context.principals, function (p, done) {
-          callback && callback(err);
+      var principalType = p.type || undefined;
-          return;
+      var principalId = p.id || undefined;
-        }
+      if (principalType && principalId) {
-        debug('Role mapping found: %j', result);
+        RoleMapping.findOne({where: {roleId: result.id,
-        callback && callback(null, !!result);
+            principalType: principalType, principalId: principalId}},
-      });
+          function (err, result) {
+            debug('Role mapping found: %j', result);
+            done(!err && result); // The only arg is the result
+          });
+      } else {
+        process.nextTick(function () {
+          done(false);
+        });
+      }
+    }, function (inRole) {
+      debug('isInRole() returns: %j', inRole);
+      callback && callback(null, inRole);
+    });
   });
+
 };
 
 /**
@@ -327,17 +361,25 @@ Role.isInRole = function (role, context, callback) {
  */
 Role.getRoles = function (context, callback) {
   debug('getRoles(): %j', context);
+
+  if(!(context instanceof AccessContext)) {
+    context = new AccessContext(context);
+  }
   var roles = [];
 
+  var addRole = function (role) {
+    if (role && roles.indexOf(role) === -1) {
+      roles.push(role);
+    }
+  };
+
   // Check against the smart roles
   var inRoleTasks = [];
   Object.keys(Role.resolvers).forEach(function (role) {
     inRoleTasks.push(function (done) {
       Role.isInRole(role, context, function (err, inRole) {
         if (!err && inRole) {
-          if (roles.indexOf(role) === -1) {
+          addRole(role);
-            roles.push(role);
-          }
           done();
         } else {
           done(err, null);
@@ -346,31 +388,37 @@ Role.getRoles = function (context, callback) {
     });
   });
 
-  // Check against the role mappings
+  context.principals.forEach(function (p) {
-  var principalType = context.principalType || undefined;
+    // Check against the role mappings
-  var principalId = context.principalId || undefined;
+    var principalType = p.type || undefined;
+    var principalId = p.id || undefined;
 
-  if (principalType && principalId) {
+    // Add the role itself
-    // Please find() treat undefined matches all values
+    if (principalType === RoleMapping.ROLE && principalId) {
-    inRoleTasks.push(function (done) {
+      addRole(principalId);
-      RoleMapping.find({where: {principalType: principalType,
+    }
-        principalId: principalId}}, function (err, mappings) {
+
-        if (err) {
+    if (principalType && principalId) {
-          done && done(err);
+      // Please find() treat undefined matches all values
-          return;
+      inRoleTasks.push(function (done) {
-        }
+        RoleMapping.find({where: {principalType: principalType,
-        mappings.forEach(function (m) {
+          principalId: principalId}}, function (err, mappings) {
-          if (roles.indexOf(m.roleId) === -1) {
+          debug('Role mappings found: %s %j', err, mappings);
-            roles.push(m.roleId);
+          if (err) {
+            done && done(err);
+            return;
           }
+          mappings.forEach(function (m) {
+            addRole(m.roleId);
+          });
+          done && done();
         });
-        done && done();
       });
-    });
+    }
-  }
+  });
 
   async.parallel(inRoleTasks, function (err, results) {
-    debug('getRoles() return: %j %j', err, results);
+    debug('getRoles() returns: %j %j', err, roles);
     callback && callback(err, roles);
   });
 };
@@ -380,3 +428,5 @@ module.exports = {
   RoleMapping: RoleMapping
 };
 
+
+

test/acl.test.js

@@ -238,7 +238,7 @@ describe('security ACLs', function () {
 
               ACL.checkAccess({
                 principals: [
-                  {principalType: ACL.USER, principalId: userId}
+                  {type: ACL.USER, id: userId}
                 ],
                 model: 'Customer',
                 property: 'name',