Repository: incubator-ranger Updated Branches: refs/heads/ranger-0.5 c829f26bd -> 18f216d02
RANGER-834 Correct the excludes flag's treatment when resource value denotes everything Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/18f216d0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/18f216d0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/18f216d0 Branch: refs/heads/ranger-0.5 Commit: 18f216d0201eab93daea0b57035f7e6e3280bcfd Parents: c829f26 Author: Alok Lal <[email protected]> Authored: Sun Jan 31 17:39:14 2016 -0800 Committer: Alok Lal <[email protected]> Committed: Wed Feb 3 23:40:02 2016 -0800 ---------------------------------------------------------------------- .../RangerAbstractResourceMatcher.java | 28 ++++++++++ .../RangerDefaultResourceMatcher.java | 7 +-- .../RangerPathResourceMatcher.java | 7 +-- .../RangerAbstractResourceMatcherTest.java | 32 +++++++++++ .../RangerDefaultResourceMatcherTest.java | 58 ++++++++++++++++++++ 5 files changed, 124 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/18f216d0/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java b/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java index 9e547f1..213d308 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java @@ -222,4 +222,32 @@ public abstract class RangerAbstractResourceMatcher implements RangerResourceMat return sb; } + + /** + * Is resource asking to authorize all possible values at this level? + * @param resource + * @return + */ + boolean isAllValuesRequested(String resource) { + boolean result = StringUtils.isEmpty(resource) || WILDCARD_ASTERISK.equals(resource); + if (LOG.isDebugEnabled()) { + LOG.debug("isAllValuesRequested(" + resource + "): " + result); + } + return result; + } + + /** + * The only case where excludes flag does NOT change the result is the following: + * - Resource denotes all possible values (i.e. resource in (null, "", "*") + * - where as policy does not allow all possible values (i.e. policy.values().contains("*") + * + * @param allValuesRequested + * @param resultWithoutExcludes + * @return + */ + public boolean applyExcludes(boolean allValuesRequested, boolean resultWithoutExcludes) { + if (!policyIsExcludes) return resultWithoutExcludes; // not an excludes policy! + if (allValuesRequested && !isMatchAny) return resultWithoutExcludes; // one case where excludes has no effect + return !resultWithoutExcludes; // all other cases flip it + } } http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/18f216d0/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java b/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java index 79c3885..8efadbe 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java @@ -37,8 +37,9 @@ public class RangerDefaultResourceMatcher extends RangerAbstractResourceMatcher } boolean ret = false; + boolean allValuesRequested = isAllValuesRequested(resource); - if(resource == null || isMatchAny) { + if(allValuesRequested || isMatchAny) { ret = isMatchAny; } else { for(String policyValue : policyValues) { @@ -56,9 +57,7 @@ public class RangerDefaultResourceMatcher extends RangerAbstractResourceMatcher } } - if(policyIsExcludes) { - ret = !ret; - } + ret = applyExcludes(allValuesRequested, ret); if(LOG.isDebugEnabled()) { LOG.debug("<== RangerDefaultResourceMatcher.isMatch(" + resource + "): " + ret); http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/18f216d0/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java b/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java index 79ab394..5c555eb 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java @@ -78,8 +78,9 @@ public class RangerPathResourceMatcher extends RangerAbstractResourceMatcher { } boolean ret = false; + boolean allValuesRequested = isAllValuesRequested(resource); - if(resource == null || isMatchAny) { + if(allValuesRequested || isMatchAny) { ret = isMatchAny; } else { IOCase caseSensitivity = optIgnoreCase ? IOCase.INSENSITIVE : IOCase.SENSITIVE; @@ -103,9 +104,7 @@ public class RangerPathResourceMatcher extends RangerAbstractResourceMatcher { } } - if(policyIsExcludes) { - ret = !ret; - } + ret = applyExcludes(allValuesRequested, ret); if(LOG.isDebugEnabled()) { LOG.debug("<== RangerPathResourceMatcher.isMatch(" + resource + "): " + ret); http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/18f216d0/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcherTest.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcherTest.java b/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcherTest.java new file mode 100644 index 0000000..7f0915f --- /dev/null +++ b/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcherTest.java @@ -0,0 +1,32 @@ +package org.apache.ranger.plugin.resourcematcher; + +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Created by alal on 1/29/16. + */ +public class RangerAbstractResourceMatcherTest { + + @Test + public void test_isAllPossibleValues() { + RangerAbstractResourceMatcher matcher = new AbstractMatcherWrapper(); + for (String resource : new String[] { null, "", "*"}) { + assertTrue(matcher.isAllValuesRequested(resource)); + } + + for (String resource : new String[] { " ", "\t", "\n", "foo"}) { + assertFalse(matcher.isAllValuesRequested(resource)); + } + } + + static class AbstractMatcherWrapper extends RangerAbstractResourceMatcher { + + @Override + public boolean isMatch(String resource) { + fail("This method is not expected to be used by test!"); + return false; + } + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/18f216d0/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcherTest.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcherTest.java b/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcherTest.java new file mode 100644 index 0000000..94b7b82 --- /dev/null +++ b/agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcherTest.java @@ -0,0 +1,58 @@ +package org.apache.ranger.plugin.resourcematcher; + +import com.google.common.collect.Lists; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Created by alal on 2/1/16. + */ +public class RangerDefaultResourceMatcherTest { + + Object[][] data = { + // { resource, policy, excludes, result + { "*", "*", false, true }, // resource is all values + { "*", "*", true, false }, + { "*", "a*", false, false }, // but, policy is not match any + { "*", "a*", true, false }, // ==> compare with above: exclude flag has no effect here + { "a*", "a", false, false }, // resource has regex marker! + { "a*", "a", true, true }, + { "a", "a", false, true }, // exact match + { "a", "a", true, false }, + { "a1", "a*", false, true }, // trivial regex match + { "a1", "a*", true, false }, + }; + + @Test + public void testIsMatch() throws Exception { + for (Object[] row : data) { + String resource = (String)row[0]; + String policyValue = (String)row[1]; + boolean excludes = (boolean)row[2]; + boolean result = (boolean)row[3]; + + MatcherWrapper matcher = new MatcherWrapper(policyValue, excludes); + assertEquals(getMessage(row), result, matcher.isMatch(resource)); + } + } + + String getMessage(Object[] row) { + return String.format("Resource=%s, Policy=%s, excludes=%s, result=%s", + (String)row[0], (String)row[1], (boolean)row[2], (boolean)row[3]); + } + + static class MatcherWrapper extends RangerDefaultResourceMatcher { + MatcherWrapper(String policyValue, boolean exclude) { + this.policyValues = Lists.newArrayList(policyValue); + if (WILDCARD_ASTERISK.equals(policyValue)) { + this.isMatchAny = true; + } + if (policyValue.contains(WILDCARD_ASTERISK)) { + this.optWildCard = true; + } + this.policyIsExcludes = exclude; + } + } + +} \ No newline at end of file
