Repository: ranger Updated Branches: refs/heads/ranger-0.7 d81a7446a -> 216704aea
RANGER-2272: Ensure that case of resource-definition names and access-type names in Ranger policy is the same as in service-definition after successful validation Project: http://git-wip-us.apache.org/repos/asf/ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/ranger/commit/216704ae Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/216704ae Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/216704ae Branch: refs/heads/ranger-0.7 Commit: 216704aeadea8b0efdd2caddfeac083465b9c9da Parents: d81a744 Author: Abhay Kulkarni <[email protected]> Authored: Tue Oct 30 15:36:23 2018 -0700 Committer: Abhay Kulkarni <[email protected]> Committed: Tue Oct 30 16:38:12 2018 -0700 ---------------------------------------------------------------------- .../plugin/errors/ValidationErrorCode.java | 2 + .../model/validation/RangerPolicyValidator.java | 37 ++++++++++---- .../validation/RangerServiceDefValidator.java | 2 +- .../model/validation/RangerValidator.java | 54 ++++++++++++++++---- .../validation/TestRangerPolicyValidator.java | 4 +- .../model/validation/TestRangerValidator.java | 37 +------------- .../org/apache/ranger/biz/ServiceDBStore.java | 18 ++++++- 7 files changed, 94 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ranger/blob/216704ae/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 c2fe4ac..25ed6b7 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,8 @@ public enum ValidationErrorCode { SERVICE_DEF_VALIDATION_ERR_ENUM_DEF_NO_VALUES(2018, "enum [{0}] does not have any elements"), 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 resource name. Resource name should consist of only lowercase, hyphen or underscore 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/216704ae/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java index e48e5e1..6c93413 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java @@ -449,7 +449,8 @@ public class RangerPolicyValidator extends RangerValidator { } boolean valid = true; - Set<String> policyResources = getPolicyResources(policy); + convertPolicyResourceNamesToLower(policy); + Set<String> policyResources = policy.getResources().keySet(); RangerServiceDefHelper defHelper = new RangerServiceDefHelper(serviceDef); Set<List<RangerResourceDef>> hierarchies = defHelper.getResourceHierarchies(policy.getPolicyType()); // this can be empty but not null! @@ -874,15 +875,20 @@ public class RangerPolicyValidator extends RangerValidator { .errorCode(error.getErrorCode()) .build()); valid = false; - } else if (!accessTypes.contains(accessType.toLowerCase())) { - ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID; - failures.add(new ValidationFailureDetailsBuilder() - .field("policy item access type") - .isSemanticallyIncorrect() - .becauseOf(error.getMessage(accessType, accessTypes)) - .errorCode(error.getErrorCode()) - .build()); - valid = false; + } else { + String matchedAccessType = getMatchedAccessType(accessType, accessTypes); + if (StringUtils.isEmpty(matchedAccessType)) { + ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID; + failures.add(new ValidationFailureDetailsBuilder() + .field("policy item access type") + .isSemanticallyIncorrect() + .becauseOf(error.getMessage(accessType, accessTypes)) + .errorCode(error.getErrorCode()) + .build()); + valid = false; + } else { + access.setType(matchedAccessType); + } } Boolean isAllowed = access.getIsAllowed(); // it can be null (which is treated as allowed) but not false @@ -903,4 +909,15 @@ public class RangerPolicyValidator extends RangerValidator { } return valid; } + + String getMatchedAccessType(String accessType, Set<String> validAccessTypes) { + String ret = null; + for (String validType : validAccessTypes) { + if (StringUtils.equalsIgnoreCase(accessType, validType)) { + ret = validType; + break; + } + } + return ret; + } } http://git-wip-us.apache.org/repos/asf/ranger/blob/216704ae/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 709c396..54d526d 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 @@ -485,7 +485,7 @@ public class RangerServiceDefValidator extends RangerValidator { return valid; } - boolean isValidResources(RangerServiceDef serviceDef, List<ValidationFailureDetails> failures, final Action action) { + public boolean isValidResources(RangerServiceDef serviceDef, List<ValidationFailureDetails> failures, final Action action) { if(LOG.isDebugEnabled()) { LOG.debug(String.format("==> RangerServiceDefValidator.isValidResources(%s, %s)", serviceDef, failures)); } http://git-wip-us.apache.org/repos/asf/ranger/blob/216704ae/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 3ae02bf..1c9071b 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; @@ -303,7 +304,7 @@ public abstract class RangerValidator { if (StringUtils.isBlank(accessType)) { LOG.warn("Access type def name was null/empty/blank!"); } else { - accessTypes.add(accessType.toLowerCase()); + accessTypes.add(accessType); } } } @@ -409,22 +410,22 @@ public abstract class RangerValidator { } return resourceNames; } - + /** - * Returns the resource-types defined on the policy converted to lowe-case + * Converts, in place, the resources defined in the policy to have lower-case resource-def-names * @param policy * @return */ - Set<String> getPolicyResources(RangerPolicy policy) { - if (policy == null || policy.getResources() == null || policy.getResources().isEmpty()) { - return new HashSet<String>(); - } else { - Set<String> result = new HashSet<String>(); - for (String name : policy.getResources().keySet()) { - result.add(name.toLowerCase()); + + void convertPolicyResourceNamesToLower(RangerPolicy policy) { + Map<String, RangerPolicyResource> lowerCasePolicyResources = new HashMap<>(); + if (policy.getResources() != null) { + for (Map.Entry<String, RangerPolicyResource> entry : policy.getResources().entrySet()) { + String lowerCasekey = entry.getKey().toLowerCase(); + lowerCasePolicyResources.put(lowerCasekey, entry.getValue()); } - return result; } + policy.setResources(lowerCasePolicyResources); } Map<String, String> getValidationRegExes(RangerServiceDef serviceDef) { @@ -582,6 +583,37 @@ public abstract class RangerValidator { return valid; } + /* + * Important: Resource-names are required to be lowercase. This is used in validating policy create/update operations. + * Ref: RANGER-2272 + */ + boolean isValidResourceName(final String value, final String valueContext, final List<ValidationFailureDetails> failures) { + boolean ret = true; + + if (value != null && !StringUtils.isEmpty(value)) { + int sz = value.length(); + + for(int i = 0; i < sz; ++i) { + char c = value.charAt(i); + if (!(Character.isLowerCase(c) || c == '-' || c == '_')) { // Allow only lowercase, hyphen or underscore characters + ret = false; + break; + } + } + } else { + ret = false; + } + if (!ret) { + 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 ret; + } + 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/216704ae/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java index 97a3ea7..e55c6eb 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java @@ -123,13 +123,13 @@ public class TestRangerPolicyValidator { private final Object[][] policyResourceMap_good = new Object[][] { // resource-name, values, excludes, recursive { "db", new String[] { "db1", "db2" }, null, null }, - { "TBL", new String[] { "tbl1", "tbl2" }, true, false } // case should not matter + { "tbl", new String[] { "tbl1", "tbl2" }, true, false } // case matters - use only lowercase characters }; private final Object[][] policyResourceMap_goodMultipleHierarchies = new Object[][] { // resource-name, values, excludes, recursive { "db", new String[] { "db1", "db2" }, null, null }, - { "UDF", new String[] { "udf1", "udf2" }, true, false } // case should not matter + { "udf", new String[] { "udf1", "udf2" }, true, false } // case matters - use only lowercase characters }; private final Object[][] policyResourceMap_bad = new Object[][] { http://git-wip-us.apache.org/repos/asf/ranger/blob/216704ae/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java index fb8073f..a6e3151 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java @@ -33,7 +33,6 @@ import java.util.Map; import java.util.Set; import org.apache.ranger.plugin.model.RangerPolicy; -import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource; import org.apache.ranger.plugin.model.RangerService; import org.apache.ranger.plugin.model.RangerServiceDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef; @@ -46,8 +45,6 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import com.google.common.collect.Maps; - public class TestRangerValidator { static class RangerValidatorForTest extends RangerValidator { @@ -270,8 +267,8 @@ public class TestRangerValidator { Assert.assertEquals(4, accessTypes.size()); Assert.assertTrue(accessTypes.contains("a")); Assert.assertTrue(accessTypes.contains("b ")); - Assert.assertTrue(accessTypes.contains(" c")); - Assert.assertTrue(accessTypes.contains(" d ")); + Assert.assertTrue(accessTypes.contains(" C")); + Assert.assertTrue(accessTypes.contains(" D ")); } @Test @@ -359,36 +356,6 @@ public class TestRangerValidator { } @Test - public void test_getPolicyResources() { - - Set<String> result; - RangerPolicy policy = null; - // null policy - result = _validator.getPolicyResources(null); - Assert.assertTrue(result != null); - Assert.assertTrue(result.isEmpty()); - // null resource map - policy = mock(RangerPolicy.class); - when(policy.getResources()).thenReturn(null); - result = _validator.getPolicyResources(null); - Assert.assertTrue(result != null); - Assert.assertTrue(result.isEmpty()); - // empty resource map - Map<String, RangerPolicyResource> input = Maps.newHashMap(); - when(policy.getResources()).thenReturn(input); - result = _validator.getPolicyResources(policy); - Assert.assertTrue(result != null); - Assert.assertTrue(result.isEmpty()); - // known resource map - input.put("r1", mock(RangerPolicyResource.class)); - input.put("R2", mock(RangerPolicyResource.class)); - result = _validator.getPolicyResources(policy); - Assert.assertEquals(2, result.size()); - Assert.assertTrue("r1", result.contains("r1")); - Assert.assertTrue("R2", result.contains("r2")); // result should lowercase the resource-names - } - - @Test public void test_getIsAuditEnabled() { // null policy RangerPolicy policy = null; http://git-wip-us.apache.org/repos/asf/ranger/blob/216704ae/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java index 773c2ed..52d0130 100644 --- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java @@ -64,6 +64,10 @@ import org.apache.ranger.common.AppConstants; import org.apache.ranger.common.ContextUtil; import org.apache.ranger.common.MessageEnums; import org.apache.ranger.common.db.RangerTransactionSynchronizationAdapter; +import org.apache.ranger.entity.*; +import org.apache.ranger.plugin.model.validation.RangerServiceDefValidator; +import org.apache.ranger.plugin.model.validation.RangerValidator; +import org.apache.ranger.plugin.model.validation.ValidationFailureDetails; import org.apache.ranger.plugin.policyengine.RangerPolicyEngine; import org.apache.ranger.plugin.policyresourcematcher.RangerDefaultPolicyResourceMatcher; import org.apache.ranger.plugin.policyresourcematcher.RangerPolicyResourceMatcher; @@ -353,9 +357,21 @@ public class ServiceDBStore extends AbstractServiceStore { + serviceDef.getName() + " already exists", MessageEnums.ERROR_DUPLICATE_OBJECT); } - + List<RangerServiceConfigDef> configs = serviceDef.getConfigs(); List<RangerResourceDef> resources = serviceDef.getResources(); + + if (CollectionUtils.isNotEmpty(resources)) { + RangerServiceDefValidator validator = new RangerServiceDefValidator(this); + List<ValidationFailureDetails> failures = new ArrayList<>(); + boolean isValidResources = validator.isValidResources(serviceDef, failures, RangerValidator.Action.CREATE); + if (!isValidResources) { + throw restErrorUtil.createRESTException("service-def with name: " + + serviceDef.getName() + " has invalid resources:[" + failures.toString() + "]", + MessageEnums.INVALID_INPUT_DATA); + } + } + List<RangerAccessTypeDef> accessTypes = serviceDef.getAccessTypes(); List<RangerPolicyConditionDef> policyConditions = serviceDef.getPolicyConditions(); List<RangerContextEnricherDef> contextEnrichers = serviceDef.getContextEnrichers();
