Repository: incubator-ranger Updated Branches: refs/heads/master 77f8ad98d -> 32f3262bc
RANGER-354 Policy validation during create/update: prevent creation of policies with duplicate resources Signed-off-by: Madhan Neethiraj <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/32f3262b Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/32f3262b Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/32f3262b Branch: refs/heads/master Commit: 32f3262bcf44622b3f0af3d5ac323ed761ea337a Parents: 77f8ad9 Author: Alok Lal <[email protected]> Authored: Wed Apr 1 01:21:38 2015 -0700 Committer: Madhan Neethiraj <[email protected]> Committed: Wed Apr 1 19:26:43 2015 -0700 ---------------------------------------------------------------------- .../RangerPolicyResourceSignature.java | 142 +++++++++++++ .../model/validation/RangerPolicyValidator.java | 130 ++++++++--- .../model/validation/RangerValidator.java | 13 +- .../ranger/plugin/util/RangerObjectFactory.java | 10 + .../TestRangerPolicyResourceSignature.java | 213 +++++++++++++++++++ .../validation/TestRangerPolicyValidator.java | 125 +++++++++-- .../model/validation/TestRangerValidator.java | 18 +- .../model/validation/ValidationTestUtils.java | 17 -- 8 files changed, 598 insertions(+), 70 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyResourceSignature.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyResourceSignature.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyResourceSignature.java new file mode 100644 index 0000000..0952ae8 --- /dev/null +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyResourceSignature.java @@ -0,0 +1,142 @@ +package org.apache.ranger.plugin.model.validation; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.TreeMap; + +import org.apache.commons.codec.digest.DigestUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.ranger.plugin.model.RangerPolicy; +import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource; + +public class RangerPolicyResourceSignature { + + private static final Log LOG = LogFactory.getLog(RangerPolicyResourceSignature.class); + static final RangerPolicyResourceSignature _EmptyResourceSignature = new RangerPolicyResourceSignature((RangerPolicy)null); + + private final String _string; + private final String _hash; + private final RangerPolicy _policy; + + public RangerPolicyResourceSignature(RangerPolicy policy) { + _policy = policy; + String asString = getResourceString(_policy); + if (asString == null) { + _string = ""; + } else { + _string = asString; + } + _hash = DigestUtils.md5Hex(_string); + } + + /** + * Only added for testability. Do not make public + * @param string + */ + RangerPolicyResourceSignature(String string) { + _policy = null; + if (string == null) { + _string = ""; + } else { + _string = string; + } + _hash = DigestUtils.md5Hex(_string); + } + + public String asString() { + return _string; + } + + public String asHashHex() { + return _hash; + } + + @Override + public int hashCode() { + // we assume no collision + return Objects.hashCode(_hash); + } + + @Override + public boolean equals(Object object) { + if (object == null || !(object instanceof RangerPolicyResourceSignature)) { + return false; + } + RangerPolicyResourceSignature that = (RangerPolicyResourceSignature)object; + return Objects.equals(this._hash, that._hash); + } + + @Override + public String toString() { + return String.format("%s: %s", _hash, _string); + } + + String getResourceString(RangerPolicy policy) { + // invalid/empty policy gets a deterministic signature as if it had an + // empty resource string + if (!isPolicyValidForResourceSignatureComputation(policy)) { + return null; + } + Map<String, RangerPolicyResourceView> resources = new TreeMap<String, RangerPolicyResourceView>(); + for (Map.Entry<String, RangerPolicyResource> entry : policy.getResources().entrySet()) { + String resourceName = entry.getKey(); + RangerPolicyResourceView resourceView = new RangerPolicyResourceView(entry.getValue()); + resources.put(resourceName, resourceView); + } + String result = resources.toString(); + return result; + } + + boolean isPolicyValidForResourceSignatureComputation(RangerPolicy policy) { + boolean valid = false; + if (policy == null) { + LOG.debug("isPolicyValidForResourceSignatureComputation: policy was null!"); + } else if (policy.getResources() == null) { + LOG.debug("isPolicyValidForResourceSignatureComputation: resources collection on policy was null!"); + } else if (policy.getResources().containsKey(null)) { + LOG.debug("isPolicyValidForResourceSignatureComputation: resources collection has resource with null name!"); + } else { + valid = true; + } + return valid; + } + + static class RangerPolicyResourceView { + final RangerPolicyResource _policyResource; + + RangerPolicyResourceView(RangerPolicyResource policyResource) { + _policyResource = policyResource; + } + + @Override + public String toString() { + StringBuilder builder = new StringBuilder(); + builder.append("{"); + if (_policyResource != null) { + builder.append("values="); + if (_policyResource.getValues() != null) { + List<String> values = _policyResource.getValues(); + Collections.sort(values); + builder.append(values); + } + builder.append(",excludes="); + if (_policyResource.getIsExcludes() == null) { // null is same as false + builder.append(Boolean.FALSE); + } else { + builder.append(_policyResource.getIsExcludes()); + } + builder.append(",recursive="); + if (_policyResource.getIsRecursive() == null) { // null is the same as false + builder.append(Boolean.FALSE); + } else { + builder.append(_policyResource.getIsRecursive()); + } + } + builder.append("}"); + return builder.toString(); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/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 f5d6bff..b7500bd 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 @@ -1,6 +1,7 @@ package org.apache.ranger.plugin.model.validation; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -133,7 +134,7 @@ public class RangerPolicyValidator extends RangerValidator { .build()); valid = false; } else { - List<RangerPolicy> policies = getPolicies(policyName, serviceName); + List<RangerPolicy> policies = getPolicies(serviceName, policyName); if (CollectionUtils.isNotEmpty(policies)) { if (policies.size() > 1) { failures.add(new ValidationFailureDetailsBuilder() @@ -171,8 +172,8 @@ public class RangerPolicyValidator extends RangerValidator { if (service == null) { failures.add(new ValidationFailureDetailsBuilder() .field("service") - .isMissing() - .becauseOf("service name was null/empty/blank") + .isSemanticallyIncorrect() + .becauseOf("service does not exist") .build()); valid = false; } @@ -202,38 +203,107 @@ public class RangerPolicyValidator extends RangerValidator { valid = isValidPolicyItems(policyItems, failures, serviceDef) && valid; } } - if (serviceDef != null) { - Set<String> mandatoryResources = getMandatoryResourceNames(serviceDef); - Set<String> policyResources = getPolicyResources(policy); - Set<String> missingResources = Sets.difference(mandatoryResources, policyResources); - if (!missingResources.isEmpty()) { - failures.add(new ValidationFailureDetailsBuilder() - .field("resources") - .subField(missingResources.iterator().next()) // we return any one parameter! - .isMissing() - .becauseOf("required resources[" + missingResources + "] are missing") - .build()); - valid = false; - } - Set<String> allResource = getAllResourceNames(serviceDef); - Set<String> unknownResources = Sets.difference(policyResources, allResource); - if (!unknownResources.isEmpty()) { - failures.add(new ValidationFailureDetailsBuilder() - .field("resources") - .subField(unknownResources.iterator().next()) // we return any one parameter! - .isSemanticallyIncorrect() - .becauseOf("resource[" + unknownResources + "] is not valid for service-def[" + serviceDefName + "]") - .build()); - valid = false; + valid = isValidResources(policy, failures, action, serviceDef, serviceName) && valid; + } + + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("<== RangerPolicyValidator.isValid(%s, %s, %s): %s", policy, action, failures, valid)); + } + return valid; + } + + boolean isValidResources(RangerPolicy policy, final List<ValidationFailureDetails> failures, Action action, final RangerServiceDef serviceDef, final String serviceName) { + + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("==> RangerPolicyValidator.isValidResources(%s, %s, %s, %s, %s)", policy, failures, action, serviceDef, serviceName)); + } + + boolean valid = true; + if (serviceDef != null) { // following checks can't be done meaningfully otherwise + valid = isValidResourceNames(policy, failures, serviceDef); + Map<String, RangerPolicyResource> resourceMap = policy.getResources(); + valid = isValidResourceValues(resourceMap, failures, serviceDef) && valid; + valid = isValidResourceFlags(resourceMap, failures, serviceDef.getResources(), serviceDef.getName(), policy.getName()) && valid; + } + if (StringUtils.isNotBlank(serviceName)) { // resource uniqueness check cannot be done meaningfully otherwise + valid = isPolicyResourceUnique(policy, failures, action, serviceName) && valid; + } + + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("<== RangerPolicyValidator.isValidResources(%s, %s, %s, %s, %s): %s", policy, failures, action, serviceDef, serviceName, valid)); + } + return valid; + } + + boolean isPolicyResourceUnique(RangerPolicy policy, final List<ValidationFailureDetails> failures, Action action, final String serviceName) { + + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("==> RangerPolicyValidator.isPolicyResourceUnique(%s, %s, %s, %s)", policy, failures, action, serviceName)); + } + + boolean foundDuplicate = false; + RangerPolicyResourceSignature signature = _factory.createPolicyResourceSignature(policy); + List<RangerPolicy> policies = getPolicies(serviceName, null); + if (CollectionUtils.isNotEmpty(policies)) { + Iterator<RangerPolicy> iterator = policies.iterator(); + while (iterator.hasNext() && !foundDuplicate) { + RangerPolicy otherPolicy = iterator.next(); + if (otherPolicy.getId().equals(policy.getId()) && action == Action.UPDATE) { + LOG.debug("isPolicyResourceUnique: Skipping self during update!"); + } else { + RangerPolicyResourceSignature otherSignature = _factory.createPolicyResourceSignature(otherPolicy); + if (signature.equals(otherSignature)) { + foundDuplicate = true; + failures.add(new ValidationFailureDetailsBuilder() + .field("resources") + .isSemanticallyIncorrect() + .becauseOf("found another policy[" + policy.getName() + "] with matching resources[" + policy.getResources() + "]!") + .build()); + } } - Map<String, RangerPolicyResource> resourceMap = policy.getResources(); - valid = isValidResourceValues(resourceMap, failures, serviceDef) && valid; - valid = isValidResourceFlags(resourceMap, failures, serviceDef.getResources(), serviceDefName, policyName) && valid; } } + + boolean valid = !foundDuplicate; + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("<== RangerPolicyValidator.isPolicyResourceUnique(%s, %s, %s, %s): %s", policy, failures, action, serviceName, valid)); + } + return valid; + } + + boolean isValidResourceNames(final RangerPolicy policy, final List<ValidationFailureDetails> failures, final RangerServiceDef serviceDef) { if(LOG.isDebugEnabled()) { - LOG.debug(String.format("<== RangerPolicyValidator.isValid(%s, %s, %s): %s", policy, action, failures, valid)); + LOG.debug(String.format("==> RangerPolicyValidator.isValidResourceNames(%s, %s, %s)", policy, failures, serviceDef)); + } + + boolean valid = true; + Set<String> mandatoryResources = getMandatoryResourceNames(serviceDef); + Set<String> policyResources = getPolicyResources(policy); + Set<String> missingResources = Sets.difference(mandatoryResources, policyResources); + if (!missingResources.isEmpty()) { + failures.add(new ValidationFailureDetailsBuilder() + .field("resources") + .subField(missingResources.iterator().next()) // we return any one parameter! + .isMissing() + .becauseOf("required resources[" + missingResources + "] are missing") + .build()); + valid = false; + } + Set<String> allResource = getAllResourceNames(serviceDef); + Set<String> unknownResources = Sets.difference(policyResources, allResource); + if (!unknownResources.isEmpty()) { + failures.add(new ValidationFailureDetailsBuilder() + .field("resources") + .subField(unknownResources.iterator().next()) // we return any one parameter! + .isSemanticallyIncorrect() + .becauseOf("resource[" + unknownResources + "] is not valid for service-def[" + serviceDef.getName() + "]") + .build()); + valid = false; + } + + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("<== RangerPolicyValidator.isValidResourceNames(%s, %s, %s): %s", policy, failures, serviceDef, valid)); } return valid; } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/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 7bf744e..492949b 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 @@ -42,6 +42,7 @@ import org.apache.ranger.plugin.model.RangerServiceDef.RangerEnumDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerServiceConfigDef; import org.apache.ranger.plugin.store.ServiceStore; +import org.apache.ranger.plugin.util.RangerObjectFactory; import org.apache.ranger.plugin.util.SearchFilter; public abstract class RangerValidator { @@ -49,6 +50,7 @@ public abstract class RangerValidator { private static final Log LOG = LogFactory.getLog(RangerValidator.class); ServiceStore _store; + RangerObjectFactory _factory = new RangerObjectFactory(); public enum Action { CREATE, UPDATE, DELETE; @@ -242,15 +244,17 @@ public abstract class RangerValidator { return result; } - List<RangerPolicy> getPolicies(final String policyName, final String serviceName) { + List<RangerPolicy> getPolicies(final String serviceName, final String policyName) { if(LOG.isDebugEnabled()) { - LOG.debug("==> RangerValidator.getPolicies(" + policyName + ", " + serviceName + ")"); + LOG.debug("==> RangerValidator.getPolicies(" + serviceName + ", " + policyName + ")"); } List<RangerPolicy> policies = null; try { SearchFilter filter = new SearchFilter(); - filter.setParam(SearchFilter.POLICY_NAME, policyName); + if (StringUtils.isNotBlank(policyName)) { + filter.setParam(SearchFilter.POLICY_NAME, policyName); + } filter.setParam(SearchFilter.SERVICE_NAME, serviceName); policies = _store.getPolicies(filter); @@ -259,7 +263,8 @@ public abstract class RangerValidator { } if(LOG.isDebugEnabled()) { - LOG.debug("<== RangerValidator.getPolicies(" + policyName + ", " + serviceName + "): " + policies); + int count = policies == null ? 0 : policies.size(); + LOG.debug("<== RangerValidator.getPolicies(" + serviceName + ", " + policyName + "): count[" + count + "], " + policies); } return policies; } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java new file mode 100644 index 0000000..e02c968 --- /dev/null +++ b/agents-common/src/main/java/org/apache/ranger/plugin/util/RangerObjectFactory.java @@ -0,0 +1,10 @@ +package org.apache.ranger.plugin.util; + +import org.apache.ranger.plugin.model.RangerPolicy; +import org.apache.ranger.plugin.model.validation.RangerPolicyResourceSignature; + +public class RangerObjectFactory { + public RangerPolicyResourceSignature createPolicyResourceSignature(RangerPolicy policy) { + return new RangerPolicyResourceSignature(policy); + } +} http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyResourceSignature.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyResourceSignature.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyResourceSignature.java new file mode 100644 index 0000000..7d34d96 --- /dev/null +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyResourceSignature.java @@ -0,0 +1,213 @@ +package org.apache.ranger.plugin.model.validation; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; + +import org.apache.commons.codec.digest.DigestUtils; +import org.apache.ranger.plugin.model.RangerPolicy; +import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource; +import org.apache.ranger.plugin.model.validation.RangerPolicyResourceSignature.RangerPolicyResourceView; +import org.junit.Test; + +public class TestRangerPolicyResourceSignature { + + @Test + public void test_RangerPolicyResourceView_toString() { + // null resource + RangerPolicyResource policyResource = null; + RangerPolicyResourceView policyResourceView = new RangerPolicyResourceView(policyResource); + assertEquals("{}", policyResourceView.toString()); + + // non-null policy resource with null values/recursive flag + policyResource = createPolicyResource(null, null, null); + policyResourceView = new RangerPolicyResourceView(policyResource); + assertEquals("{values=,excludes=false,recursive=false}", policyResourceView.toString()); + + // valid values in non-asending order + policyResource = createPolicyResource(new String[]{"b", "a", "d", "c"}, true, false); + policyResourceView = new RangerPolicyResourceView(policyResource); + assertEquals("{values=[a, b, c, d],excludes=false,recursive=true}", policyResourceView.toString()); + + // recursive flag is false and different variation of values to show lexicographic ordering + policyResource = createPolicyResource(new String[]{"9", "A", "e", "_"}, false, true); + policyResourceView = new RangerPolicyResourceView(policyResource); + assertEquals("{values=[9, A, _, e],excludes=true,recursive=false}", policyResourceView.toString()); + } + + RangerPolicyResource createPolicyResource(String[] values, Boolean recursive, Boolean excludes) { + + RangerPolicyResource resource = mock(RangerPolicyResource.class); + if (values == null) { + when(resource.getValues()).thenReturn(null); + } else { + when(resource.getValues()).thenReturn(Arrays.asList(values)); + } + when(resource.getIsRecursive()).thenReturn(recursive); + when(resource.getIsExcludes()).thenReturn(excludes); + + return resource; + } + + @Test + public void test_isPolicyValidForResourceSignatureComputation() { + // null policy is invalid + RangerPolicyResourceSignature utils = new RangerPolicyResourceSignature((String)null); + RangerPolicy rangerPolicy = null; + assertFalse("policy==null", utils.isPolicyValidForResourceSignatureComputation(rangerPolicy)); + + // null resource map is invalid + rangerPolicy = mock(RangerPolicy.class); + when(rangerPolicy.getResources()).thenReturn(null); + assertFalse("policy.getResources()==null", utils.isPolicyValidForResourceSignatureComputation(rangerPolicy)); + + // empty resources map is ok! + Map<String, RangerPolicyResource> policyResources = new HashMap<String, RangerPolicyResource>(); + when(rangerPolicy.getResources()).thenReturn(policyResources); + assertTrue("policy.getResources().isEmpty()", utils.isPolicyValidForResourceSignatureComputation(rangerPolicy)); + + // but having a resource map with null key is not ok! + RangerPolicyResource aPolicyResource = mock(RangerPolicyResource.class); + policyResources.put(null, aPolicyResource); + assertFalse("policy.getResources().contains(null)", utils.isPolicyValidForResourceSignatureComputation(rangerPolicy)); + } + + @Test + public void test_RangerPolicyResourceSignature() { + // String rep of a null policy is an empty string! and its hash is sha of empty string! + RangerPolicyResourceSignature signature = new RangerPolicyResourceSignature((String)null); + assertEquals("", signature.asString()); + assertEquals(DigestUtils.md5Hex(""), signature.asHashHex()); + } + + /* + * Format of data expected by the utility function which uses this is: + * { "resource-name", "values" "isExcludes", "isRecursive" } + */ + Object[][] first = new Object[][] { + { "table", new String[] { "tbl3", "tbl1", "tbl2"}, true, false}, + { "db", new String[] { "db1", "db2"}, false, null}, + { "col", new String[] { "col2", "col1", "col3"}, null, true}, + }; + + Object[][] first_recursive_null_or_false = new Object[][] { + { "table", new String[] { "tbl3", "tbl1", "tbl2"}, true, null}, // recursive flag is false in first + { "db", new String[] { "db1", "db2"}, false, null}, + { "col", new String[] { "col2", "col1", "col3"}, null, true}, + }; + + Object[][] first_recursive_flag_different = new Object[][] { + { "table", new String[] { "tbl3", "tbl1", "tbl2"}, true, false}, + { "db", new String[] { "db1", "db2"}, false, null}, + { "col", new String[] { "col2", "col1", "col3"}, null, false}, // recursive flag is true in first + }; + + Object[][] first_excludes_null_or_false = new Object[][] { + { "table", new String[] { "tbl3", "tbl1", "tbl2"}, true, false}, + { "db", new String[] { "db1", "db2"}, false, null}, // excludes flag is null in first + { "col", new String[] { "col2", "col1", "col3"}, false, true}, + }; + + Object[][] first_excludes_flag_different = new Object[][] { + { "table", new String[] { "tbl3", "tbl1", "tbl2"}, true, false}, + { "db", new String[] { "db1", "db2"}, false, null}, + { "col", new String[] { "col2", "col1", "col3"}, true, true}, // excludes flag is false in first + }; + + Object[][] data_second = new Object[][] { + { "db", new String[] { "db2", "db1"}, false, null}, + { "table", new String[] { "tbl2", "tbl3", "tbl1"}, true, false}, + { "col", new String[] { "col1", "col3", "col2"}, null, true}, + }; + + @Test + public void test_getResourceSignature_happyPath() { + // null policy returns signature of empty resource + RangerPolicy policy = null; + RangerPolicyResourceSignature sig = new RangerPolicyResourceSignature(policy); + assertEquals(null, sig.getResourceString(policy)); + + policy = mock(RangerPolicy.class); + Map<String, RangerPolicyResource> policyResources = _utils.createPolicyResourceMap(first); + when(policy.getResources()).thenReturn(policyResources); + String expected = "{" + + "col={values=[col1, col2, col3],excludes=false,recursive=true}, " + + "db={values=[db1, db2],excludes=false,recursive=false}, " + + "table={values=[tbl1, tbl2, tbl3],excludes=true,recursive=false}" + + "}"; + assertEquals(expected, sig.getResourceString(policy)); + + // order of values should not matter + policyResources = _utils.createPolicyResourceMap(data_second); + when(policy.getResources()).thenReturn(policyResources); + assertEquals(expected, sig.getResourceString(policy)); + } + + + @Test + public void test_nullRecursiveFlagIsSameAsFlase() { + // create two policies with resources that differ only in the recursive flag such that flags are null in one and false in another + RangerPolicy policy1 = createPolicy(first); + RangerPolicy policy2 = createPolicy(first_recursive_null_or_false); + RangerPolicyResourceSignature signature = new RangerPolicyResourceSignature((String)null); + assertEquals("null is same as false", signature.getResourceString(policy1), signature.getResourceString(policy2)); + } + + @Test + public void test_onlyDifferByRecursiveFlag() { + // create two policies with resources that differ only in the recursive flag, i.e. null/false in one and true in another + RangerPolicy policy1 = createPolicy(first); + RangerPolicy policy2 = createPolicy(first_recursive_flag_different); + RangerPolicyResourceSignature signature = new RangerPolicyResourceSignature((String)null); + assertFalse("Resources differ only by recursive flag true vs false/null", signature.getResourceString(policy1).equals(signature.getResourceString(policy2))); + } + + @Test + public void test_nullExcludesFlagIsSameAsFlase() { + // create two policies with resources that differ only in the excludes flag such that flags are null in one and false in another + RangerPolicy policy1 = createPolicy(first); + RangerPolicy policy2 = createPolicy(first_excludes_null_or_false); + RangerPolicyResourceSignature signature = new RangerPolicyResourceSignature((String)null); + assertEquals("null is same as false", signature.getResourceString(policy1), signature.getResourceString(policy2)); + } + + @Test + public void test_onlyDifferByExcludesFlag() { + // create two policies with resources that differ only in the excludes flag, i.e. null/false in one and true in another + RangerPolicy policy1 = createPolicy(first); + RangerPolicy policy2 = createPolicy(first_excludes_flag_different); + RangerPolicyResourceSignature signature = new RangerPolicyResourceSignature((String)null); + assertFalse("Resources differ only by recursive flag true vs false/null", signature.getResourceString(policy1).equals(signature.getResourceString(policy2))); + } + + RangerPolicy createPolicy(Object[][] data) { + RangerPolicy policy = mock(RangerPolicy.class); + Map<String, RangerPolicyResource> resources = _utils.createPolicyResourceMap(data); + when(policy.getResources()).thenReturn(resources); + return policy; + } + + @Test + public void test_integration() { + // setup two policies with resources that are structurally different but semantically the same. + RangerPolicy aPolicy = mock(RangerPolicy.class); + Map<String, RangerPolicyResource> resources = _utils.createPolicyResourceMap(first); + when(aPolicy.getResources()).thenReturn(resources); + RangerPolicyResourceSignature signature = new RangerPolicyResourceSignature(aPolicy); + + RangerPolicy anotherPolicy = mock(RangerPolicy.class); + resources = _utils.createPolicyResourceMap(data_second); + when(anotherPolicy.getResources()).thenReturn(resources); + RangerPolicyResourceSignature anotherSignature = new RangerPolicyResourceSignature(anotherPolicy); + assertTrue(signature.equals(anotherSignature)); + assertTrue(anotherSignature.equals(signature)); + } + + ValidationTestUtils _utils = new ValidationTestUtils(); +} http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/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 e0f68ad..edf19d5 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 @@ -3,10 +3,12 @@ package org.apache.ranger.plugin.model.validation; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -19,13 +21,13 @@ 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.RangerResourceDef; -import org.apache.ranger.plugin.model.validation.RangerPolicyValidator; -import org.apache.ranger.plugin.model.validation.ValidationFailureDetails; import org.apache.ranger.plugin.model.validation.RangerValidator.Action; import org.apache.ranger.plugin.store.ServiceStore; +import org.apache.ranger.plugin.util.RangerObjectFactory; import org.apache.ranger.plugin.util.SearchFilter; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentMatcher; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Sets; @@ -38,6 +40,8 @@ public class TestRangerPolicyValidator { _policy = mock(RangerPolicy.class); _validator = new RangerPolicyValidator(_store); _serviceDef = mock(RangerServiceDef.class); + _factory = mock(RangerObjectFactory.class); + _validator._factory = _factory; } final Action[] cu = new Action[] { Action.CREATE, Action.UPDATE }; @@ -179,7 +183,13 @@ public class TestRangerPolicyValidator { when(_serviceDef.getResources()).thenReturn(resourceDefs); Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap(policyResourceMap_good); when(_policy.getResources()).thenReturn(resourceMap); - + // let's add some other policies in the store for this service that have a different signature + SearchFilter resourceDuplicationFilter = new SearchFilter(); + resourceDuplicationFilter.setParam(SearchFilter.SERVICE_NAME, "service-name"); + when(_factory.createPolicyResourceSignature(_policy)).thenReturn(new RangerPolicyResourceSignature("policy")); + when(_factory.createPolicyResourceSignature(existingPolicy)).thenReturn(new RangerPolicyResourceSignature("policy-name-2")); + // we are reusing the same policies collection here -- which is fine + when(_store.getPolicies(resourceDuplicationFilter)).thenReturn(existingPolicies); for (Action action : cu) { if (action == Action.CREATE) { when(_policy.getId()).thenReturn(7L); @@ -277,17 +287,35 @@ public class TestRangerPolicyValidator { // policy must have service name on it and it should be valid when(_policy.getName()).thenReturn("policy-name"); + for (Action action : cu) { + when(_policy.getService()).thenReturn(null); + _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); + _utils.checkFailureForMissingValue(_failures, "service"); + + when(_policy.getService()).thenReturn(""); + _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); + _utils.checkFailureForMissingValue(_failures, "service"); + } + + // service name should be valid when(_store.getServiceByName("service-name")).thenReturn(null); when(_store.getServiceByName("another-service-name")).thenThrow(new Exception()); - for (Action action : cu) { - when(_policy.getService()).thenReturn("service-name"); + when(_policy.getService()).thenReturn(null); _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); _utils.checkFailureForMissingValue(_failures, "service"); - when(_policy.getService()).thenReturn("another-service-name"); + when(_policy.getService()).thenReturn(null); _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); _utils.checkFailureForMissingValue(_failures, "service"); + + when(_policy.getService()).thenReturn("service-name"); + _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); + _utils.checkFailureForSemanticError(_failures, "service"); + + when(_policy.getService()).thenReturn("another-service-name"); + _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); + _utils.checkFailureForSemanticError(_failures, "service"); } // policy must contain at least one policy item @@ -331,7 +359,8 @@ public class TestRangerPolicyValidator { List<RangerResourceDef> resourceDefs = _utils.createResourceDefs(resourceDefData); when(_serviceDef.getResources()).thenReturn(resourceDefs); when(_store.getServiceDefByName("service-type")).thenReturn(_serviceDef); - // one mandtory is missing (tbl) and one unknown resource is specified (extra), and values of option resource don't conform to validation pattern (col) + + // one mandatory is missing (tbl) and one unknown resource is specified (extra), and values of option resource don't conform to validation pattern (col) Map<String, RangerPolicyResource> policyResources = _utils.createPolicyResourceMap(policyResourceMap_bad); when(_policy.getResources()).thenReturn(policyResources); for (Action action : cu) { @@ -342,6 +371,28 @@ public class TestRangerPolicyValidator { _utils.checkFailureForSemanticError(_failures, "isRecursive", "db"); // for specifying it as true when def did not allow it _utils.checkFailureForSemanticError(_failures, "isExcludes", "col"); // for specifying it as true when def did not allow it } + + // create the right resource def but let it clash with another policy with matching resource-def + policyResources = _utils.createPolicyResourceMap(policyResourceMap_good); + when(_policy.getResources()).thenReturn(policyResources); + filter = new SearchFilter(); filter.setParam(SearchFilter.SERVICE_NAME, "service-name"); + when(_store.getPolicies(filter)).thenReturn(existingPolicies); + // we are doctoring the factory to always return the same signature + when(_factory.createPolicyResourceSignature(anyPolicy())).thenReturn(new RangerPolicyResourceSignature("blah")); + for (Action action : cu) { + _failures.clear(); assertFalse(_validator.isValid(_policy, action, _failures)); + _utils.checkFailureForSemanticError(_failures, "resources"); + } + } + + RangerPolicy anyPolicy() { + return argThat(new ArgumentMatcher<RangerPolicy>() { + + @Override + public boolean matches(Object argument) { + return true; + } + }); } @Test @@ -480,10 +531,11 @@ public class TestRangerPolicyValidator { }; private Object[][] policyResourceMap_happyPath = new Object[][] { - // { "resource-name", "isExcludes", "isRecursive" } - { "db", null, true }, // null should be treated as false - { "tbl", false, false }, // set to false where def is null and def is true - { "col", true, null} // set to null where def is false + // { "resource-name", "values" "isExcludes", "isRecursive" } + // values collection is null as it isn't relevant to the part being tested with this data + { "db", null, null, true }, // null should be treated as false + { "tbl", null, false, false }, // set to false where def is null and def is true + { "col", null, true, null} // set to null where def is false }; @Test @@ -491,34 +543,73 @@ public class TestRangerPolicyValidator { // passing null values effectively bypasses the filter assertTrue(_validator.isValidResourceFlags(null, _failures, null, "a-service-def", "a-policy")); // so does passing in empty collections - Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap2(policyResourceMap_happyPath); + Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap(policyResourceMap_happyPath); List<RangerResourceDef> resourceDefs = _utils.createResourceDefs2(resourceDef_happyPath); when(_serviceDef.getResources()).thenReturn(resourceDefs); assertTrue(_validator.isValidResourceFlags(resourceMap, _failures, resourceDefs, "a-service-def", "a-policy")); } private Object[][] policyResourceMap_failures = new Object[][] { - // { "resource-name", "isExcludes", "isRecursive" } - { "db", true, true }, // ok: def has true for both - { "tbl", true, null }, // excludes: def==false, policy==true - { "col", false, true } // recursive: def==null (i.e. false), policy==true + // { "resource-name", "values" "isExcludes", "isRecursive" } + // values collection is null as it isn't relevant to the part being tested with this data + { "db", null, true, true }, // ok: def has true for both + { "tbl", null, true, null }, // excludes: def==false, policy==true + { "col", null, false, true } // recursive: def==null (i.e. false), policy==true }; @Test public final void test_isValidResourceFlags_failures() { // passing true when def says false/null List<RangerResourceDef> resourceDefs = _utils.createResourceDefs2(resourceDef_happyPath); - Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap2(policyResourceMap_failures); + Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap(policyResourceMap_failures); when(_serviceDef.getResources()).thenReturn(resourceDefs); assertFalse(_validator.isValidResourceFlags(resourceMap, _failures, resourceDefs, "a-service-def", "a-policy")); _utils.checkFailureForSemanticError(_failures, "isExcludes", "tbl"); _utils.checkFailureForSemanticError(_failures, "isRecursive", "col"); } + @Test + public final void test_isPolicyResourceUnique() throws Exception { + + RangerPolicy[] policies = new RangerPolicy[3]; + RangerPolicyResourceSignature[] signatures = new RangerPolicyResourceSignature[3]; + for (int i = 0; i < 3; i++) { + RangerPolicy policy = mock(RangerPolicy.class); + when(policy.getId()).thenReturn((long)i); + policies[i] = policy; + signatures[i] = new RangerPolicyResourceSignature("policy" + i); + when(_factory.createPolicyResourceSignature(policies[i])).thenReturn(signatures[i]); + } + + SearchFilter searchFilter = new SearchFilter(); + String serviceName = "aService"; + searchFilter.setParam(SearchFilter.SERVICE_NAME, serviceName); + + List<RangerPolicy> existingPolicies = Arrays.asList(new RangerPolicy[] { policies[1], policies[2]} ); + // all existing policies have distinct signatures + for (Action action : cu) { + when(_store.getPolicies(searchFilter)).thenReturn(existingPolicies); + assertTrue("No duplication: " + action, _validator.isPolicyResourceUnique(policies[0], _failures, action, serviceName)); + } + + // Failure if signature matches an existing policy + // We change the signature of 3rd policy to be same as that of 1st so duplication check will fail + for (Action action : cu) { + when(_factory.createPolicyResourceSignature(policies[2])).thenReturn(new RangerPolicyResourceSignature("policy0")); + when(_store.getPolicies(searchFilter)).thenReturn(existingPolicies); + assertFalse("Duplication:" + action, _validator.isPolicyResourceUnique(policies[0], _failures, action, serviceName)); + } + + // update should exclude itself! - let's change id of 3rd policy to be the same as the 1st one. + when(policies[2].getId()).thenReturn((long)0); + assertTrue("No duplication if updating policy", _validator.isPolicyResourceUnique(policies[0], _failures, Action.UPDATE, serviceName)); + } + private ValidationTestUtils _utils = new ValidationTestUtils(); private List<ValidationFailureDetails> _failures = new ArrayList<ValidationFailureDetails>(); private ServiceStore _store; private RangerPolicy _policy; private RangerPolicyValidator _validator; private RangerServiceDef _serviceDef; + private RangerObjectFactory _factory; } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/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 f17b2c2..f014552 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 @@ -401,15 +401,29 @@ public class TestRangerValidator { filter.setParam(SearchFilter.SERVICE_NAME, serviceName); when(_store.getPolicies(filter)).thenReturn(null); - List<RangerPolicy> result = _validator.getPolicies(policyName, serviceName); + List<RangerPolicy> result = _validator.getPolicies(serviceName, policyName); // validate store is queried with both parameters verify(_store).getPolicies(filter); assertNull(result); // returns null if store throws an exception when(_store.getPolicies(filter)).thenThrow(new Exception()); - result = _validator.getPolicies(policyName, serviceName); + result = _validator.getPolicies(serviceName, policyName); assertNull(result); + + // does not shove policy into search filter if policy name passed in is "blank" + filter = new SearchFilter(); + filter.setParam(SearchFilter.SERVICE_NAME, serviceName); + + List<RangerPolicy> policies = new ArrayList<RangerPolicy>(); + RangerPolicy policy = mock(RangerPolicy.class); + policies.add(policy); + + when(_store.getPolicies(filter)).thenReturn(policies); + for (String aName : new String[]{ null, "", " "}) { + result = _validator.getPolicies(serviceName, aName); + assertTrue(result.iterator().next() == policy); + } } @Test http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/32f3262b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java index 5ed2691..1e81ec3 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java @@ -295,23 +295,6 @@ public class ValidationTestUtils { return defs; } - Map<String, RangerPolicyResource> createPolicyResourceMap2(Object[][] input) { - if (input == null) { - return null; - } - Map<String, RangerPolicyResource> result = new HashMap<String, RangerPolicyResource>(input.length); - for (Object[] row : input) { - String resourceName = (String)row[0]; - Boolean isExcludes = (Boolean)row[1]; - Boolean isRecursive = (Boolean)row[2]; - RangerPolicyResource aResource = mock(RangerPolicyResource.class); - when(aResource.getIsExcludes()).thenReturn(isExcludes); - when(aResource.getIsRecursive()).thenReturn(isRecursive); - result.put(resourceName, aResource); - } - return result; - } - List<RangerEnumElementDef> createEnumElementDefs(String[] input) { if (input == null) { return null;
