Skip to content

Commit 29c5f20

Browse files
moshemalbajtos
authored andcommitted
Fix ACL check to support model wildcard
1 parent be91ba3 commit 29c5f20

File tree

2 files changed

+62
-17
lines changed

2 files changed

+62
-17
lines changed

common/models/acl.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -478,8 +478,15 @@ module.exports = function(ACL) {
478478
var effectiveACLs = [];
479479
var staticACLs = self.getStaticACLs(model.modelName, property);
480480

481-
this.find({where: {model: model.modelName, property: propertyQuery,
482-
accessType: accessTypeQuery}}, function(err, acls) {
481+
const query = {
482+
where: {
483+
model: {inq: [model.modelName, ACL.ALL]},
484+
property: propertyQuery,
485+
accessType: accessTypeQuery,
486+
},
487+
};
488+
489+
this.find(query, function(err, acls) {
483490
if (err) return callback(err);
484491
var inRoleTasks = [];
485492

test/acl.test.js

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,13 @@ var supertest = require('supertest');
1515
var Role = loopback.Role;
1616
var RoleMapping = loopback.RoleMapping;
1717
var User = loopback.User;
18-
var testModel;
1918
var async = require('async');
2019

2120
// Speed up the password hashing algorithm for tests
2221
User.settings.saltWorkFactor = 4;
2322

2423
var ds = null;
25-
before(function() {
26-
ds = loopback.createDataSource({connector: loopback.Memory});
27-
});
24+
var testModel;
2825

2926
describe('ACL model', function() {
3027
it('provides DEFAULT_SCOPE constant', () => {
@@ -33,16 +30,7 @@ describe('ACL model', function() {
3330
});
3431

3532
describe('security scopes', function() {
36-
beforeEach(function() {
37-
var ds = this.ds = loopback.createDataSource({connector: loopback.Memory});
38-
testModel = loopback.PersistedModel.extend('testModel');
39-
ACL.attachTo(ds);
40-
Role.attachTo(ds);
41-
RoleMapping.attachTo(ds);
42-
User.attachTo(ds);
43-
Scope.attachTo(ds);
44-
testModel.attachTo(ds);
45-
});
33+
beforeEach(setupTestModels);
4634

4735
it('should allow access to models for the given scope by wildcard', function(done) {
4836
Scope.create({name: 'userScope', description: 'access user information'},
@@ -98,6 +86,8 @@ describe('security scopes', function() {
9886
});
9987

10088
describe('security ACLs', function() {
89+
beforeEach(setupTestModels);
90+
10191
it('supports checkPermission() returning a promise', function() {
10292
return ACL.create({
10393
principalType: ACL.USER,
@@ -115,6 +105,44 @@ describe('security ACLs', function() {
115105
});
116106
});
117107

108+
it('supports ACL rules with a wildcard for models', function() {
109+
const A_USER_ID = 'a-test-user';
110+
111+
// By default, access is allowed to all users
112+
return assertPermission(ACL.ALLOW, 'initial state')
113+
// An ACL rule applying to all models denies access to everybody
114+
.then(() => ACL.create({
115+
model: '*',
116+
property: '*',
117+
accessType: '*',
118+
principalType: 'ROLE',
119+
principalId: '$everyone',
120+
permission: 'DENY',
121+
}))
122+
.then(() => assertPermission(ACL.DENY, 'all denied'))
123+
// A rule for a specific model overrides the rule matching all models
124+
.then(() => ACL.create({
125+
model: testModel.modelName,
126+
property: '*',
127+
accessType: '*',
128+
principalType: ACL.USER,
129+
principalId: A_USER_ID,
130+
permission: ACL.ALLOW,
131+
}))
132+
.then(() => assertPermission(ACL.ALLOW, 'only a single model allowed'));
133+
134+
function assertPermission(expectedPermission, msg) {
135+
return ACL.checkAccessForContext({
136+
principals: [{type: ACL.USER, id: A_USER_ID}],
137+
model: testModel.modelName,
138+
accessType: ACL.ALL,
139+
}).then(accessContext => {
140+
const actual = accessContext.isAllowed() ? ACL.ALLOW : ACL.DENY;
141+
expect(actual, msg).to.equal(expectedPermission);
142+
});
143+
}
144+
});
145+
118146
it('supports checkAccessForContext() returning a promise', function() {
119147
var testModel = ds.createModel('testModel', {
120148
acls: [
@@ -399,7 +427,6 @@ describe('security ACLs', function() {
399427
});
400428

401429
it('should check access against LDL, ACL, and Role', function(done) {
402-
// var log = console.log;
403430
var log = function() {};
404431

405432
// Create
@@ -645,3 +672,14 @@ describe('authorized roles propagation in RemotingContext', function() {
645672
.expect(200);
646673
}
647674
});
675+
676+
function setupTestModels() {
677+
ds = this.ds = loopback.createDataSource({connector: loopback.Memory});
678+
testModel = loopback.PersistedModel.extend('testModel');
679+
ACL.attachTo(ds);
680+
Role.attachTo(ds);
681+
RoleMapping.attachTo(ds);
682+
User.attachTo(ds);
683+
Scope.attachTo(ds);
684+
testModel.attachTo(ds);
685+
}

0 commit comments

Comments
 (0)