Repository: incubator-ranger Updated Branches: refs/heads/master 2a429d703 -> f991a8b62
RANGER-462 Signature check should not span a service and several other changes related to policy signature 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/f991a8b6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/f991a8b6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/f991a8b6 Branch: refs/heads/master Commit: f991a8b62c3b735c0e30518bc3b0df3de46035e5 Parents: 2a429d7 Author: Alok Lal <[email protected]> Authored: Sat May 9 13:51:35 2015 -0700 Committer: Madhan Neethiraj <[email protected]> Committed: Mon May 11 14:53:56 2015 -0700 ---------------------------------------------------------------------- .../model/RangerPolicyResourceSignature.java | 80 +++++++++++------- .../model/validation/RangerPolicyValidator.java | 44 ++++++---- .../model/validation/RangerValidator.java | 8 +- .../plugin/store/AbstractServiceStore.java | 29 ++++--- .../ranger/plugin/store/ServiceStore.java | 2 +- .../plugin/store/file/ServiceFileStore.java | 9 ++- .../plugin/store/rest/ServiceRESTStore.java | 2 +- .../TestRangerPolicyResourceSignature.java | 85 ++++++++++++-------- .../validation/TestRangerPolicyValidator.java | 36 +++++++-- .../model/validation/TestRangerValidator.java | 14 ++-- .../org/apache/ranger/biz/ServiceDBStore.java | 21 +++-- .../java/org/apache/ranger/db/XXPolicyDao.java | 23 +++++- .../resources/META-INF/jpa_named_queries.xml | 9 ++- 13 files changed, 239 insertions(+), 123 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java index 62fdabc..6d19f44 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java @@ -33,6 +33,7 @@ import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource; public class RangerPolicyResourceSignature { + static final int _SignatureVersion = 1; private static final Log LOG = LogFactory.getLog(RangerPolicyResourceSignature.class); static final RangerPolicyResourceSignature _EmptyResourceSignature = new RangerPolicyResourceSignature((RangerPolicy)null); @@ -42,7 +43,8 @@ public class RangerPolicyResourceSignature { public RangerPolicyResourceSignature(RangerPolicy policy) { _policy = policy; - String asString = getResourceString(_policy); + PolicySerializer serializer = new PolicySerializer(_policy); + String asString = serializer.toString(); if (asString == null) { _string = ""; } else { @@ -93,40 +95,62 @@ public class RangerPolicyResourceSignature { 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; + static class PolicySerializer { + final RangerPolicy _policy; + PolicySerializer(RangerPolicy policy) { + _policy = policy; } - 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; + boolean isPolicyValidForResourceSignatureComputation() { + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("==> RangerPolicyResourceSignature.isPolicyValidForResourceSignatureComputation(%s)", _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; + } + + if (LOG.isDebugEnabled()) { + LOG.debug(String.format("<== RangerPolicyResourceSignature.isPolicyValidForResourceSignatureComputation(%s): %s", _policy, valid)); + } + return valid; } - return valid; + + @Override + public String toString() { + // invalid/empty policy gets a deterministic signature as if it had an + // empty resource string + if (!isPolicyValidForResourceSignatureComputation()) { + return null; + } + int type = 0; + if (_policy.getPolicyType() != null) { + type = _policy.getPolicyType(); + } + Map<String, ResourceSerializer> resources = new TreeMap<String, ResourceSerializer>(); + for (Map.Entry<String, RangerPolicyResource> entry : _policy.getResources().entrySet()) { + String resourceName = entry.getKey(); + ResourceSerializer resourceView = new ResourceSerializer(entry.getValue()); + resources.put(resourceName, resourceView); + } + String resource = resources.toString(); + String result = String.format("{version=%d,type=%d,resource=%s}", _SignatureVersion, type, resource); + return result; + } + } - static class RangerPolicyResourceView { + static class ResourceSerializer { final RangerPolicyResource _policyResource; - RangerPolicyResourceView(RangerPolicyResource policyResource) { + ResourceSerializer(RangerPolicyResource policyResource) { _policyResource = policyResource; } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/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 1acf81f..caaf68f 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 @@ -178,6 +178,7 @@ public class RangerPolicyValidator extends RangerValidator { } } RangerService service = null; + boolean serviceNameValid = false; if (StringUtils.isBlank(serviceName)) { failures.add(new ValidationFailureDetailsBuilder() .field("service") @@ -194,6 +195,8 @@ public class RangerPolicyValidator extends RangerValidator { .becauseOf("service does not exist") .build()); valid = false; + } else { + serviceNameValid = true; } } List<RangerPolicyItem> policyItems = policy.getPolicyItems(); @@ -221,7 +224,9 @@ public class RangerPolicyValidator extends RangerValidator { valid = isValidPolicyItems(policyItems, failures, serviceDef) && valid; } } - valid = isValidResources(policy, failures, action, isAdmin, serviceDef, serviceName) && valid; + if (serviceNameValid) { // resource checks can't be done meaningfully otherwise + valid = isValidResources(policy, failures, action, isAdmin, serviceDef) && valid; + } } if(LOG.isDebugEnabled()) { @@ -230,10 +235,11 @@ public class RangerPolicyValidator extends RangerValidator { return valid; } - boolean isValidResources(RangerPolicy policy, final List<ValidationFailureDetails> failures, Action action, boolean isAdmin, final RangerServiceDef serviceDef, final String serviceName) { + boolean isValidResources(RangerPolicy policy, final List<ValidationFailureDetails> failures, Action action, + boolean isAdmin, final RangerServiceDef serviceDef) { if(LOG.isDebugEnabled()) { - LOG.debug(String.format("==> RangerPolicyValidator.isValidResources(%s, %s, %s, %s, %s, %s)", policy, failures, action, isAdmin, serviceDef, serviceName)); + LOG.debug(String.format("==> RangerPolicyValidator.isValidResources(%s, %s, %s, %s, %s)", policy, failures, action, isAdmin, serviceDef)); } boolean valid = true; @@ -248,7 +254,7 @@ public class RangerPolicyValidator extends RangerValidator { } if(LOG.isDebugEnabled()) { - LOG.debug(String.format("<== RangerPolicyValidator.isValidResources(%s, %s, %s, %s, %s, %s): %s", policy, failures, action, isAdmin, serviceDef, serviceName, valid)); + LOG.debug(String.format("<== RangerPolicyValidator.isValidResources(%s, %s, %s, %s, %s): %s", policy, failures, action, isAdmin, serviceDef, valid)); } return valid; } @@ -260,19 +266,23 @@ public class RangerPolicyValidator extends RangerValidator { } boolean valid = true; - RangerPolicyResourceSignature policySignature = _factory.createPolicyResourceSignature(policy); - String signature = policySignature.getSignature(); - List<RangerPolicy> policies = getPoliciesForResourceSignature(signature); - if (CollectionUtils.isNotEmpty(policies)) { - RangerPolicy matchedPolicy = policies.iterator().next(); - // there shouldn't be a matching policy for create. During update only match should be to itself - if (action == Action.CREATE || (action == Action.UPDATE && (policies.size() > 1 || !matchedPolicy.getId().equals(policy.getId())))) { - failures.add(new ValidationFailureDetailsBuilder() - .field("resources") - .isSemanticallyIncorrect() - .becauseOf("found another policy[" + matchedPolicy.getName() + "] with matching resources[" + matchedPolicy.getResources() + "]!") - .build()); - valid = false; + if (!Boolean.TRUE.equals(policy.getIsEnabled())) { + LOG.debug("Policy is disabled. Skipping resource uniqueness validation."); + } else { + RangerPolicyResourceSignature policySignature = _factory.createPolicyResourceSignature(policy); + String signature = policySignature.getSignature(); + List<RangerPolicy> policies = getPoliciesForResourceSignature(policy.getService(), signature); + if (CollectionUtils.isNotEmpty(policies)) { + RangerPolicy matchedPolicy = policies.iterator().next(); + // there shouldn't be a matching policy for create. During update only match should be to itself + if (action == Action.CREATE || (action == Action.UPDATE && (policies.size() > 1 || !matchedPolicy.getId().equals(policy.getId())))) { + failures.add(new ValidationFailureDetailsBuilder() + .field("resources") + .isSemanticallyIncorrect() + .becauseOf("found another policy[" + matchedPolicy.getName() + "] with matching resources[" + matchedPolicy.getResources() + "]!") + .build()); + valid = false; + } } } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/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 ec16eee..3246138 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 @@ -270,21 +270,21 @@ public abstract class RangerValidator { return policies; } - List<RangerPolicy> getPoliciesForResourceSignature(String hexSignature) { + List<RangerPolicy> getPoliciesForResourceSignature(String serviceName, String policySignature) { if(LOG.isDebugEnabled()) { - LOG.debug("==> RangerValidator.getPolicies(" + hexSignature + ")"); + LOG.debug(String.format("==> RangerValidator.getPoliciesForResourceSignature(%s, %s)", serviceName, policySignature)); } List<RangerPolicy> policies = null; try { - policies = _store.getPoliciesByResourceSignature(hexSignature); + policies = _store.getPoliciesByResourceSignature(serviceName, policySignature, true); // only look for enabled policies } catch (Exception e) { LOG.debug("Encountred exception while retrieving policies from service store!", e); } if(LOG.isDebugEnabled()) { int count = policies == null ? 0 : policies.size(); - LOG.debug("<== RangerValidator.getPolicies(" + hexSignature + "): count[" + count + "], " + policies); + LOG.debug(String.format("<== RangerValidator.getPoliciesForResourceSignature(%s, %s): count[%d], %s", serviceName, policySignature, count, policies)); } return policies; } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java b/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java index 2ce08bb..9bba5e3 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/store/AbstractServiceStore.java @@ -26,6 +26,7 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; @@ -81,7 +82,10 @@ public abstract class AbstractServiceStore implements ServiceStore { addPredicateForIsRecursive(filter.getParam(SearchFilter.IS_RECURSIVE), predicates); addPredicateForUserName(filter.getParam(SearchFilter.USER), predicates); addPredicateForGroupName(filter.getParam(SearchFilter.GROUP), predicates); - addPredicateForResourceSignature(filter.getParam(SearchFilter.RESOURCE_SIGNATURE), predicates); + addPredicateForResourceSignature( + filter.getParam(SearchFilter.SERVICE_NAME), + filter.getParam(SearchFilter.RESOURCE_SIGNATURE), + filter.getParam(SearchFilter.IS_ENABLED), predicates); addPredicateForResources(filter.getParamsWithPrefix(SearchFilter.RESOURCE_PREFIX, true), predicates); Predicate ret = CollectionUtils.isEmpty(predicates) ? null : PredicateUtils.allPredicate(predicates); @@ -685,9 +689,13 @@ public abstract class AbstractServiceStore implements ServiceStore { return ret; } - private Predicate addPredicateForResourceSignature(final String hexSignature, List<Predicate> predicates) { + private Predicate addPredicateForResourceSignature(final String serviceName, String signature, String isPolicyEnabled, List<Predicate> predicates) { - Predicate ret = createPredicateForResourceSignature(hexSignature); + boolean enabled = false; + if ("1".equals(isPolicyEnabled)) { + enabled = true; + } + Predicate ret = createPredicateForResourceSignature(serviceName, signature, enabled); if(predicates != null && ret != null) { predicates.add(ret); @@ -695,15 +703,16 @@ public abstract class AbstractServiceStore implements ServiceStore { return ret; } - + /** - * NOTE: Null or empty hexSignature, though invalid, is supported in search. - * @param hexSignature + * @param serviceName + * @param policySignature + * @param isPolicyEnabled * @return */ - public Predicate createPredicateForResourceSignature(final String hexSignature) { + public Predicate createPredicateForResourceSignature(final String serviceName, final String policySignature, final Boolean isPolicyEnabled) { - if(StringUtils.isEmpty(hexSignature)) { + if (StringUtils.isEmpty(policySignature) || StringUtils.isEmpty(serviceName) || isPolicyEnabled == null) { return null; } @@ -719,7 +728,9 @@ public abstract class AbstractServiceStore implements ServiceStore { if (object instanceof RangerPolicy) { RangerPolicy policy = (RangerPolicy)object; - ret = StringUtils.equals(hexSignature, policy.getResourceSignature()); + ret = StringUtils.equals(policy.getResourceSignature(), policySignature) && + Objects.equals(policy.getService(), serviceName) && + Objects.equals(policy.getIsEnabled(), isPolicyEnabled); } else { ret = true; } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java b/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java index 708fbd2..b998e93 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java @@ -66,7 +66,7 @@ public interface ServiceStore { List<RangerPolicy> getPolicies(SearchFilter filter) throws Exception; - List<RangerPolicy> getPoliciesByResourceSignature(String hexSignature) throws Exception; + List<RangerPolicy> getPoliciesByResourceSignature(String serviceName, String policySignature, Boolean isPolicyEnabled) throws Exception; List<RangerPolicy> getServicePolicies(Long serviceId, SearchFilter filter) throws Exception; http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/agents-common/src/main/java/org/apache/ranger/plugin/store/file/ServiceFileStore.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/store/file/ServiceFileStore.java b/agents-common/src/main/java/org/apache/ranger/plugin/store/file/ServiceFileStore.java index 00b7521..2c161a7 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/store/file/ServiceFileStore.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/store/file/ServiceFileStore.java @@ -615,17 +615,18 @@ public class ServiceFileStore extends BaseFileStore { } @Override - public List<RangerPolicy> getPoliciesByResourceSignature(String hexSignature) throws Exception { + public List<RangerPolicy> getPoliciesByResourceSignature(String serviceName, String policySignature, Boolean isPolicyEnabled) throws Exception { if (LOG.isDebugEnabled()) { - LOG.debug("==> ServiceFileStore.getPolicies()"); + LOG.debug(String.format("==> ServiceFileStore.getPoliciesByResourceSignature(%s, %s, %s)", serviceName, policySignature, isPolicyEnabled)); } List<RangerPolicy> ret = getAllPolicies(); - CollectionUtils.filter(ret, createPredicateForResourceSignature(hexSignature)); + CollectionUtils.filter(ret, createPredicateForResourceSignature(serviceName, policySignature, isPolicyEnabled)); if (LOG.isDebugEnabled()) { - LOG.debug("<== ServiceFileStore.getPolicies(): count=" + (ret == null ? 0 : ret.size())); + LOG.debug(String.format("<== ServiceFileStore.getPoliciesByResourceSignature(%s, %s, %s): count[%d]: %s", + serviceName, policySignature, isPolicyEnabled, (ret == null ? 0 : ret.size()), ret)); } return ret; http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/agents-common/src/main/java/org/apache/ranger/plugin/store/rest/ServiceRESTStore.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/store/rest/ServiceRESTStore.java b/agents-common/src/main/java/org/apache/ranger/plugin/store/rest/ServiceRESTStore.java index 5c742f9..71405cf 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/store/rest/ServiceRESTStore.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/store/rest/ServiceRESTStore.java @@ -614,7 +614,7 @@ public class ServiceRESTStore implements ServiceStore { } @Override - public List<RangerPolicy> getPoliciesByResourceSignature(String hexSignature) throws Exception { + public List<RangerPolicy> getPoliciesByResourceSignature(String serviceName, String policySignature, Boolean isPolicyEnabled) throws Exception { throw new UnsupportedOperationException("Querying policies by resource signature is not supported!"); } } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/agents-common/src/test/java/org/apache/ranger/plugin/model/TestRangerPolicyResourceSignature.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/TestRangerPolicyResourceSignature.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/TestRangerPolicyResourceSignature.java index 46e924f..a605cd7 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/TestRangerPolicyResourceSignature.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/TestRangerPolicyResourceSignature.java @@ -30,10 +30,9 @@ 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.RangerPolicyResourceSignature; import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource; -import org.apache.ranger.plugin.model.RangerPolicyResourceSignature.RangerPolicyResourceView; +import org.apache.ranger.plugin.model.RangerPolicyResourceSignature.ResourceSerializer; +import org.apache.ranger.plugin.model.RangerPolicyResourceSignature.PolicySerializer; import org.apache.ranger.plugin.model.validation.ValidationTestUtils; import org.junit.Test; @@ -42,24 +41,24 @@ public class TestRangerPolicyResourceSignature { @Test public void test_RangerPolicyResourceView_toString() { // null resource - RangerPolicyResource policyResource = null; - RangerPolicyResourceView policyResourceView = new RangerPolicyResourceView(policyResource); - assertEquals("{}", policyResourceView.toString()); + RangerPolicyResource resource = null; + ResourceSerializer serializer = new ResourceSerializer(resource); + assertEquals("{}", serializer.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()); + resource = createPolicyResource(null, null, null); + serializer = new ResourceSerializer(resource); + assertEquals("{values=,excludes=false,recursive=false}", serializer.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()); + resource = createPolicyResource(new String[]{"b", "a", "d", "c"}, true, false); + serializer = new ResourceSerializer(resource); + assertEquals("{values=[a, b, c, d],excludes=false,recursive=true}", serializer.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()); + resource = createPolicyResource(new String[]{"9", "A", "e", "_"}, false, true); + serializer = new ResourceSerializer(resource); + assertEquals("{values=[9, A, _, e],excludes=true,recursive=false}", serializer.toString()); } RangerPolicyResource createPolicyResource(String[] values, Boolean recursive, Boolean excludes) { @@ -79,24 +78,27 @@ public class TestRangerPolicyResourceSignature { @Test public void test_isPolicyValidForResourceSignatureComputation() { // null policy is invalid - RangerPolicyResourceSignature utils = new RangerPolicyResourceSignature((String)null); RangerPolicy rangerPolicy = null; - assertFalse("policy==null", utils.isPolicyValidForResourceSignatureComputation(rangerPolicy)); + PolicySerializer policySerializer = new PolicySerializer(rangerPolicy); + assertFalse("policy==null", policySerializer.isPolicyValidForResourceSignatureComputation()); // null resource map is invalid rangerPolicy = mock(RangerPolicy.class); when(rangerPolicy.getResources()).thenReturn(null); - assertFalse("policy.getResources()==null", utils.isPolicyValidForResourceSignatureComputation(rangerPolicy)); + policySerializer = new PolicySerializer(rangerPolicy); + assertFalse("policy.getResources()==null", policySerializer.isPolicyValidForResourceSignatureComputation()); // 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)); + policySerializer = new PolicySerializer(rangerPolicy); + assertTrue("policy.getResources().isEmpty()", policySerializer.isPolicyValidForResourceSignatureComputation()); // 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)); + policySerializer = new PolicySerializer(rangerPolicy); + assertFalse("policy.getResources().contains(null)", policySerializer.isPolicyValidForResourceSignatureComputation()); } @Test @@ -151,23 +153,34 @@ public class TestRangerPolicyResourceSignature { 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)); + PolicySerializer serializer = new PolicySerializer(policy); + assertTrue("Null policy", serializer.toString() == null); policy = mock(RangerPolicy.class); + when(policy.getPolicyType()).thenReturn(null); Map<String, RangerPolicyResource> policyResources = _utils.createPolicyResourceMap(first); when(policy.getResources()).thenReturn(policyResources); - String expected = "{" + + serializer = new PolicySerializer(policy); + String expectedVersion = "version=1"; + String expectedType = "type=0"; + String expectedResource = "{" + "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)); + "}"; + String serializationFormat = "{%s,%s,resource=%s}"; + String expectedFull = String.format(serializationFormat, expectedVersion, expectedType, expectedResource); + assertEquals(expectedFull, serializer.toString()); // order of values should not matter policyResources = _utils.createPolicyResourceMap(data_second); when(policy.getResources()).thenReturn(policyResources); - assertEquals(expected, sig.getResourceString(policy)); + assertEquals(expectedFull, serializer.toString()); + // changing the policy type has expected changes + when(policy.getPolicyType()).thenReturn(1); + expectedType="type=1"; + expectedFull = String.format(serializationFormat, expectedVersion, expectedType, expectedResource); + assertEquals(expectedFull, serializer.toString()); } @@ -176,8 +189,9 @@ public class TestRangerPolicyResourceSignature { // 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)); + RangerPolicyResourceSignature signature1 = new RangerPolicyResourceSignature(policy1); + RangerPolicyResourceSignature signature2 = new RangerPolicyResourceSignature(policy2); + assertEquals("Recursive flag: null is same as false", signature1.toString(), signature2.toString()); } @Test @@ -185,8 +199,9 @@ public class TestRangerPolicyResourceSignature { // 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))); + RangerPolicyResourceSignature signature1 = new RangerPolicyResourceSignature(policy1); + RangerPolicyResourceSignature signature2 = new RangerPolicyResourceSignature(policy2); + assertFalse("Resources differ only by recursive flag true vs false/null", signature1.toString().equals(signature2.toString())); } @Test @@ -194,8 +209,9 @@ public class TestRangerPolicyResourceSignature { // 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)); + RangerPolicyResourceSignature signature1 = new RangerPolicyResourceSignature(policy1); + RangerPolicyResourceSignature signature2 = new RangerPolicyResourceSignature(policy2); + assertEquals("Excludes flag: null is same as false", signature1.toString(), signature2.toString()); } @Test @@ -203,8 +219,9 @@ public class TestRangerPolicyResourceSignature { // 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))); + RangerPolicyResourceSignature signature1 = new RangerPolicyResourceSignature(policy1); + RangerPolicyResourceSignature signature2 = new RangerPolicyResourceSignature(policy2); + assertFalse("Resources differ only by excludes flag true vs false/null", signature1.toString().equals(signature2.toString())); } RangerPolicy createPolicy(Object[][] data) { http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/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 5828f6f..27c6318 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 @@ -252,7 +252,7 @@ public class TestRangerPolicyValidator { when(_factory.createPolicyResourceSignature(_policy)).thenReturn(policySignature); // setup the store to indicate that no other policy exists with matching signature when(policySignature.getSignature()).thenReturn("hash-1"); - when(_store.getPoliciesByResourceSignature("hash-1")).thenReturn(null); + when(_store.getPoliciesByResourceSignature("service-name", "hash-1", true)).thenReturn(null); // we are reusing the same policies collection here -- which is fine for (Action action : cu) { if (action == Action.CREATE) { @@ -447,7 +447,7 @@ public class TestRangerPolicyValidator { RangerPolicyResourceSignature signature = mock(RangerPolicyResourceSignature.class); when(_factory.createPolicyResourceSignature(_policy)).thenReturn(signature); when(signature.getSignature()).thenReturn("hash-1"); - when(_store.getPoliciesByResourceSignature("hash-1")).thenReturn(null); // store does not have any policies for that signature hash + when(_store.getPoliciesByResourceSignature("service-name", "hash-1", true)).thenReturn(null); // store does not have any policies for that signature hash for (Action action : cu) { for (boolean isAdmin : new boolean[] { true, false }) { _failures.clear(); assertFalse(_validator.isValid(_policy, action, isAdmin, _failures)); @@ -460,7 +460,7 @@ public class TestRangerPolicyValidator { } // Check if error around resource signature clash are reported. have Store return policies for same signature - when(_store.getPoliciesByResourceSignature("hash-1")).thenReturn(existingPolicies); + when(_store.getPoliciesByResourceSignature("service-name", "hash-1", true)).thenReturn(existingPolicies); for (Action action : cu) { for (boolean isAdmin : new boolean[] { true, false }) { _failures.clear(); assertFalse(_validator.isValid(_policy, action, isAdmin, _failures)); @@ -676,35 +676,55 @@ public class TestRangerPolicyValidator { String hash = "hash-1"; when(signature.getSignature()).thenReturn(hash); when(_factory.createPolicyResourceSignature(_policy)).thenReturn(signature); + when(_policy.getService()).thenReturn("service-name"); List<RangerPolicy> policies = null; - when(_store.getPoliciesByResourceSignature(hash)).thenReturn(policies); + when(_store.getPoliciesByResourceSignature("service-name", hash, true)).thenReturn(policies); policies = new ArrayList<RangerPolicy>(); for (Action action : cu) { assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, action)); assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, action)); } /* - * If store does have any policy then test should fail with appropriate error message. - * For create any match is a problem + * If store has a policy with matching signature then the check should fail with appropriate error message. + * - For create any match is a problem + * - Signature check can never fail for disabled policies! */ RangerPolicy policy1 = mock(RangerPolicy.class); policies.add(policy1); - when(_store.getPoliciesByResourceSignature(hash)).thenReturn(policies); - assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.CREATE)); + when(_store.getPoliciesByResourceSignature("service-name", hash, true)).thenReturn(policies); + when(_policy.getIsEnabled()).thenReturn(true); // ensure policy is enabled + _failures.clear(); assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.CREATE)); _utils.checkFailureForSemanticError(_failures, "resources"); + // same check should pass if the policy is disabled + when(_policy.getIsEnabled()).thenReturn(false); + _failures.clear(); assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, Action.CREATE)); + assertTrue("failures collection wasn't empty!", _failures.isEmpty()); + // For Update match with itself is not a problem as long as it isn't itself, i.e. same id. + when(_policy.getIsEnabled()).thenReturn(true); // ensure policy is enabled when(policy1.getId()).thenReturn(103L); when(_policy.getId()).thenReturn(103L); assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE)); + // matching policy can't be some other policy (i.e. different id) because that implies a conflict. when(policy1.getId()).thenReturn(104L); assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE)); _utils.checkFailureForSemanticError(_failures, "resources"); + // same check should pass if the policy is disabled + when(_policy.getIsEnabled()).thenReturn(false); + _failures.clear(); assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE)); + assertTrue("failures collection wasn't empty!", _failures.isEmpty()); + // And validation should never pass if there are more than one policies with matching signature, regardless of their ID!! RangerPolicy policy2 = mock(RangerPolicy.class); when(policy2.getId()).thenReturn(103L); // has same id as the policy being tested (_policy) policies.add(policy2); + when(_policy.getIsEnabled()).thenReturn(true); // ensure policy is enabled assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE)); _utils.checkFailureForSemanticError(_failures, "resources"); + // same check should pass if the policy is disabled + when(_policy.getIsEnabled()).thenReturn(false); + _failures.clear(); assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE)); + assertTrue("failures collection wasn't empty!", _failures.isEmpty()); } @Test http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/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 01c0e6d..e180263 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 @@ -199,10 +199,12 @@ public class TestRangerValidator { public final void test_getPoliciesForResourceSignature() throws Exception { // return null if store returns null or throws an exception String hexSignature = "aSignature"; - when(_store.getPoliciesByResourceSignature(hexSignature)).thenReturn(null); - assertNull(_validator.getPoliciesForResourceSignature(hexSignature)); - when(_store.getPoliciesByResourceSignature(hexSignature)).thenThrow(new Exception()); - assertNull(_validator.getPoliciesForResourceSignature(hexSignature)); + String serviceName = "service-name"; + boolean isPolicyEnabled = true; + when(_store.getPoliciesByResourceSignature(serviceName, hexSignature, isPolicyEnabled)).thenReturn(null); + assertNull(_validator.getPoliciesForResourceSignature(serviceName, hexSignature)); + when(_store.getPoliciesByResourceSignature(serviceName, hexSignature, isPolicyEnabled)).thenThrow(new Exception()); + assertNull(_validator.getPoliciesForResourceSignature(serviceName, hexSignature)); // what ever store returns should come back hexSignature = "anotherSignature"; @@ -211,8 +213,8 @@ public class TestRangerValidator { policies.add(policy1); RangerPolicy policy2 = mock(RangerPolicy.class); policies.add(policy2); - when(_store.getPoliciesByResourceSignature(hexSignature)).thenReturn(policies); - List<RangerPolicy> result = _validator.getPoliciesForResourceSignature(hexSignature); + when(_store.getPoliciesByResourceSignature(serviceName, hexSignature, isPolicyEnabled)).thenReturn(policies); + List<RangerPolicy> result = _validator.getPoliciesForResourceSignature(serviceName, hexSignature); assertTrue(result.contains(policy1) && result.contains(policy2)); } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/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 e6bbc15..7f83562 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 @@ -28,7 +28,6 @@ import java.util.Map; import java.util.Map.Entry; import javax.annotation.PostConstruct; -import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; @@ -116,7 +115,6 @@ import org.apache.ranger.service.XUserService; import org.apache.ranger.view.RangerPolicyList; import org.apache.ranger.view.RangerServiceDefList; import org.apache.ranger.view.RangerServiceList; -import org.apache.ranger.view.VXResponse; import org.apache.ranger.view.VXString; import org.apache.ranger.view.VXUser; import org.springframework.beans.factory.annotation.Autowired; @@ -1211,8 +1209,9 @@ public class ServiceDBStore extends AbstractServiceStore { } @Override - public List<RangerPolicy> getPoliciesByResourceSignature(String hexSignature) throws Exception { - List<XXPolicy> xxPolicies = daoMgr.getXXPolicy().findByResourceSignature(hexSignature); + public List<RangerPolicy> getPoliciesByResourceSignature(String serviceName, String policySignature, Boolean isPolicyEnabled) throws Exception { + + List<XXPolicy> xxPolicies = daoMgr.getXXPolicy().findByResourceSignatureByPolicyStatus(serviceName, policySignature, isPolicyEnabled); List<RangerPolicy> policies = new ArrayList<RangerPolicy>(xxPolicies.size()); for (XXPolicy xxPolicy : xxPolicies) { RangerPolicy policy = policyService.getPopulatedViewObject(xxPolicy); @@ -1299,9 +1298,7 @@ public class ServiceDBStore extends AbstractServiceStore { List<RangerPolicyItem> policyItems = policy.getPolicyItems(); policy.setVersion(new Long(1)); - RangerPolicyResourceSignature signature = factory.createPolicyResourceSignature(policy); - String hexSignature = signature.getSignature(); - policy.setResourceSignature(hexSignature); + updatePolicySignature(policy); if(populateExistingBaseFields) { assignedIdPolicyService.setPopulateExistingBaseFields(true); @@ -1376,6 +1373,7 @@ public class ServiceDBStore extends AbstractServiceStore { } policy.setVersion(version); + updatePolicySignature(policy); policy = policyService.update(policy); XXPolicy newUpdPolicy = daoMgr.getXXPolicy().getById(policy.getId()); @@ -1987,4 +1985,13 @@ public class ServiceDBStore extends AbstractServiceStore { return policy; } + void updatePolicySignature(RangerPolicy policy) { + RangerPolicyResourceSignature policySignature = factory.createPolicyResourceSignature(policy); + String signature = policySignature.getSignature(); + policy.setResourceSignature(signature); + if (LOG.isDebugEnabled()) { + String message = String.format("Setting signature on policy id=%d, name=%s to [%s]", policy.getId(), policy.getName(), signature); + LOG.debug(message); + } + } } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java ---------------------------------------------------------------------- diff --git a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java index eb7c2aa..9901832 100644 --- a/security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java +++ b/security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java @@ -72,13 +72,30 @@ public class XXPolicyDao extends BaseDao<XXPolicy> { } } - public List<XXPolicy> findByResourceSignature(String resSignature) { - if (resSignature == null) { + public List<XXPolicy> findByResourceSignatureByPolicyStatus(String serviceName, String policySignature, Boolean isPolicyEnabled) { + if (policySignature == null || serviceName == null || isPolicyEnabled == null) { + return new ArrayList<XXPolicy>(); + } + try { + return getEntityManager().createNamedQuery("XXPolicy.findByResourceSignatureByPolicyStatus", tClass) + .setParameter("resSignature", policySignature) + .setParameter("serviceName", serviceName) + .setParameter("isPolicyEnabled", isPolicyEnabled) + .getResultList(); + } catch (NoResultException e) { + return new ArrayList<XXPolicy>(); + } + } + + public List<XXPolicy> findByResourceSignature(String serviceName, String policySignature) { + if (policySignature == null || serviceName == null) { return new ArrayList<XXPolicy>(); } try { return getEntityManager().createNamedQuery("XXPolicy.findByResourceSignature", tClass) - .setParameter("resSignature", resSignature).getResultList(); + .setParameter("resSignature", policySignature) + .setParameter("serviceName", serviceName) + .getResultList(); } catch (NoResultException e) { return new ArrayList<XXPolicy>(); } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/f991a8b6/security-admin/src/main/resources/META-INF/jpa_named_queries.xml ---------------------------------------------------------------------- diff --git a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml index 737675d..054a0bd 100644 --- a/security-admin/src/main/resources/META-INF/jpa_named_queries.xml +++ b/security-admin/src/main/resources/META-INF/jpa_named_queries.xml @@ -206,8 +206,15 @@ <query>select MAX(obj.id) from XXPolicy obj</query> </named-query> + <named-query name="XXPolicy.findByResourceSignatureByPolicyStatus"> + <query>select obj from XXPolicy obj, XXService xSvc where obj.service = xSvc.id + and obj.resourceSignature = :resSignature and xSvc.name = :serviceName and obj.isEnabled = :isPolicyEnabled</query> + </named-query> + + <!-- currently not exposed but should be when we have a use case for it --> <named-query name="XXPolicy.findByResourceSignature"> - <query>select obj from XXPolicy obj where obj.resourceSignature = :resSignature</query> + <query>select obj from XXPolicy obj, XXService xSvc where obj.service = xSvc.id + and obj.resourceSignature = :resSignature and xSvc.name = :serviceName</query> </named-query> <named-query name="XXPolicy.findByServiceDefId">
