Repository: ranger Updated Branches: refs/heads/master 5dced6f66 -> b041df1dd
RANGER-2108: Ensure that resource and access-type names in service definition are in lower case Project: http://git-wip-us.apache.org/repos/asf/ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/ranger/commit/b041df1d Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/b041df1d Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/b041df1d Branch: refs/heads/master Commit: b041df1ddade3d3a78fe86513258d617a42e7f10 Parents: 5dced6f Author: Abhay Kulkarni <[email protected]> Authored: Wed May 30 16:07:18 2018 -0700 Committer: Abhay Kulkarni <[email protected]> Committed: Wed May 30 16:07:18 2018 -0700 ---------------------------------------------------------------------- .../plugin/errors/ValidationErrorCode.java | 1 + .../validation/RangerServiceDefValidator.java | 3 +++ .../model/validation/RangerValidator.java | 13 ++++++++++ .../TestRangerServiceDefValidator.java | 26 ++++++++++++++++++++ 4 files changed, 43 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ranger/blob/b041df1d/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java index ab120b7..fbe3030 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java @@ -61,6 +61,7 @@ public enum ValidationErrorCode { SERVICE_DEF_VALIDATION_ERR_ENUM_DEF_INVALID_DEFAULT_INDEX(2019, "default index[{0}] for enum [{1}] is invalid"), SERVICE_DEF_VALIDATION_ERR_ENUM_DEF_NULL_ENUM_ELEMENT(2020, "An enum element in enum element collection of enum [{0}] is null"), SERVICE_DEF_VALIDATION_ERR_INVALID_SERVICE_RESOURCE_LEVELS(2021, "Resource-def levels are not in increasing order in an hierarchy"), + SERVICE_DEF_VALIDATION_ERR_NOT_LOWERCASE_NAME(2022, "{0}:[{1}] Invalid name. Name should consist of only lower case characters"), // POLICY VALIDATION POLICY_VALIDATION_ERR_UNSUPPORTED_ACTION(3001, "Internal error: method signature isValid(Long) is only supported for DELETE"), http://git-wip-us.apache.org/repos/asf/ranger/blob/b041df1d/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java index d5f3fe5..3f9315a 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java @@ -259,6 +259,7 @@ public class RangerServiceDefValidator extends RangerValidator { Set<Long> ids = new HashSet<>(); for (RangerAccessTypeDef def : accessTypeDefs) { String name = def.getName(); + valid = isInLowerCase(name, "access type name", failures) && valid; valid = isUnique(name, accessNames, "access type name", "access types", failures) && valid; valid = isUnique(def.getItemId(), ids, "access type itemId", "access types", failures) && valid; if (CollectionUtils.isNotEmpty(def.getImpliedGrants())) { @@ -472,6 +473,8 @@ public class RangerServiceDefValidator extends RangerValidator { Set<String> names = new HashSet<String>(resources.size()); Set<Long> ids = new HashSet<Long>(resources.size()); for (RangerResourceDef resource : resources) { + valid = isInLowerCase(resource.getName(), "resource type name", failures) && valid; + /* * While id is the natural key, name is a surrogate key. At several places code expects resource name to be unique within a service. */ http://git-wip-us.apache.org/repos/asf/ranger/blob/b041df1d/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java index 55973f5..bdac640 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java @@ -33,6 +33,7 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.ranger.plugin.errors.ValidationErrorCode; import org.apache.ranger.plugin.model.RangerPolicy; import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource; import org.apache.ranger.plugin.model.RangerService; @@ -588,6 +589,18 @@ public abstract class RangerValidator { return valid; } + boolean isInLowerCase(final String value, final String valueContext, final List<ValidationFailureDetails> failures) { + if (!StringUtils.isAllLowerCase(value)) { + ValidationErrorCode errorCode = ValidationErrorCode.SERVICE_DEF_VALIDATION_ERR_NOT_LOWERCASE_NAME; + failures.add(new ValidationFailureDetailsBuilder() + .errorCode(errorCode.getErrorCode()) + .field(value) + .becauseOf(errorCode.getMessage(valueContext, value)) + .build()); + return false; + } + return true; + } boolean isUnique(final String value, final Set<String> alreadySeen, final String valueName, final String collectionName, final List<ValidationFailureDetails> failures) { return isUnique(value, null, alreadySeen, valueName, collectionName, failures); } http://git-wip-us.apache.org/repos/asf/ranger/blob/b041df1d/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java index 72c4520..1fafb12 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefValidator.java @@ -209,6 +209,12 @@ public class TestRangerServiceDefValidator { { 3L, "admin", new String[] { "write", "admin" } } // non-existent access type (execute) }; + final Object[][] accessTypes_mixed_case_names = new Object[][] { + { 1L, "Read", null }, + { 2L, "WRITE", new String[] { } }, + { 3L, "adminPrivilege", new String[] { "write", "admin" } } + }; + @Test public final void test_isValidAccessTypes_happyPath() { List<RangerAccessTypeDef> input = _utils.createAccessTypeDefs(accessTypes_good); @@ -252,6 +258,13 @@ public class TestRangerServiceDefValidator { accessTypeDefs = _utils.createAccessTypeDefs(accessTypes_bad_selfReference); _failures.clear(); assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures)); _utils.checkFailureForSemanticError(_failures, "implied grants", "admin"); + + // Mixed case access types + accessTypeDefs = _utils.createAccessTypeDefs(accessTypes_mixed_case_names); + _failures.clear(); assertFalse(_validator.isValidAccessTypes(accessTypeDefs, _failures)); + _utils.checkFailure(_failures, null, null, null, "Read",null); + _utils.checkFailure(_failures, null, null, null, "WRITE",null); + _utils.checkFailure(_failures, null, null, null, "adminPrivilege",null); } final Object[][] enums_bad_enumName_null = new Object[][] { @@ -392,6 +405,13 @@ public class TestRangerServiceDefValidator { { 3L, 30, " " } // Name is all whitespace }; + Object[][] mixedCaseResources = new Object[][] { + // { id, level, name } + { 4L, -10, "DBase" }, // -ve value for level is ok + { 5L, 10, "TABLE" }, // id is duplicate + { 6L, -10, "Column" } // (in different case) but name and level are duplicate + }; + @Test public final void test_isValidResources() { // null/empty resources are an error @@ -410,6 +430,12 @@ public class TestRangerServiceDefValidator { _utils.checkFailureForMissingValue(_failures, "resource itemId"); _utils.checkFailureForSemanticError(_failures, "resource itemId", "1"); // id 1 is duplicate _utils.checkFailureForSemanticError(_failures, "resource name", "DataBase"); + + resources.clear(); resources.addAll(_utils.createResourceDefsWithIds(mixedCaseResources)); + _failures.clear(); assertFalse(_validator.isValidResources(_serviceDef, _failures)); + _utils.checkFailure(_failures, null, null, null, "DBase",null); + _utils.checkFailure(_failures, null, null, null, "TABLE",null); + _utils.checkFailure(_failures, null, null, null, "Column",null); } @Test
