Skip to content

Commit e7a9a4e

Browse files
author
Tom Kirkpatrick
committed
Add new setting "applyToStatic" to make the application of filtering on static methods optional
1 parent 4f413e6 commit e7a9a4e

File tree

4 files changed

+100
-65
lines changed

4 files changed

+100
-65
lines changed

README.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,18 @@ Options:
117117

118118
[String] : The name of the model that should be used to store and check group access roles. *(default: 'GroupAccess')*
119119

120+
- `foreignKey`
121+
122+
[String] : The foreign key that should be used to determine which access group a model belongs to. *(default: 'groupId')*
123+
120124
- `groupRoles`
121125

122126
[Array] : A list of group names. *(default: [ '$group:admin', '$group:member' ])*
123127

124-
- `foreignKey`
128+
- `applyToStatic`
129+
130+
[Boolean] : Set to *true* to apply ACLs to static methods (by means of query filtering). *(default: false)*
125131

126-
[String] : The foreign key that should be used to determine which access group a model belongs to. *(default: 'groupId')*
127132

128133
## Tests
129134

lib/index.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,10 @@ module.exports = function loopbackComponentAccess(app, options) {
2727
// Set up role resolvers.
2828
accessUtils.setupRoleResolvers();
2929

30-
// TODO: Rethink wether access filtering can be applied safely.
31-
// Set up model opertion hooks
32-
// accessUtils.setupModels();
30+
// Set up model opertion hooks.
31+
if (options.applyToStatic) {
32+
accessUtils.setupModels();
33+
}
3334

3435
// TODO: Create Group Access model automatically if one hasn't been specified
3536
};

lib/utils.js

