Merge pull request #2957 from ebarault/propagate-resolved-roles-in-context

propagate resolved roles in remoting context

common/models/acl.js

@@ -34,6 +34,7 @@ var g = require('../../lib/globalize');
 var loopback = require('../../lib/loopback');
 var utils = require('../../lib/utils');
 var async = require('async');
+var extend = require('util')._extend;
 var assert = require('assert');
 var debug = require('debug')('loopback:security:acl');
 
@@ -204,11 +205,12 @@ module.exports = function(ACL) {
   /*!
    * Resolve permission from the ACLs
    * @param {Object[]) acls The list of ACLs
-   * @param {Object} req The request
+   * @param {AccessRequest} req The access request
-   * @returns {AccessRequest} result The effective ACL
+   * @returns {AccessRequest} result The resolved access request
    */
   ACL.resolvePermission = function resolvePermission(acls, req) {
     if (!(req instanceof AccessRequest)) {
+      req.registry = this.registry;
       req = new AccessRequest(req);
     }
     // Sort by the matching score in descending order
@@ -250,9 +252,16 @@ module.exports = function(ACL) {
         debug('with score:', acl.score(req));
       });
     }
+    var res = new AccessRequest({
+      model: req.model,
+      property: req.property,
+      accessType: req.accessType,
+      permission: permission || ACL.DEFAULT,
+      registry: this.registry});
+
+    // Elucidate permission status if DEFAULT
+    res.settleDefaultPermission();
 
-    var res = new AccessRequest(req.model, req.property, req.accessType,
-        permission || ACL.DEFAULT);
     return res;
   };
 
