Repository: ranger Updated Branches: refs/heads/ranger-1 690e4a30c -> 407346e63
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/407346e6 Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/407346e6 Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/407346e6 Branch: refs/heads/ranger-1 Commit: 407346e63815f0d1c9c8581618d8910d2109fba9 Parents: 690e4a3 Author: Abhay Kulkarni <[email protected]> Authored: Tue Oct 30 15:36:23 2018 -0700 Committer: Abhay Kulkarni <[email protected]> Committed: Tue Oct 30 16:46:58 2018 -0700 ---------------------------------------------------------------------- .../model/validation/RangerPolicyValidator.java | 37 ++++++++++++++------ .../validation/RangerServiceDefValidator.java | 2 +- .../model/validation/RangerValidator.java | 26 ++++++++------ .../validation/TestRangerPolicyValidator.java | 4 +-- .../model/validation/TestRangerValidator.java | 37 ++------------------ .../org/apache/ranger/biz/ServiceDBStore.java | 17 ++++++++- 6 files changed, 63 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ranger/blob/407346e6/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 9de860d..ddedf3e 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 @@ -500,7 +500,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! @@ -935,15 +936,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 @@ -964,4 +970,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/407346e6/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 6a1b3e1..f96fcfc 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 @@ -495,7 +495,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/407346e6/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 c7062dd..51324b0 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 @@ -303,7 +303,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 +409,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<>(); - } else { - Set<String> result = new HashSet<>(); - 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 +582,10 @@ 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; http://git-wip-us.apache.org/repos/asf/ranger/blob/407346e6/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 140a9ed..8cdb9c3 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/407346e6/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 5bdffda..6de590b 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 { @@ -264,8 +261,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 @@ -353,36 +350,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/407346e6/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 55d2d9b..ea2c220 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 @@ -65,6 +65,9 @@ import org.apache.ranger.common.MessageEnums; import org.apache.ranger.common.RangerCommonEnums; 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; @@ -351,9 +354,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();