Lines changed: 87 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,14 @@ module.exports = class AccessUtils {
1313
this.options = _defaults({ }, options, {
1414
userModel: 'User',
1515
roleModel: 'Role',
16-
groupModel: 'Group',
1716
groupAccessModel: 'GroupAccess',
17+
groupModel: 'Group',
18+
foreignKey: 'groupId',
1819
groupRoles: [
1920
'$group:admin',
2021
'$group:member'
21-
]
22+
],
23+
applyToStatic: false
2224
});
2325
// Default the foreignKey to the group model name + Id.
2426
this.options.foreignKey = this.options.foreignKey || `${this.options.groupModel.toLowerCase()}Id`;
@@ -279,21 +281,8 @@ module.exports = class AccessUtils {
279281
const GroupAccess = this.app.models[this.options.groupAccessModel];
280282
const scope = { };
281283

282-
if (userId) {
283-
this.app.loopback.getCurrentContext().set('groupAccessApplied', true);
284-
}
285-
286284
debug(`Role resolver for ${role}: evaluate ${modelClass.modelName} with id: ${modelId} for user: ${userId}`);
287285

288-
if (!context || !context.model || !context.modelId) {
289-
process.nextTick(() => {
290-
debug('Deny access (context: %s, context.model: %s, context.modelId: %s)',
291-
!!context, !!context.model, !!context.modelId);
292-
if (cb) cb(null, false);
293-
});
294-
return;
295-
}
296-
297286
// No userId is present
298287
if (!userId) {
299288
process.nextTick(() => {
@@ -303,53 +292,92 @@ module.exports = class AccessUtils {
303292
return;
304293
}
305294

306-
return this.isGroupMemberWithRole(modelClass, modelId, userId, roleName)
307-
.then(res => {
308-
cb(null, res);
295+
this.app.loopback.getCurrentContext().set('groupAccessApplied', true);
296+
297+
/**
298+
* Basic application that does not cover static methods.
299+
* Similar to $owner.
300+
*/
301+
if (!this.options.applyToStatic) {
302+
if (!context || !context.model || !context.modelId) {
303+
process.nextTick(() => {
304+
debug('Deny access (context: %s, context.model: %s, context.modelId: %s)',
305+
!!context, !!context.model, !!context.modelId);
306+
if (cb) cb(null, false);
307+
});
308+
return;
309+
}
310+
311+
return this.isGroupMemberWithRole(modelClass, modelId, userId, roleName)
312+
.then(res => {
313+
cb(null, res);
314+
})
315+
.catch(cb);
316+
}
317+
318+
/**
319+
* More complex application that also covers static methods.
320+
*/
321+
return Promise.join(this.getCurrentGroupId(context), this.getTargetGroupId(context),
322+
(currentGroupId, targetGroupId) => {
323+
if (!currentGroupId) {
324+
// TODO: Use promise cancellation to abort the chain early.
325+
// Causes the access check to be bypassed (see below).
326+
return [ false ];
327+
}
328+
329+
scope.currentGroupId = currentGroupId;
330+
scope.targetGroupId = targetGroupId;
331+
const actions = [ ];
332+
const conditions = {
333+
userId: userId,
334+
role: roleName
335+
};
336+
337+
conditions[this.options.foreignKey] = currentGroupId;
338+
actions.push(GroupAccess.count(conditions));
339+
340+
// If this is an attempt to save the item into a new group, check the user has access to the target group.
341+
if (targetGroupId && targetGroupId !== currentGroupId) {
342+
conditions[this.options.foreignKey] = targetGroupId;
343+
actions.push(GroupAccess.count(conditions));
344+
}
345+
346+
return actions;
347+
})
348+
.spread((currentGroupCount, targetGroupCount) => {
349+
let res = false;
350+
351+
if (currentGroupCount === false) {
352+
// No group context was determined, so allow passthrough access.
353+
res = true;
354+
}
355+
else {
356+
// Determine grant based on the current/target group context.
357+
res = currentGroupCount > 0;
358+
359+
debug(`user ${userId} ${res ? 'is a' : 'is not a'} ${roleName} of group ${scope.currentGroupId}`);
360+
361+
// If it's an attempt to save into a new group, also ensure the user has access to the target group.
362+
if (scope.targetGroupId && scope.targetGroupId !== scope.currentGroupId) {
363+
const tMember = targetGroupCount > 0;
364+
365+
debug(`user ${userId} ${tMember ? 'is a' : 'is not a'} ${roleName} of group ${scope.targetGroupId}`);
366+
res = res && tMember;
367+
}
368+
}
369+
370+
// Note the fact that we are allowing access due to passing an ACL.
371+
if (res) {
372+
this.app.loopback.getCurrentContext().set('groupAccessApplied', true);
373+
}
374+
375+
return cb(null, res);
309376
})
310377
.catch(cb);
311378

312-
// return this.getTargetGroupId(context)
313-
// .then(targetGroupId => {
314-
// const actions = [ ];
315-
//
316-
// actions.push(this.isGroupMemberWithRole(modelClass, modelId, userId, roleName));
317-
//
318-
// // If this is an attempt to save the item into a new group, check the user has access to the target group.
319-
// if (targetGroupId && targetGroupId !== modelId) {
320-
// scope.targetGroupId = targetGroupId;
321-
// actions.push(this.isGroupMemberWithRole(modelClass, targetGroupId, userId, roleName));
322-
// }
323-
//
324-
// return actions;
325-
// })
326-
// .spread((currentGroupCount, targetGroupCount) => {
327-
// let res = false;
328-
//
329-
// // Determine grant based on the current/target group context.
330-
// res = currentGroupCount > 0;
331-
//
332-
// debug(`user ${userId} ${res ? 'is a' : 'is not a'} ${roleName} of group ${modelId}`);
333-
//
334-
// // If it's an attempt to save into a new group, also ensure the user has access to the target group.
335-
// if (scope.targetGroupId && scope.targetGroupId !== modelId) {
336-
// const tMember = targetGroupCount > 0;
337-
//
338-
// debug(`user ${userId} ${tMember ? 'is a' : 'is not a'} ${roleName} of group ${scope.targetGroupId}`);
339-
// res = res && tMember;
340-
// }
341-
//
342-
// // Note the fact that we are allowing access due to passing an ACL.
343-
// if (res) {
344-
// this.app.loopback.getCurrentContext().set('groupAccessApplied', true);
345-
// }
346-
//
347-
// debug(`${accessGroup} role resolver returns ${res} for user ${userId}`);
348-
// return cb(null, res);
349-
// })
350-
// .catch(cb);
351-
});
352-
}
379+
});
380+
};
353381

354382
/**
355383
* Check if a given user ID has a given role in the model instances group.

test/fixtures/simple-app/server/component-config.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"$group:admin",
1818
"$group:manager",
1919
"$group:member"
20-
]
20+
],
21+
"applyToStatic": true
2122
}
2223
}

0 commit comments

Comments
 (0)