!fixup only set ctx.accessType when sharedMethod is available

This commit is contained in:
Ritchie Martori 2014-06-02 13:41:14 -07:00
parent a2f931ed3f
commit fea1cee1c4
6 changed files with 38 additions and 39 deletions

View File

@ -44,7 +44,10 @@ function AccessContext(context) {
this.methodNames = []; this.methodNames = [];
} }
this.accessType = this.model._getAccessTypeForMethod(this.method); if(this.sharedMethod) {
this.accessType = this.model._getAccessTypeForMethod(this.sharedMethod);
}
this.accessType = context.accessType || AccessContext.ALL; this.accessType = context.accessType || AccessContext.ALL;
this.accessToken = context.accessToken || AccessToken.ANONYMOUS; this.accessToken = context.accessToken || AccessToken.ANONYMOUS;
@ -240,8 +243,9 @@ function AccessRequest(model, property, accessType, permission, methodNames) {
} }
/** /**
* Is the request a wildcard * Does the request contain any wildcards?
* @returns {boolean} *
* @returns {Boolean}
*/ */
AccessRequest.prototype.isWildcard = function () { AccessRequest.prototype.isWildcard = function () {
return this.model === AccessContext.ALL || return this.model === AccessContext.ALL ||
@ -259,12 +263,7 @@ AccessRequest.prototype.exactlyMatches = function(acl) {
var matchesModel = acl.model === this.model; var matchesModel = acl.model === this.model;
var matchesProperty = acl.property === this.property; var matchesProperty = acl.property === this.property;
var matchesMethodName = this.methodNames.indexOf(acl.property) !== -1; var matchesMethodName = this.methodNames.indexOf(acl.property) !== -1;
var matchesAccessType = (acl.accessType || '*') === this.accessType; var matchesAccessType = acl.accessType === this.accessType;
debug('matchesModel %s === %s %j', acl.model, this.model, acl.model === this.model);
debug('matchesProperty %s === %s %j', acl.property, this.property, acl.property === this.property);
debug('matchesMethodName %s in %j %j', acl.property, this.methodNames, this.methodNames.indexOf(acl.property) !== -1);
debug('matchesAccessType %s === %s %j', acl.accessType, this.accessType, acl.accessType === this.accessType);
if(matchesModel && matchesAccessType) { if(matchesModel && matchesAccessType) {
return matchesProperty || matchesMethodName; return matchesProperty || matchesMethodName;

View File

@ -125,12 +125,9 @@ ACL.getMatchingScore = function getMatchingScore(rule, req) {
score = score * 4; score = score * 4;
var val1 = rule[props[i]] || ACL.ALL; var val1 = rule[props[i]] || ACL.ALL;
var val2 = req[props[i]] || ACL.ALL; var val2 = req[props[i]] || ACL.ALL;
var isMatchingMethodName = props[i] === 'property' && req.methodNames.indexOf(val1) !== -1;
if(props[i] === 'property' && req.methodNames.indexOf(val1) !== -1) { if (val1 === val2 || isMatchingMethodName) {
score += 3;
}
if (val1 === val2) {
// Exact match // Exact match
score += 3; score += 3;
} else if (val1 === ACL.ALL) { } else if (val1 === ACL.ALL) {
@ -192,6 +189,16 @@ ACL.getMatchingScore = function getMatchingScore(rule, req) {
return score; return score;
}; };
/**
* Get matching score for the given `AccessRequest`.
* @param {AccessRequest} req The request
* @returns {Number} score
*/
ACL.prototype.score = function(req) {
return this.constructor.getMatchingScore(this, req);
}
/*! /*!
* Resolve permission from the ACLs * Resolve permission from the ACLs
* @param {Object[]) acls The list of ACLs * @param {Object[]) acls The list of ACLs
@ -208,47 +215,35 @@ ACL.resolvePermission = function resolvePermission(acls, req) {
}); });
var permission = ACL.DEFAULT; var permission = ACL.DEFAULT;
var score = 0; var score = 0;
var matchingACL;
for (var i = 0; i < acls.length; i++) { for (var i = 0; i < acls.length; i++) {
score = ACL.getMatchingScore(acls[i], req); score = ACL.getMatchingScore(acls[i], req);
if (score < 0) { if (score < 0) {
// the highest scored ACL did not match
break; break;
} }
if (!req.isWildcard()) { if (!req.isWildcard()) {
// We should stop from the first match for non-wildcard // We should stop from the first match for non-wildcard
debug('Found first match (non-wildcard):'); permission = acls[i].permission;
matchingACL = acls[i];
permission = matchingACL.permission;
break; break;
} else { } else {
if(req.exactlyMatches(acls[i])) { if(req.exactlyMatches(acls[i])) {
debug('Exact ACL match:'); permission = acls[i].permission;
acls[i].debug();
// We should stop at the exact match
matchingACL = acls[i];
permission = matchingACL.permission;
break; break;
} }
// For wildcard match, find the strongest permission // For wildcard match, find the strongest permission
if(AccessContext.permissionOrder[acls[i].permission] if(AccessContext.permissionOrder[acls[i].permission]
> AccessContext.permissionOrder[permission]) { > AccessContext.permissionOrder[permission]) {
matchingACL = acls[i]; permission = acls[i].permission;
permission = matchingACL.permission;
} }
} }
} }
if(debug.enabled) { if(debug.enabled) {
if(matchingACL) {
debug('Matching ACL:');
matchingACL.debug();
} else {
debug('No matching ACL found!')
}
debug('The following ACLs were searched: '); debug('The following ACLs were searched: ');
acls.forEach(function(acl) { acls.forEach(function(acl) {
acl.debug(); acl.debug();
debug('with score:', acl.score(req));
}); });
} }
@ -274,7 +269,7 @@ ACL.getStaticACLs = function getStaticACLs(model, property) {
property: acl.property || ACL.ALL, property: acl.property || ACL.ALL,
principalType: acl.principalType, principalType: acl.principalType,
principalId: acl.principalId, // TODO: Should it be a name? principalId: acl.principalId, // TODO: Should it be a name?
accessType: acl.accessType, accessType: acl.accessType || ACL.ALL,
permission: acl.permission permission: acl.permission
})); }));
}); });
@ -393,7 +388,7 @@ ACL.checkAccessForContext = function (context, callback) {
var propertyQuery = (property === ACL.ALL) ? undefined : {inq: methodNames.concat([ACL.ALL])}; var propertyQuery = (property === ACL.ALL) ? undefined : {inq: methodNames.concat([ACL.ALL])};
var accessTypeQuery = (accessType === ACL.ALL) ? undefined : {inq: [accessType, ACL.ALL]}; var accessTypeQuery = (accessType === ACL.ALL) ? undefined : {inq: [accessType, ACL.ALL]};
var req = new AccessRequest(modelName, property, accessType, methodNames); var req = new AccessRequest(modelName, property, accessType, ACL.DEFAULT, methodNames);
var effectiveACLs = []; var effectiveACLs = [];
var staticACLs = this.getStaticACLs(model.modelName, property); var staticACLs = this.getStaticACLs(model.modelName, property);

View File

@ -155,7 +155,8 @@ Model.checkAccess = function(token, modelId, sharedMethod, callback) {
property: sharedMethod.name, property: sharedMethod.name,
method: sharedMethod.name, method: sharedMethod.name,
sharedMethod: sharedMethod, sharedMethod: sharedMethod,
modelId: modelId modelId: modelId,
accessType: this._getAccessTypeForMethod(sharedMethod)
}, function(err, accessRequest) { }, function(err, accessRequest) {
if(err) return callback(err); if(err) return callback(err);
callback(null, accessRequest.isAllowed()); callback(null, accessRequest.isAllowed());
@ -172,7 +173,7 @@ Model.checkAccess = function(token, modelId, sharedMethod, callback) {
Model._getAccessTypeForMethod = function(method) { Model._getAccessTypeForMethod = function(method) {
if(typeof method === 'string') { if(typeof method === 'string') {
method = {name: method}; method = {name: method};
}40 }
assert( assert(
typeof method === 'object', typeof method === 'object',
'method is a required argument and must be a RemoteMethod object' 'method is a required argument and must be a RemoteMethod object'

View File

@ -319,13 +319,13 @@ Role.registerResolver(Role.EVERYONE, function (role, context, callback) {
* @param {Boolean} isInRole * @param {Boolean} isInRole
*/ */
Role.isInRole = function (role, context, callback) { Role.isInRole = function (role, context, callback) {
debug('isInRole(): %s', role);
context.debug();
if (!(context instanceof AccessContext)) { if (!(context instanceof AccessContext)) {
context = new AccessContext(context); context = new AccessContext(context);
} }
debug('isInRole(): %s', role);
context.debug();
var resolver = Role.resolvers[role]; var resolver = Role.resolvers[role];
if (resolver) { if (resolver) {
debug('Custom resolver found for role %s', role); debug('Custom resolver found for role %s', role);

View File

@ -101,11 +101,15 @@ describe('security ACLs', function () {
property: 'find', property: 'find',
accessType: 'WRITE' accessType: 'WRITE'
}; };
acls = acls.map(function(a) { return new ACL(a)});
var perm = ACL.resolvePermission(acls, req); var perm = ACL.resolvePermission(acls, req);
assert.deepEqual(perm, { model: 'account', assert.deepEqual(perm, { model: 'account',
property: 'find', property: 'find',
accessType: 'WRITE', accessType: 'WRITE',
permission: 'ALLOW' }); permission: 'ALLOW',
methodNames: []});
}); });
it("should allow access to models for the given principal by wildcard", function () { it("should allow access to models for the given principal by wildcard", function () {