This is an automated email from the ASF dual-hosted git repository. abhay pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/master by this push: new ffda77536 RANGER-4762:Prevent duplicate values for a resource while validating a policy ffda77536 is described below commit ffda775366ac8ac8a6869991dbab5e12d307a423 Author: Fateh Singh <fateh...@gmail.com> AuthorDate: Tue Apr 2 11:13:57 2024 -0700 RANGER-4762:Prevent duplicate values for a resource while validating a policy --- .../ranger/plugin/errors/ValidationErrorCode.java | 1 + .../model/validation/RangerPolicyValidator.java | 68 +++++++++++++++------- .../validation/TestRangerPolicyValidator.java | 18 ++++++ 3 files changed, 65 insertions(+), 22 deletions(-) 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 bf119773b..00855458d 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 @@ -107,6 +107,7 @@ public enum ValidationErrorCode { POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_USER(3053, "policy items user was null"), POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_GROUP(3054, "policy items group was null"), POLICY_VALIDATION_ERR_NULL_POLICY_ITEM_ROLE(3055, "policy items role was null"), + POLICY_VALIDATION_ERR_DUPLICATE_VALUES_FOR_RESOURCE(3056, "Values for the resource={0} contained a duplicate value={1}. Ensure all values for a resource are unique"), POLICY_VALIDATION_ERR_INVALID_SERVICE_TYPE(4009," Invalid service type [{0}] provided for service [{1}]"), // SECURITY_ZONE Validations 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 76e9dee8c..cdfc2628c 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 @@ -883,15 +883,7 @@ public class RangerPolicyValidator extends RangerValidator { String name = entry.getKey(); RangerPolicyResource policyResource = entry.getValue(); if(policyResource != null) { - if(CollectionUtils.isNotEmpty(policyResource.getValues())) { - Set<String> resources = new HashSet<>(policyResource.getValues()); - for (String aValue : resources) { - if (StringUtils.isBlank(aValue)) { - policyResource.getValues().remove(aValue); - } - } - } - + policyResource.getValues().removeIf(StringUtils::isBlank); if(CollectionUtils.isEmpty(policyResource.getValues())){ ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_MISSING_RESOURCE_LIST; if(LOG.isDebugEnabled()) { @@ -906,23 +898,40 @@ public class RangerPolicyValidator extends RangerValidator { .build()); valid=false; } - - if (validationRegExMap.containsKey(name) && CollectionUtils.isNotEmpty(policyResource.getValues())) { - String regEx = validationRegExMap.get(name); - for (String aValue : policyResource.getValues()) { - if (!aValue.matches(regEx)) { - if (LOG.isDebugEnabled()) { - LOG.debug(String.format("Resource failed regex check: value[%s], resource-name[%s], regEx[%s], service-def-name[%s]", aValue, name, regEx, serviceDef.getName())); - } - ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_VALUE_REGEX; - failures.add(new ValidationFailureDetailsBuilder() + else{ + String duplicateValue = getDuplicate(policyResource.getValues()); + if (!StringUtils.isBlank(duplicateValue)){ + ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_DUPLICATE_VALUES_FOR_RESOURCE; + if (LOG.isDebugEnabled()){ + LOG.debug(String.format("Duplicate values found for the resource name[%s] value[%s] service-def-name[%s]",name, duplicateValue,serviceDef.getName())); + } + failures.add(new ValidationFailureDetailsBuilder() .field("resource-values") .subField(name) .isSemanticallyIncorrect() - .becauseOf(error.getMessage(aValue, name)) + .becauseOf(error.getMessage(name, duplicateValue)) .errorCode(error.getErrorCode()) - .build()); - valid = false; + .build() + ); + valid = false; + } + if (validationRegExMap.containsKey(name)) { + String regEx = validationRegExMap.get(name); + for (String aValue : policyResource.getValues()) { + if (!aValue.matches(regEx)) { + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("Resource failed regex check: value[%s], resource-name[%s], regEx[%s], service-def-name[%s]", aValue, name, regEx, serviceDef.getName())); + } + ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_INVALID_RESOURCE_VALUE_REGEX; + failures.add(new ValidationFailureDetailsBuilder() + .field("resource-values") + .subField(name) + .isSemanticallyIncorrect() + .becauseOf(error.getMessage(aValue, name)) + .errorCode(error.getErrorCode()) + .build()); + valid = false; + } } } } @@ -935,6 +944,21 @@ public class RangerPolicyValidator extends RangerValidator { return valid; } + private String getDuplicate(List<String> values) { + String duplicate = ""; + if (values!=null) { + HashSet<String> set = new HashSet<>(); + for (String val:values){ + if (set.contains(val)){ + duplicate = val; + break; + } + set.add(val); + } + } + return duplicate; + } + boolean isValidPolicyItems(List<RangerPolicyItem> policyItems, List<ValidationFailureDetails> failures, RangerServiceDef serviceDef) { if(LOG.isDebugEnabled()) { LOG.debug(String.format("==> RangerPolicyValidator.isValid(%s, %s, %s)", policyItems, failures, serviceDef)); 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 be049a820..7e16082bc 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 @@ -140,6 +140,16 @@ public class TestRangerPolicyValidator { {"extra", new String[] { "extra1", "extra2" }, null, null } // spurious "extra" specified }; + private final Object[][] policyResourceMap_bad_duplicate_values = new Object[][]{ + // resource-name, values, excludes, recursive + { "db", new String[] {"db1", "db2" }, null, true}, + { "tbl", new String[] {"tbl1", "tbl1"}, null, null} // invalid: there should not be any duplicates for any resource value + }; + private final Object[][] policyResourceMap_good_duplicate_values = new Object[][]{ + // resource-name, values, excludes, recursive + { "db", new String[] {"db1", "db2" }, null, true}, + { "tbl", new String[] {"tbl1", "tbl2"}, null, null} // valid: there should not be any duplicates for any resource value + }; private final Object[][] policyResourceMap_bad_multiple_hierarchies = new Object[][] { // resource-name, values, excludes, recursive { "db", new String[] { "db1", "db2" }, null, true }, @@ -533,6 +543,14 @@ public class TestRangerPolicyValidator { policyResources = _utils.createPolicyResourceMap(policyResourceMap_good); Assert.assertTrue(_validator.isValidResourceValues(policyResources, _failures, _serviceDef)); + + policyResources = _utils.createPolicyResourceMap(policyResourceMap_bad_duplicate_values); + Assert.assertFalse(_validator.isValidResourceValues(policyResources, _failures, _serviceDef)); + _utils.checkFailureForSemanticError(_failures, "resource-values", "tbl"); + + policyResources = _utils.createPolicyResourceMap(policyResourceMap_good_duplicate_values); + Assert.assertTrue(_validator.isValidResourceValues(policyResources, _failures, _serviceDef)); + } @Test