From c538aa764da5ba2e9d46f1995104721e2b2d7a2d Mon Sep 17 00:00:00 2001 From: Benjamin Kroeger Date: Tue, 16 Aug 2016 17:02:34 +0200 Subject: [PATCH 1/2] resolve related models from correct registry Also modify setup of test servers when ACL was used, force the app to `loadBuiltinModels` with localRegistry. --- common/models/acl.js | 12 ++++---- test/access-control.integration.js | 29 +++++++++++++++++++ .../access-control/common/models/bank.json | 6 ++++ test/fixtures/access-control/server/server.js | 5 +++- .../user-integration-app/server/server.js | 6 +++- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/common/models/acl.js b/common/models/acl.js index e9340d76..19d5241c 100644 --- a/common/models/acl.js +++ b/common/models/acl.js @@ -394,7 +394,8 @@ module.exports = function(ACL) { */ ACL.checkAccessForContext = function(context, callback) { - var registry = this.registry; + this.resolveRelatedModels(); + var roleModel = this.roleModel; if (!(context instanceof AccessContext)) { context = new AccessContext(context); @@ -420,7 +421,6 @@ module.exports = function(ACL) { var staticACLs = this.getStaticACLs(model.modelName, property); var self = this; - var roleModel = registry.getModelByType(Role); this.find({where: {model: model.modelName, property: propertyQuery, accessType: accessTypeQuery}}, function(err, acls) { if (err) { @@ -507,10 +507,10 @@ module.exports = function(ACL) { ACL.resolveRelatedModels = function() { if (!this.roleModel) { var reg = this.registry; - this.roleModel = reg.getModelByType(loopback.Role); - this.roleMappingModel = reg.getModelByType(loopback.RoleMapping); - this.userModel = reg.getModelByType(loopback.User); - this.applicationModel = reg.getModelByType(loopback.Application); + this.roleModel = reg.getModelByType('Role'); + this.roleMappingModel = reg.getModelByType('RoleMapping'); + this.userModel = reg.getModelByType('User'); + this.applicationModel = reg.getModelByType('Application'); } }; diff --git a/test/access-control.integration.js b/test/access-control.integration.js index 3fe2e9c5..b2fe3311 100644 --- a/test/access-control.integration.js +++ b/test/access-control.integration.js @@ -152,6 +152,34 @@ describe('access control - integration', function() { }); describe('/banks', function() { + var SPECIAL_USER = { email: 'special@test.test', password: 'test' }; + + // define dynamic role that would only grant access when the authenticated user's email is equal to + // SPECIAL_USER's email + + before(function() { + var roleModel = app.registry.getModel('Role'); + var userModel = app.registry.getModel('user'); + + roleModel.registerResolver('$dynamic-role', function(role, context, callback) { + if (!(context && context.accessToken && context.accessToken.userId)) { + return process.nextTick(function() { + callback && callback(null, false); + }); + } + var accessToken = context.accessToken; + userModel.findById(accessToken.userId, function(err, user) { + if (err) { + return callback(err, false); + } + if (user && user.email === SPECIAL_USER.email) { + return callback(null, true); + } + return callback(null, false); + }); + }); + }); + lt.beforeEach.givenModel('bank'); lt.it.shouldBeAllowedWhenCalledAnonymously('GET', '/api/banks'); @@ -173,6 +201,7 @@ describe('access control - integration', function() { lt.it.shouldBeDeniedWhenCalledAnonymously('DELETE', urlForBank); lt.it.shouldBeDeniedWhenCalledUnauthenticated('DELETE', urlForBank); lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'DELETE', urlForBank); + lt.it.shouldBeAllowedWhenCalledByUser(SPECIAL_USER, 'DELETE', urlForBank); function urlForBank() { return '/api/banks/' + this.bank.id; diff --git a/test/fixtures/access-control/common/models/bank.json b/test/fixtures/access-control/common/models/bank.json index a06555b8..f31c7876 100644 --- a/test/fixtures/access-control/common/models/bank.json +++ b/test/fixtures/access-control/common/models/bank.json @@ -22,6 +22,12 @@ "permission": "ALLOW", "principalType": "ROLE", "principalId": "$everyone" + }, + { + "accessType": "WRITE", + "permission": "ALLOW", + "principalType": "ROLE", + "principalId": "$dynamic-role" } ], "properties": {} diff --git a/test/fixtures/access-control/server/server.js b/test/fixtures/access-control/server/server.js index ba596a36..988fb5c3 100644 --- a/test/fixtures/access-control/server/server.js +++ b/test/fixtures/access-control/server/server.js @@ -5,7 +5,10 @@ var loopback = require('../../../..'); var boot = require('loopback-boot'); -var app = module.exports = loopback({ localRegistry: true }); +var app = module.exports = loopback({ + localRegistry: true, + loadBuiltinModels: true +}); boot(app, __dirname); diff --git a/test/fixtures/user-integration-app/server/server.js b/test/fixtures/user-integration-app/server/server.js index bfd7e1af..4f636e7a 100644 --- a/test/fixtures/user-integration-app/server/server.js +++ b/test/fixtures/user-integration-app/server/server.js @@ -5,7 +5,11 @@ var loopback = require('../../../../index'); var boot = require('loopback-boot'); -var app = module.exports = loopback({ localRegistry: true }); +var app = module.exports = loopback({ + localRegistry: true, + loadBuiltinModels: true +}); + app.enableAuth(); boot(app, __dirname); app.use(loopback.token({model: app.models.AccessToken})); From ecd881a0f3bf77348b051bafec4742b5eb85a223 Mon Sep 17 00:00:00 2001 From: Benjamin Kroeger Date: Tue, 16 Aug 2016 17:03:21 +0200 Subject: [PATCH 2/2] streamline use if `self` --- common/models/acl.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/models/acl.js b/common/models/acl.js index 19d5241c..96cd7929 100644 --- a/common/models/acl.js +++ b/common/models/acl.js @@ -394,8 +394,9 @@ module.exports = function(ACL) { */ ACL.checkAccessForContext = function(context, callback) { - this.resolveRelatedModels(); - var roleModel = this.roleModel; + var self = this; + self.resolveRelatedModels(); + var roleModel = self.roleModel; if (!(context instanceof AccessContext)) { context = new AccessContext(context); @@ -418,10 +419,9 @@ module.exports = function(ACL) { var req = new AccessRequest(modelName, property, accessType, ACL.DEFAULT, methodNames); var effectiveACLs = []; - var staticACLs = this.getStaticACLs(model.modelName, property); + var staticACLs = self.getStaticACLs(model.modelName, property); - var self = this; - this.find({where: {model: model.modelName, property: propertyQuery, + self.find({where: {model: model.modelName, property: propertyQuery, accessType: accessTypeQuery}}, function(err, acls) { if (err) { if (callback) callback(err);