Skip to content

Commit

Permalink
Enhance ACL validation (hyperledger-archives#3914)
Browse files Browse the repository at this point in the history
Add file and location information to ACL validation errors and
improve messages

Closes #382

Signed-off-by: James Taylor <[email protected]>
  • Loading branch information
jt-nti authored Apr 25, 2018
1 parent f6ae19e commit 89c0e7d
Show file tree
Hide file tree
Showing 12 changed files with 422 additions and 53 deletions.
6 changes: 2 additions & 4 deletions packages/composer-common/lib/acl/aclfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class AclFile {
* @param {ModelManager} modelManager - the ModelManager that manages this
* ModelFile and that will be used to validate the rules in the AclFile
* @param {string} definitions - The ACL rules as a string.
* @throws {IllegalModelException}
* @throws {IllegalAclException}
*/
constructor(id, modelManager, definitions) {
this.modelManager = modelManager;
Expand Down Expand Up @@ -62,8 +62,6 @@ class AclFile {
// TODO (DCS) check that the id of the AclRule does not already exist
this.rules.push(aclRule);
}

// console.log(JSON.stringify(this.ast));
}

/**
Expand Down Expand Up @@ -97,7 +95,7 @@ class AclFile {
/**
* Validates the ModelFile.
*
* @throws {IllegalModelException} if the model is invalid
* @throws {IllegalAclException} if the model is invalid
* @private
*/
validate() {
Expand Down
28 changes: 11 additions & 17 deletions packages/composer-common/lib/acl/aclrule.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@

'use strict';

const IllegalModelException = require('../introspect/illegalmodelexception');
const IllegalAclException = require('./illegalaclexception');
const ModelBinding = require('./modelbinding');
const ParticipantDeclaration = require('../introspect/participantdeclaration');
const Operation = require('./operation');
const Predicate = require('./predicate');
const TransactionDeclaration = require('../introspect/transactiondeclaration');

Expand All @@ -37,11 +38,11 @@ class AclRule {
*
* @param {AclFile} aclFile - the AclFile for this rule
* @param {string} ast - the AST created by the parser
* @throws {IllegalModelException}
* @throws {IllegalAclException}
*/
constructor(aclFile, ast) {
if(!aclFile || !ast) {
throw new IllegalModelException('Invalid AclFile or AST');
throw new IllegalAclException('Invalid AclFile or AST');
}

this.ast = ast;
Expand Down Expand Up @@ -72,13 +73,13 @@ class AclRule {
/**
* Process the AST and build the model
*
* @throws {IllegalModelException}
* @throws {IllegalAclException}
* @private
*/
process() {
this.name = this.ast.id.name;
this.noun = new ModelBinding(this, this.ast.noun, this.ast.nounVariable);
this.verbs = this.ast.verbs;
this.operation = new Operation(this, this.ast.operation);

this.participant = null;
if(this.ast.participant && this.ast.participant !== 'ANY') {
Expand All @@ -105,26 +106,19 @@ class AclRule {
/**
* Semantic validation of the structure of this AclRule.
*
* @throws {IllegalModelException}
* @throws {IllegalAclException}
* @private
*/
validate() {
this.noun.validate();

const foundVerbs = {};
this.verbs.forEach((verb) => {
if (foundVerbs[verb]) {
throw new IllegalModelException(`The verb '${verb}' has been specified more than once in the ACL rule '${this.name}'`);
}
foundVerbs[verb] = true;
});
this.operation.validate();

if(this.participant) {
this.participant.validate();

let participantClassDeclaration = this.participant.getClassDeclaration();
if (participantClassDeclaration && !(participantClassDeclaration instanceof ParticipantDeclaration)) {
throw new IllegalModelException(`The participant '${participantClassDeclaration.getName()}' must be a participant`);
throw new IllegalAclException(`Expected '${participantClassDeclaration.getName()}' to be a participant`, this.aclFile, this.participant.ast.location);
}
}

Expand All @@ -133,7 +127,7 @@ class AclRule {

let transactionClassDeclaration = this.transaction.getClassDeclaration();
if (transactionClassDeclaration && !(transactionClassDeclaration instanceof TransactionDeclaration)) {
throw new IllegalModelException(`The transaction '${transactionClassDeclaration.getName()}' must be a transaction`);
throw new IllegalAclException(`Expected '${transactionClassDeclaration.getName()}' to be a transaction`, this.aclFile, this.transaction.ast.location);
}
}

Expand Down Expand Up @@ -166,7 +160,7 @@ class AclRule {
* @return {string} the verb
*/
getVerbs() {
return this.verbs;
return this.operation.verbs;
}

/**
Expand Down
70 changes: 70 additions & 0 deletions packages/composer-common/lib/acl/illegalaclexception.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const BaseFileException = require('../basefileexception');

/**
* Exception throws when a composer acl file is semantically invalid
* @extends BaseFileException
* @see See {@link BaseFileException}
* @class
* @memberof module:composer-common
* @private
*/
class IllegalAclException extends BaseFileException {

/**
* Create an IllegalAclException.
* @param {String} message - the message for the exception
* @param {AclFile} [aclFile] - the optional aclFile associated with the exception
* @param {Object} [fileLocation] - location details of the error within the model file.
* @param {String} fileLocation.start.line - start line of the error location.
* @param {String} fileLocation.start.column - start column of the error location.
* @param {String} fileLocation.end.line - end line of the error location.
* @param {String} fileLocation.end.column - end column of the error location.
*/
constructor(message, aclFile, fileLocation) {

let messageSuffix = '';
if(aclFile && aclFile.getIdentifier()) {
messageSuffix = 'File \'' + aclFile.getIdentifier() + '\': ' ;
}

if(fileLocation) {
messageSuffix = messageSuffix + 'line ' + fileLocation.start.line + ' column ' +
fileLocation.start.column + ', to line ' + fileLocation.end.line + ' column ' +
fileLocation.end.column + '. ';
}

// First character to be uppercase, and prepended with a space
if (messageSuffix) {
messageSuffix = ' ' + messageSuffix.charAt(0).toUpperCase() + messageSuffix.slice(1);
}

super(message, fileLocation, message + messageSuffix);
this.aclFile = aclFile;
}

/**
* Returns the aclFile associated with the exception or null
* @return {AclFile} the optional acl file associated with the exception
*/
getAclFile() {
return this.aclFile;
}
}

module.exports = IllegalAclException;
23 changes: 13 additions & 10 deletions packages/composer-common/lib/acl/modelbinding.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

'use strict';

const IllegalModelException = require('../introspect/illegalmodelexception');
const IllegalAclException = require('./illegalaclexception');
const ModelUtil = require('../modelutil');

/**
Expand All @@ -35,11 +35,11 @@ class ModelBinding {
* @param {AclRule} aclRule - the AclRule for this ModelBinding
* @param {Object} ast - the AST created by the parser
* @param {Object} variableAst - the variable binding AST created by the parser
* @throws {IllegalModelException}
* @throws {IllegalAclException}
*/
constructor(aclRule, ast, variableAst) {
if(!aclRule || !ast) {
throw new IllegalModelException('Invalid AclRule or AST');
throw new IllegalAclException('Invalid AclRule or AST');
}

this.ast = ast;
Expand Down Expand Up @@ -78,7 +78,7 @@ class ModelBinding {
/**
* Process the AST and build the model
*
* @throws {IllegalModelException}
* @throws {IllegalAclException}
* @private
*/
process() {
Expand All @@ -97,7 +97,7 @@ class ModelBinding {
}

/**
* Returns strind representation of this object
* Returns string representation of this object
*
* @return {string} the string version of the object
*/
Expand Down Expand Up @@ -153,7 +153,7 @@ class ModelBinding {
/**
* Semantic validation of the structure of this ModelBinding.
*
* @throws {IllegalModelException}
* @throws {IllegalAclException}
* @private
*/
validate() {
Expand All @@ -171,26 +171,29 @@ class ModelBinding {
if (namespaces.findIndex(function (element, index, array) {
return (ns === element || element.startsWith(ns + '.'));
})=== -1) {
throw new IllegalModelException('Failed to find namespace ' + this.qualifiedName);
throw new IllegalAclException(`Expected namespace \"${this.qualifiedName}\" to be defined`, this.aclRule.getAclFile(), this.ast.location);
}
} else if (ModelUtil.isWildcardName(this.qualifiedName)) {
const modelFile = mm.getModelFile(ns);

if(!modelFile) {
throw new IllegalModelException('Failed to find namespace ' + this.qualifiedName);
throw new IllegalAclException(`Expected namespace \"${this.qualifiedName}\" to be defined`, this.aclRule.getAclFile(), this.ast.location);
}
} else {
if (!ns) {
throw new IllegalAclException(`Expected class \"${this.qualifiedName}\" to include namespace`, this.aclRule.getAclFile(), this.ast.location);
}
const modelFile = mm.getModelFile(ns);

if(!modelFile) {
throw new IllegalModelException('Failed to find namespace ' + ns);
throw new IllegalAclException(`Expected class \"${this.qualifiedName}\" to be defined but namespace \"${ns}\" not found`, this.aclRule.getAclFile(), this.ast.location);
}

const className = ModelUtil.getShortName(this.qualifiedName);
const classDeclaration = modelFile.getLocalType(className);

if(!classDeclaration) {
throw new IllegalModelException('Failed to find class ' + this.qualifiedName);
throw new IllegalAclException(`Expected class \"${this.qualifiedName}\" to be defined`, this.aclRule.getAclFile(), this.ast.location);
}

this.classDeclaration = classDeclaration;
Expand Down
104 changes: 104 additions & 0 deletions packages/composer-common/lib/acl/operation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const IllegalAclException = require('./illegalaclexception');

/**
* Operation captures the array ofaction verbs that the ACL rule
* governs.
*
* @private
* @class
* @memberof module:composer-common
*/
class Operation {

/**
* Create an Operation from an Abstract Syntax Tree. The AST is the
* result of parsing.
*
* @param {AclRule} aclRule - the AclRule for this Operation
* @param {Object} ast - the AST created by the parser
* @throws {IllegalAclException}
*/
constructor(aclRule, ast) {
if(!aclRule || !ast) {
throw new IllegalAclException('Invalid AclRule or AST');
}

this.ast = ast;
this.aclRule = aclRule;
this.process();
}

/**
* Visitor design pattern
* @param {Object} visitor - the visitor
* @param {Object} parameters - the parameter
* @return {Object} the result of visiting or null
* @private
*/
accept(visitor,parameters) {
return visitor.visit(this, parameters);
}

/**
* Returns the AclRule that owns this Operation.
*
* @return {AclRule} the owning AclRule
*/
getAclRule() {
return this.aclRule;
}

/**
* Returns the expression as a text string.
*
* @return {string} the verbs for the operation
*/
getVerbs() {
return this.verbs;
}

/**
* Process the AST and build the model
*
* @throws {IllegalAclException}
* @private
*/
process() {
this.verbs = this.ast.verbs;
}

/**
* Semantic validation of the structure of this Operation.
*
* @throws {IllegalAclException}
* @private
*/
validate() {
const foundVerbs = {};
this.verbs.forEach((verb) => {
if (foundVerbs[verb]) {
throw new IllegalAclException(`The verb '${verb}' has been specified more than once in the ACL rule '${this.aclRule.getName()}'`, this.aclRule.getAclFile(), this.ast.location);
}
foundVerbs[verb] = true;
});
}

}

module.exports = Operation;
Loading

0 comments on commit 89c0e7d

Please sign in to comment.