diff --git a/lib/models/access-context.js b/lib/models/access-context.js index 9bbc1464..72b24810 100644 --- a/lib/models/access-context.js +++ b/lib/models/access-context.js @@ -217,10 +217,19 @@ 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; + if (arguments.length === 1 && typeof model === 'object') { + // The argument is an object that contains all required properties + var obj = model || {}; + this.model = obj.model || AccessContext.ALL; + this.property = obj.property || AccessContext.ALL; + this.accessType = obj.accessType || AccessContext.ALL; + this.permission = obj.permission || AccessContext.DEFAULT; + } else { + this.model = model || AccessContext.ALL; + this.property = property || AccessContext.ALL; + this.accessType = accessType || AccessContext.ALL; + this.permission = permission || AccessContext.DEFAULT; + } if(debug.enabled) { debug('---AccessRequest---'); diff --git a/lib/models/acl.js b/lib/models/acl.js index 883661e7..6a155f9a 100644 --- a/lib/models/acl.js +++ b/lib/models/acl.js @@ -137,6 +137,44 @@ ACL.getMatchingScore = function getMatchingScore(rule, req) { return -1; } } + + // Weigh agaist the principal type + score = score * 4; + switch(rule.principalType) { + case ACL.USER: + score += 4; + break; + case ACL.APP: + score += 3; + break; + case ACL.ROLE: + score += 2; + break; + default: + score +=1; + } + + // Weigh against the roles + score = score * 8; + if(rule.principalType === ACL.ROLE) { + switch(rule.principalId) { + case Role.OWNER: + score += 4; + break; + case Role.RELATED: + score += 3; + break; + case Role.AUTHENTICATED: + case Role.UNAUTHENTICATED: + score += 2; + break; + case Role.EVERYONE: + score += 1; + break; + default: + score += 5; + } + } score = score * 4; score += AccessContext.permissionOrder[rule.permission || ACL.ALLOW] - 1; return score; @@ -149,10 +187,16 @@ ACL.getMatchingScore = function getMatchingScore(rule, req) { * @returns {AccessRequest} result The effective ACL */ ACL.resolvePermission = function resolvePermission(acls, req) { + if(!(req instanceof AccessRequest)) { + req = new AccessRequest(req); + } // Sort by the matching score in descending order 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++) { @@ -351,6 +395,9 @@ ACL.checkAccess = 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); } diff --git a/lib/models/role.js b/lib/models/role.js index e249f780..0af2efd4 100644 --- a/lib/models/role.js +++ b/lib/models/role.js @@ -196,6 +196,21 @@ function isUserClass(modelClass) { modelClass.prototype instanceof loopback.User; } +/*! + * Check if two user ids matches + * @param {*} id1 + * @param {*} id2 + * @returns {boolean} + */ +function matches(id1, id2) { + if (id1 === undefined || id1 === null || id1 ==='' + || id2 === undefined || id2 === null || id2 === '') { + return false; + } + // The id can be a MongoDB ObjectID + return id1 === id2 || id1.toString() === id2.toString(); +} + /** * Check if a given userId is the owner the model instance * @param {Function} modelClass The model class @@ -205,7 +220,7 @@ function isUserClass(modelClass) { */ Role.isOwner = function isOwner(modelClass, modelId, userId, callback) { assert(modelClass, 'Model class is required'); - debug('isOwner(): %s %s %s', modelClass && modelClass.modelName, modelId, userId); + debug('isOwner(): %s %s userId: %s', modelClass && modelClass.modelName, modelId, userId); // No userId is present if(!userId) { process.nextTick(function() { @@ -217,19 +232,21 @@ Role.isOwner = function isOwner(modelClass, modelId, userId, callback) { // Is the modelClass User or a subclass of User? if(isUserClass(modelClass)) { process.nextTick(function() { - callback(null, modelId == userId); + callback(null, matches(modelId, userId)); }); return; } modelClass.findById(modelId, function(err, inst) { if(err || !inst) { + debug('Model not found for id %j', modelId); callback && callback(err, false); return; } debug('Model found: %j', inst); - if(inst.userId || inst.owner) { - callback && callback(null, (inst.userId || inst.owner) === userId); + var ownerId = inst.userId || inst.owner; + if(ownerId) { + callback && callback(null, matches(ownerId, userId)); return; } else { // Try to follow belongsTo @@ -240,7 +257,7 @@ Role.isOwner = function isOwner(modelClass, modelId, userId, callback) { inst[r](function(err, user) { if(!err && user) { debug('User found: %j', user.id); - callback && callback(null, user.id === userId); + callback && callback(null, matches(user.id, userId)); } else { callback && callback(err, false); } @@ -248,6 +265,7 @@ Role.isOwner = function isOwner(modelClass, modelId, userId, callback) { return; } } + debug('No matching belongsTo relation found for model %j and user: %j', modelId, userId); callback && callback(null, false); } }); @@ -401,6 +419,9 @@ Role.getRoles = function (context, callback) { Object.keys(Role.resolvers).forEach(function (role) { inRoleTasks.push(function (done) { self.isInRole(role, context, function (err, inRole) { + if(debug.enabled) { + debug('In role %j: %j', role, inRole); + } if (!err && inRole) { addRole(role); done(); diff --git a/test/access-control.integration.js b/test/access-control.integration.js index c962c666..e96de823 100644 --- a/test/access-control.integration.js +++ b/test/access-control.integration.js @@ -11,6 +11,7 @@ describe('access control - integration', function () { lt.beforeEach.withApp(app); + /* describe('accessToken', function() { // it('should be a sublcass of AccessToken', function () { // assert(app.models.accessToken.prototype instanceof loopback.AccessToken); @@ -135,6 +136,7 @@ describe('access control - integration', function () { return '/api/banks/' + this.bank.id; } }); + */ describe('/accounts', function () { lt.beforeEach.givenModel('account'); @@ -147,6 +149,7 @@ describe('access control - integration', function () { lt.it.shouldBeDeniedWhenCalledUnauthenticated('GET', urlForAccount); lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'GET', urlForAccount); + lt.it.shouldBeDeniedWhenCalledAnonymously('POST', '/api/accounts'); lt.it.shouldBeDeniedWhenCalledUnauthenticated('POST', '/api/accounts'); lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'POST', '/api/accounts'); @@ -156,12 +159,25 @@ describe('access control - integration', function () { lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'PUT', urlForAccount); lt.describe.whenLoggedInAsUser(CURRENT_USER, function() { - beforeEach(function() { - this.url = '/api/accounts/' + this.user.accountId; + beforeEach(function(done) { + var self = this; + + // Create an account under the given user + app.models.account.create({ + userId: self.user.id, + balance: 100 + }, function(err, act) { + self.url = '/api/accounts/' + act.id; + done(); + }); + }); lt.describe.whenCalledRemotely('PUT', '/api/accounts/:id', function() { lt.it.shouldBeAllowed(); }); + lt.describe.whenCalledRemotely('GET', '/api/accounts/:id', function() { + lt.it.shouldBeAllowed(); + }); lt.describe.whenCalledRemotely('DELETE', '/api/accounts/:id', function() { lt.it.shouldBeDenied(); }); diff --git a/test/acl.test.js b/test/acl.test.js index 76449d5f..488638e7 100644 --- a/test/acl.test.js +++ b/test/acl.test.js @@ -70,6 +70,40 @@ describe('security scopes', function () { }); describe('security ACLs', function () { + it('should order ACL entries based on the matching score', function() { + var acls = [ + { + "model": "account", + "accessType": "*", + "permission": "DENY", + "principalType": "ROLE", + "principalId": "$everyone" + }, + { + "model": "account", + "accessType": "*", + "permission": "ALLOW", + "principalType": "ROLE", + "principalId": "$owner" + }, + { + "model": "account", + "accessType": "READ", + "permission": "ALLOW", + "principalType": "ROLE", + "principalId": "$everyone" + }]; + var req = { + model: 'account', + property: 'find', + accessType: 'WRITE' + }; + var perm = ACL.resolvePermission(acls, req); + assert.deepEqual(perm, { model: 'account', + property: 'find', + accessType: 'WRITE', + permission: 'ALLOW' }); + }); it("should allow access to models for the given principal by wildcard", function () { ACL.create({principalType: ACL.USER, principalId: 'u001', model: 'User', property: ACL.ALL, diff --git a/test/fixtures/access-control/models.json b/test/fixtures/access-control/models.json index f814353a..6db81abe 100644 --- a/test/fixtures/access-control/models.json +++ b/test/fixtures/access-control/models.json @@ -23,10 +23,6 @@ "type": "hasMany", "foreignKey": "userId" }, - "account": { - "model": "account", - "type": "belongsTo" - }, "transactions": { "model": "transaction", "type": "hasMany" @@ -103,6 +99,11 @@ "transactions": { "model": "transaction", "type": "hasMany" + }, + "user": { + "model": "user", + "type": "belongsTo", + "foreignKey": "userId" } }, "acls": [