From 8aa98a80efaf5871b2af21056916038e082501c8 Mon Sep 17 00:00:00 2001 From: ebarault Date: Tue, 6 Dec 2016 15:24:49 +0100 Subject: [PATCH] Propagate authorized roles in remoting context Adds an authorizedRoles object to remotingContext.args.options which contains all the roles (static and dynamic) that are granted to the user when performing a request through strong-remoting to an app with authentication enabled. The authorizedRoles object for example looks like: { $everyone: true, $authenticated: true, myRole: true } NOTE: this pr also covers a number of jsdoc fixes as well as refactoring in ACL.js and access-context.js --- common/models/acl.js | 118 ++++++++++++++++++++++++++++-------- lib/access-context.js | 68 +++++++++++++++++---- test/acl.test.js | 137 ++++++++++++++++++++++++++++++++++++++++++ test/model.test.js | 2 +- 4 files changed, 287 insertions(+), 38 deletions(-) diff --git a/common/models/acl.js b/common/models/acl.js index 4b1e7c27b..e849312fb 100644 --- a/common/models/acl.js +++ b/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 - * @returns {AccessRequest} result The effective ACL + * @param {AccessRequest} req The access request + * @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 {AccessRequest} result The access permission + * @param {String|Error} err The error object. + * @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. - * @param {Function} callback Callback function + * READ, REPLICATE, WRITE, or EXECUTE. + * @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(); diff --git a/lib/access-context.js b/lib/access-context.js index 96bef34df..156620346 100644 --- a/lib/access-context.js +++ b/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 {AccessToken} accessToken The access token - * + * @property {String} accessType The access type: READ, REPLICATE, WRITE, or EXECUTE. + * @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,
+ * 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? * diff --git a/test/acl.test.js b/test/acl.test.js index 4f71764a2..15cd87065 100644 --- a/test/acl.test.js +++ b/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); + } +}); diff --git a/test/model.test.js b/test/model.test.js index 04fdba11c..9b7b32354 100644 --- a/test/model.test.js +++ b/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(); }); });