@@ -316,8 +325,8 @@ module.exports = function(ACL) {
    * @param {String} property The property/method/relation name.
    * @param {String} accessType The access type.
    * @callback {Function} callback Callback function.
-   * @param {String|Error} err The error object
+   * @param {String|Error} err The error object.
-   * @param {AccessRequest} result The access permission
+   * @param {AccessRequest} result The resolved access request.
    */
   ACL.checkPermission = function checkPermission(principalType, principalId,
                                                  model, property, accessType,
@@ -332,10 +341,11 @@ module.exports = function(ACL) {
     var accessTypeQuery = (accessType === ACL.ALL) ? undefined :
       {inq: [accessType, ACL.ALL, ACL.EXECUTE]};
 
-    var req = new AccessRequest(model, property, accessType);
+    var req = new AccessRequest({model, property, accessType, registry: this.registry});
 
     var acls = this.getStaticACLs(model, property);
 
+    // resolved is an instance of AccessRequest
     var resolved = this.resolvePermission(acls, req);
 
     if (resolved && resolved.permission === ACL.DENY) {
@@ -355,11 +365,8 @@ module.exports = function(ACL) {
           return callback(err);
         }
         acls = acls.concat(dynACLs);
+        // resolved is an instance of AccessRequest
         resolved = self.resolvePermission(acls, req);
-        if (resolved && resolved.permission === ACL.DEFAULT) {
-          var modelClass = self.registry.findModel(model);
-          resolved.permission = (modelClass && modelClass.settings.defaultPermission) || ACL.ALLOW;
-        }
         return callback(null, resolved);
       });
     return callback.promise;
@@ -377,30 +384,63 @@ module.exports = function(ACL) {
     }
   };
 
+  // NOTE Regarding ACL.isAllowed() and ACL.prototype.isAllowed()
+  // Extending existing logic, including from ACL.checkAccessForContext() method,
+  // ACL instance with missing property `permission` are not promoted to
+  // permission = ACL.DEFAULT config. Such ACL instances will hence always be
+  // inefective
+
+  /**
+   * Test if ACL's permission is ALLOW
+   * @param {String} permission The permission to test, expects one of 'ALLOW', 'DENY', 'DEFAULT'
+   * @param {String} defaultPermission The default permission to apply if not providing a finite one in the permission parameter
+   * @returns {Boolean} true if ACL permission is ALLOW
+   */
+  ACL.isAllowed = function(permission, defaultPermission) {
+    if (permission === ACL.DEFAULT) {
+      permission = defaultPermission || ACL.ALLOW;
+    }
+    return permission !== loopback.ACL.DENY;
+  };
+
+  /**
+   * Test if ACL's permission is ALLOW
+   * @param {String} defaultPermission The default permission to apply if missing in ACL instance
+   * @returns {Boolean} true if ACL permission is ALLOW
+   */
+  ACL.prototype.isAllowed = function(defaultPermission) {
+    return this.constructor.isAllowed(this.permission, defaultPermission);
+  };
+
   /**
    * Check if the request has the permission to access.
-   * @options {Object} context See below.
+   * @options {AccessContext|Object} context
+   * An AccessContext instance or a plain object with the following properties.
    * @property {Object[]} principals An array of principals.
    * @property {String|Model} model The model name or model class.
-   * @property {*} id The model instance ID.
+   * @property {*} modelId The model instance ID.
    * @property {String} property The property/method/relation name.
    * @property {String} accessType The access type:
-   *   READ, REPLICATE, WRITE, or EXECUTE.
+   * READ, REPLICATE, WRITE, or EXECUTE.
-   * @param {Function} callback Callback function
+   * @callback {Function} callback Callback function
+   * @param {String|Error} err The error object.
+   * @param {AccessRequest} result The resolved access request.
    */
-
   ACL.checkAccessForContext = function(context, callback) {
     if (!callback) callback = utils.createPromiseCallback();
     var self = this;
     self.resolveRelatedModels();
     var roleModel = self.roleModel;
 
-    context.registry = this.registry;
     if (!(context instanceof AccessContext)) {
+      context.registry = this.registry;
       context = new AccessContext(context);
     }
 
+    var authorizedRoles = {};
+    var remotingContext = context.remotingContext;
     var model = context.model;
+    var modelDefaultPermission = model && model.settings.defaultPermission;
     var property = context.property;
     var accessType = context.accessType;
     var modelName = context.modelName;
@@ -414,7 +454,13 @@ module.exports = function(ACL) {
         {inq: [ACL.REPLICATE, ACL.WRITE, ACL.ALL]} :
         {inq: [accessType, ACL.ALL]};
 
-    var req = new AccessRequest(modelName, property, accessType, ACL.DEFAULT, methodNames);
+    var req = new AccessRequest({
+      model: modelName,
+      property,
+      accessType,
+      permission: ACL.DEFAULT,
+      methodNames,
+      registry: this.registry});
 
     var effectiveACLs = [];
     var staticACLs = self.getStaticACLs(model.modelName, property);
@@ -445,6 +491,9 @@ module.exports = function(ACL) {
               function(err, inRole) {
                 if (!err && inRole) {
                   effectiveACLs.push(acl);
+                  // add the role to authorizedRoles if allowed
+                  if (acl.isAllowed(modelDefaultPermission))
+                    authorizedRoles[acl.principalId] = true;
                 }
                 done(err, acl);
               });
@@ -455,18 +504,31 @@ module.exports = function(ACL) {
       async.parallel(inRoleTasks, function(err, results) {
         if (err) return callback(err, null);
 
+        // resolved is an instance of AccessRequest
         var resolved = self.resolvePermission(effectiveACLs, req);
-        if (resolved && resolved.permission === ACL.DEFAULT) {
-          resolved.permission = (model && model.settings.defaultPermission) || ACL.ALLOW;
-        }
         debug('---Resolved---');
         resolved.debug();
+
+        // set authorizedRoles in remotingContext options argument if
+        // resolved AccessRequest permission is ALLOW, else set it to empty object
+        authorizedRoles = resolved.isAllowed() ? authorizedRoles : {};
+        saveAuthorizedRolesToRemotingContext(remotingContext, authorizedRoles);
         return callback(null, resolved);
       });
     });
     return callback.promise;
   };
 
+  function saveAuthorizedRolesToRemotingContext(remotingContext, authorizedRoles) {
+    const options = remotingContext && remotingContext.args && remotingContext.args.options;
+    // authorizedRoles key/value map is added to the options argument only if
+    // the latter exists and is an object. This means that the feature's availability
+    // will depend on the app configuration
+    if (options && typeof options === 'object') { // null is object too
+      options.authorizedRoles = authorizedRoles;
+    }
+  }
+
   /**
    * Check if the given access token can invoke the method
    * @param {AccessToken} token The access token
@@ -489,9 +551,9 @@ module.exports = function(ACL) {
       modelId: modelId,
     });
 
-    this.checkAccessForContext(context, function(err, access) {
+    this.checkAccessForContext(context, function(err, accessRequest) {
       if (err) callback(err);
-      else callback(null, access.permission !== ACL.DENY);
+      else callback(null, accessRequest.isAllowed());
     });
     return callback.promise;
   };
@@ -510,7 +572,9 @@ module.exports = function(ACL) {
    * Resolve a principal by type/id
    * @param {String} type Principal type - ROLE/APP/USER
    * @param {String|Number} id Principal id or name
-   * @param {Function} cb Callback function
+   * @callback {Function} callback Callback function
+   * @param {String|Error} err The error object
+   * @param {Object} result An instance of principal (Role, Application or User)
    */
   ACL.resolvePrincipal = function(type, id, cb) {
     cb = cb || utils.createPromiseCallback();
@@ -530,7 +594,7 @@ module.exports = function(ACL) {
           {where: {or: [{name: id}, {email: id}, {id: id}]}}, cb);
         break;
       default:
-        // try resolving a user model that matches principalType
+        // try resolving a user model with a name matching the principalType
         var userModel = this.registry.findModel(type);
         if (userModel) {
           userModel.findOne(
@@ -553,7 +617,9 @@ module.exports = function(ACL) {
    * @param {String} principalType Principal type
    * @param {String|*} principalId Principal id/name
    * @param {String|*} role Role id/name
-   * @param {Function} cb Callback function
+   * @callback {Function} callback Callback function
+   * @param {String|Error} err The error object
+   * @param {Boolean} isMapped is the ACL mapped to the role
    */
   ACL.isMappedToRole = function(principalType, principalId, role, cb) {
     cb = cb || utils.createPromiseCallback();

lib/access-context.js

@@ -12,17 +12,29 @@ var debug = require('debug')('loopback:security:access-context');
  * Access context represents the context for a request to access protected
  * resources
  *
+ * NOTE While the method expects an array of principals in the AccessContext instance/object,
+ * it also accepts a single principal defined with the following properties:
+ * ```js
+ * {
+ *   // AccessContext instance/object
+ *   // ..
+ *   principalType: 'somePrincipalType', // APP, ROLE, USER, or custom user model name
+ *   principalId: 'somePrincipalId',
+ * }
+ * ```
+ *
  * @class
- * @options {Object} context The context object
+ * @options {AccessContext|Object} context An AccessContext instance or an object
  * @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 {*} 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 {String} accessType The access type: READ, REPLICATE, WRITE, or EXECUTE.
- * @property {AccessToken} accessToken The access token
+ * @property {AccessToken} accessToken The access token resolved for the request
- *
+ * @property {RemotingContext} remotingContext The request's remoting context
+ * @property {Registry} registry The application or global registry
  * @returns {AccessContext}
  * @constructor
  */
@@ -250,16 +262,23 @@ Principal.prototype.equals = function(p) {
 
 /**
  * A request to access protected resources.
- * @param {String} model The model name
+ *
- * @param {String} property
+ * The method can either be called with the following signature or with a single
+ * argument: an AccessRequest instance or an object containing all the required properties.
+ *
+ * @class
+ * @options {String|AccessRequest|Object} model|req The model name,<br>
+ *    or an AccessRequest instance/object.
+ * @param {String} property The property/method/relation name
  * @param {String} accessType The access type
  * @param {String} permission The requested permission
+ * @param {String[]} methodNames The names of involved methods
+ * @param {Registry} registry The application or global registry
  * @returns {AccessRequest}
- * @class
  */
-function AccessRequest(model, property, accessType, permission, methodNames) {
+function AccessRequest(model, property, accessType, permission, methodNames, registry) {
   if (!(this instanceof AccessRequest)) {
-    return new AccessRequest(model, property, accessType);
+    return new AccessRequest(model, property, accessType, permission, methodNames);
   }
   if (arguments.length === 1 && typeof model === 'object') {
     // The argument is an object that contains all required properties
@@ -268,14 +287,19 @@ function AccessRequest(model, property, accessType, permission, methodNames) {
     this.property = obj.property || AccessContext.ALL;
     this.accessType = obj.accessType || AccessContext.ALL;
     this.permission = obj.permission || AccessContext.DEFAULT;
-    this.methodNames = methodNames || [];
+    this.methodNames = obj.methodNames || [];
+    this.registry = obj.registry;
   } else {
     this.model = model || AccessContext.ALL;
     this.property = property || AccessContext.ALL;
     this.accessType = accessType || AccessContext.ALL;
     this.permission = permission || AccessContext.DEFAULT;
     this.methodNames = methodNames || [];
+    this.registry = registry;
   }
+  // do not create AccessRequest without a registry
+  assert(this.registry,
+    'Application registry is mandatory in AccessRequest but missing in provided argument(s)');
 }
 
 /**
@@ -308,6 +332,28 @@ AccessRequest.prototype.exactlyMatches = function(acl) {
   return false;
 };
 
+/**
+ * Settle the accessRequest's permission if DEFAULT
+ * In most situations, the default permission can be resolved from the nested model
+ * config. An default permission can also be explicitly provided to override it or
+ * cope with AccessRequest instances without a nested model (e.g. model is '*')
+ *
+ * @param {String} defaultPermission (optional) the default permission to apply
+ */
+
+AccessRequest.prototype.settleDefaultPermission = function(defaultPermission) {
+  if (this.permission !== 'DEFAULT')
+    return;
+
+  var modelName = this.model;
+  if (!defaultPermission) {
+    var modelClass = this.registry.findModel(modelName);
+    defaultPermission = modelClass && modelClass.settings.defaultPermission;
+  }
+
+  this.permission = defaultPermission || 'ALLOW';
+};
+
 /**
  * Is the request for access allowed?
  *

test/acl.test.js

@@ -5,10 +5,13 @@
 
 'use strict';
 var assert = require('assert');
+var expect = require('./helpers/expect');
 var loopback = require('../index');
 var Scope = loopback.Scope;
 var ACL = loopback.ACL;
 var request = require('supertest');
+var Promise = require('bluebird');
+var supertest = require('supertest');
 var Role = loopback.Role;
 var RoleMapping = loopback.RoleMapping;
 var User = loopback.User;
@@ -157,11 +160,24 @@ describe('security ACLs', function() {
     acls = acls.map(function(a) { return new ACL(a); });
 
     var perm = ACL.resolvePermission(acls, req);
+    // remove the registry from AccessRequest instance to ease asserting
+    delete perm.registry;
     assert.deepEqual(perm, {model: 'account',
       property: 'find',
       accessType: 'WRITE',
       permission: 'ALLOW',
       methodNames: []});
+
+    // NOTE: when fixed in chaijs, use this implement rather than modifying
+    // the resolved access request
+    //
+    // expect(perm).to.deep.include({
+    //   model: 'account',
+    //   property: 'find',
+    //   accessType: 'WRITE',
+    //   permission: 'ALLOW',
+    //   methodNames: [],
+    // });
   });
 
   it('should allow access to models for the given principal by wildcard', function() {
@@ -250,6 +266,7 @@ describe('security ACLs', function() {
       ],
     });
 
+    // ACL default permission is to DENY for model Customer
     Customer.settings.defaultPermission = ACL.DENY;
 
     ACL.checkPermission(ACL.USER, 'u001', 'Customer', 'name', ACL.WRITE,
@@ -472,3 +489,123 @@ describe('access check', function() {
       });
   });
 });
+
+describe('authorized roles propagation in RemotingContext', function() {
+  var app, request, accessToken;
+  var models = {};
+
+  beforeEach(setupAppAndRequest);
+
+  it('contains all authorized roles for a principal if query is allowed', function() {
+    return createACLs('MyTestModel', [
+      {permission: ACL.ALLOW, principalId: '$everyone'},
+      {permission: ACL.ALLOW, principalId: '$authenticated'},
+      {permission: ACL.ALLOW, principalId: 'myRole'},
+    ])
+    .then(makeAuthorizedHttpRequestOnMyTestModel)
+    .then(function() {
+      var ctx = models.MyTestModel.lastRemotingContext;
+      expect(ctx.args.options.authorizedRoles).to.eql(
+        {
+          $everyone: true,
+          $authenticated: true,
+          myRole: true,
+        }
+      );
+    });
+  });
+
+  it('does not contain any denied role even if query is allowed', function() {
+    return createACLs('MyTestModel', [
+      {permission: ACL.ALLOW, principalId: '$everyone'},
+      {permission: ACL.DENY,  principalId: '$authenticated'},
+      {permission: ACL.ALLOW, principalId: 'myRole'},
+    ])
+    .then(makeAuthorizedHttpRequestOnMyTestModel)
+    .then(function() {
+      var ctx = models.MyTestModel.lastRemotingContext;
+      expect(ctx.args.options.authorizedRoles).to.eql(
+        {
+          $everyone: true,
+          myRole: true,
+        }
+      );
+    });
+  });
+
+  it('honors default permission setting', function() {
+    // default permission is set to DENY for MyTestModel
+    models.MyTestModel.settings.defaultPermission = ACL.DENY;
+
+    return createACLs('MyTestModel', [
+      {permission: ACL.DEFAULT, principalId: '$everyone'},
+      {permission: ACL.DENY,    principalId: '$authenticated'},
+      {permission: ACL.ALLOW,   principalId: 'myRole'},
+    ])
+    .then(makeAuthorizedHttpRequestOnMyTestModel)
+    .then(function() {
+      var ctx = models.MyTestModel.lastRemotingContext;
+      expect(ctx.args.options.authorizedRoles).to.eql(
+        // '$everyone' is not expected as default permission is DENY
+        {myRole: true}
+      );
+    });
+  });
+
+  // helpers
+  function setupAppAndRequest() {
+    app = loopback({localRegistry: true, loadBuiltinModels: true});
+    app.use(loopback.rest());
+    app.set('remoting', {errorHandler: {debug: true, log: true}});
+    app.dataSource('db', {connector: 'memory'});
+    request = supertest(app);
+
+    app.enableAuth({dataSource: 'db'});
+    models = app.models;
+
+    // creating a custom model
+    const MyTestModel = app.registry.createModel('MyTestModel');
+    app.model(MyTestModel, {dataSource: 'db'});
+
+    // capturing the value of the last remoting context
+    models.MyTestModel.beforeRemote('find', function(ctx, unused, next) {
+      models.MyTestModel.lastRemotingContext = ctx;
+      next();
+    });
+
+    // creating a user, a role and a rolemapping binding that user with that role
+    return Promise.all([
+      models.User.create({username: 'myUser', email: 'myuser@example.com', password: 'pass'}),
+      models.Role.create({name: 'myRole'}),
+    ])
+    .spread(function(myUser, myRole) {
+      return Promise.all([
+        myRole.principals.create({principalType: 'USER', principalId: myUser.id}),
+        models.User.login({username: 'myUser', password: 'pass'}),
+      ]);
+    })
+    .spread(function(role, token) {
+      accessToken = token;
+    });
+  }
+
+  function createACLs(model, acls) {
+    acls = acls.map(function(acl) {
+      return models.ACL.create({
+        principalType: acl.principalType || ACL.ROLE,
+        principalId: acl.principalId,
+        model: acl.model || model,
+        property: acl.property || ACL.ALL,
+        accessType: acl.accessType || ACL.ALL,
+        permission: acl.permission,
+      });
+    });
+    return Promise.all(acls);
+  };
+
+  function makeAuthorizedHttpRequestOnMyTestModel() {
+    return request.get('/MyTestModels')
+      .set('X-Access-Token', accessToken.id)
+      .expect(200);
+  }
+});

test/model.test.js

@@ -897,7 +897,7 @@ describe.onServer('Remote Methods', function() {
       request(app).get('/TestModels/saveOptions')
         .expect(204, function(err) {
           if (err) return done(err);
-          expect(actualOptions).to.eql({accessToken: null});
+          expect(actualOptions).to.include({accessToken: null});
           done();
         });
     });