RANGER-1603: Code improvement as recommended by good coding practices
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 864709c..26b9866 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
@@ -335,14 +335,16 @@
}
}
- if (needWildcardMatch) {
+ if (needWildcardMatch) { // test?, test*a*, test*a*b, *test*a
ret = optIgnoreCase ? new CaseInsensitiveWildcardMatcher(policyValue) : new CaseSensitiveWildcardMatcher(policyValue);
- } else if (wildcardStartIdx == -1) {
+ } else if (wildcardStartIdx == -1) { // test, testa, testab
ret = optIgnoreCase ? new CaseInsensitiveStringMatcher(policyValue) : new CaseSensitiveStringMatcher(policyValue);
- } else if (wildcardStartIdx == 0) {
+ } else if (wildcardStartIdx == 0) { // *test, **test, *testa, *testab
String matchStr = policyValue.substring(wildcardEndIdx + 1);
ret = optIgnoreCase ? new CaseInsensitiveEndsWithMatcher(matchStr) : new CaseSensitiveEndsWithMatcher(matchStr);
- } else {
+ } else if (wildcardEndIdx != (len - 1)) { // test*a, test*ab
+ ret = optIgnoreCase ? new CaseInsensitiveWildcardMatcher(policyValue) : new CaseSensitiveWildcardMatcher(policyValue);
+ } else { // test*, test**, testa*, testab*
String matchStr = policyValue.substring(0, wildcardStartIdx);
ret = optIgnoreCase ? new CaseInsensitiveStartsWithMatcher(matchStr) : new CaseSensitiveStartsWithMatcher(matchStr);
}
diff --git a/agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json b/agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json
index 86da28c..97765f9 100644
--- a/agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json
+++ b/agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json
@@ -1,6 +1,277 @@
{
"testCases":[
{
+ "name": "1a:value=/test?; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/test?"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/testa", "result": true},
+ {"name": "incomplete-path", "input": "/test", "result": false},
+ {"name": "extra-path", "input": "/testab", "result": false}
+ ]
+ },
+ {
+ "name": "1b:value=/test*a*; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/test*a*"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/testa", "result": true},
+ {"name": "expanded-path-1", "input": "/test1a", "result": true},
+ {"name": "expanded-path-2", "input": "/test1a2", "result": true},
+ {"name": "incorrect-path", "input": "/tes1a2", "result": false}
+ ]
+ },
+ {
+ "name": "1c:value=/test*a*b; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/test*a*b"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/testab", "result": true},
+ {"name": "expanded-path-1", "input": "/test1ab", "result": true},
+ {"name": "expanded-path-2", "input": "/test1a2b", "result": true},
+ {"name": "incorrect-path", "input": "/tesa12b", "result": false}
+ ]
+ },
+ {
+ "name": "1d:value=/*test*a; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/*test*a"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/testa", "result": true},
+ {"name": "expanded-path-1", "input": "/1test2a", "result": true},
+ {"name": "expanded-path-2", "input": "/01test23a", "result": true},
+ {"name": "incorrect-path", "input": "/tesa", "result": false}
+ ]
+ },
+ {
+ "name": "2:value=/test; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/test"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/test", "result": true},
+ {"name": "incorrect-path-1", "input": "/testa", "result": false},
+ {"name": "incorrect-path-2", "input": "/1test", "result": false}
+ ]
+ },
+ {
+ "name": "3:value=/*test; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/*test", "**test"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/test", "result": true},
+ {"name": "expanded-path-1", "input": "/1test", "result": true},
+ {"name": "expanded-path-2", "input": "/12test", "result": true},
+ {"name": "incorrect-path", "input": "/12testa", "result": false}
+ ]
+ },
+ {
+ "name": "4a:value=/test*a; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/test*a"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/testa", "result": true},
+ {"name": "expanded-path-1", "input": "/test1a", "result": true},
+ {"name": "expanded-path-2", "input": "/test12a", "result": true},
+ {"name": "incorrect-path", "input": "/testb", "result": false}
+ ]
+ },
+ {
+ "name": "4b:value=/*test*a; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/*test*a"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/testa", "result": true},
+ {"name": "expanded-path-1", "input": "/test1a", "result": true},
+ {"name": "expanded-path-2", "input": "/test12a", "result": true},
+ {"name": "expanded-path-3", "input": "/0test12a", "result": true},
+ {"name": "incorrect-path", "input": "/0testb", "result": false}
+ ]
+ },
+ {
+ "name": "5:value=/test*; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/test*", "/test**"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "correct-path", "input": "/test", "result": true},
+ {"name": "expanded-path-1", "input": "/test1", "result": true},
+ {"name": "expanded-path-2", "input": "/test12", "result": true},
+ {"name": "incorrect-path-1", "input": "/0test", "result": false},
+ {"name": "incorrect-path-2", "input": "/tes", "result": false}
+ ]
+ },
+ {
+ "name": "Case 0:value=/home/; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/home/", "/home"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "seemingly-correct-path", "input": "/home/", "result": true},
+ {"name": "without-slash-path", "input": "/home", "result": true},
+ {"name": "incorrect-path", "input": "/home/a.txt", "result": false}
+ ]
+ },
+ {
+ "name": "Case 0:value=/home/; isRecursive=true; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/home/"],
+ "isRecursive": true
+ },
+ "tests": [
+ {"name": "seemingly-correct-path", "input": "/home/", "result": false},
+ {"name": "correct-path", "input": "/home/a.txt", "result": true}
+ ]
+ },
+ {
+ "name": "Case 4:value=/home/*/a.txt; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/home/*/a.txt"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "incorrect-path", "input": "/home/1/b.txt", "result": false},
+ {"name": "missing-level-path", "input": "/home/a.txt", "result": false},
+ {"name": "one-level-path", "input": "/home/1/a.txt", "result": true},
+ {"name": "multi-level-path", "input": "/home/1/2/a.txt", "result": true},
+ {"name": "multi-level-path", "input": "/home/1/2/ba.txt", "result": false}
+ ]
+ }
+ ,
+ {
+ "name": "Case 7:value=/home/*a.txt; isRecursive=false; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/home/*a.txt"],
+ "isRecursive": false
+ },
+ "tests": [
+ {"name": "multi-level-path", "input": "/home/1/2/3a.txt", "result": true},
+ {"name": "incorrect-path", "input": "/homea.txt", "result": false},
+ {"name": "exact-path", "input": "/home/a.txt", "result": true}
+ ]
+ }
+ ,
+ {
+ "name": "Case 4:value=/home/*/a.txt; isRecursive=true; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/home/*/a.txt"],
+ "isRecursive": true
+ },
+ "tests": [
+ {"name": "missing-level-path", "input": "/home/a.txt", "result": false},
+ {"name": "correct-path", "input": "/home/1/a.txt", "result": true},
+ {"name": "incorrect-path", "input": "/home/1/b.txt", "result": false},
+ {"name": "multi-level-path", "input": "/home/1/2/a.txt", "result": true},
+ {"name": "multi-level-path", "input": "/home/1/2/ba.txt", "result": false}
+ ]
+ }
+ ,
+ {
+ "name": "Case 4:value=/home/*/*/a.txt; isRecursive=true; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {
+ "values": ["/home/*/*/a.txt"],
+ "isRecursive": true
+ },
+ "tests": [
+ {"name": "missing-levels-path", "input": "/home/a.txt", "result": false},
+ {"name": "missing-level-path", "input": "/home/1/a.txt", "result": false},
+ {"name": "incorrect-path", "input": "/home/1/b.txt", "result": false},
+ {"name": "correct-path", "input": "/home/1/2/a.txt", "result": true}
+ ]
+ }
+ ,
+ {
+ "name": "Case 3: value=/home/; isRecursive=true; wildCard=true; ignoreCase=true",
+ "resourceDef": {
+ "matcher": "org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
+ "matcherOptions": {"wildCard": true, "ignoreCase": true}
+ },
+ "policyResource": {"values": ["/home/"], "isRecursive": true},
+ "tests": [
+ {"name": "slash-at-end-path", "input": "/home/", "result": false},
+ {"name": "correct-path", "input": "/home/a.txt", "result": true},
+ {"name": "incomplete-path", "input": "/home", "result": false}
+ ]
+ }
+ ,
+ {
"name":"value=/a/b*y.txt; isRecursive=true; wildCard=true; ignoreCase=true",
"resourceDef":{
"matcher":"org.apache.ranger.plugin.resourcematcher.RangerPathResourceMatcher",
@@ -15,7 +286,7 @@
{ "name":"child-path","input":"/a/b/y.txt", "result":true},
{ "name":"grand-child-path","input":"/a/b1/b2/y.txt", "result":true},
{ "name":"descendant-child-path","input":"/a/b1/c1/d1/any.txt", "result":true},
- { "name":"mismatche-path","input":"/a/any.txt", "result":false},
+ { "name":"mismatche-path","input":"/a/any.txt", "result":false}
]
}
@@ -35,7 +306,7 @@
{ "name":"child-path","input":"/a/b/y.txt", "result":true},
{ "name":"grand-child-path","input":"/a/b1/b2/y.txt", "result":true},
{ "name":"descendant-child-path","input":"/a/b1/c1/d1/any.txt", "result":true},
- { "name":"mismatche-path","input":"/a/any.txt", "result":false},
+ { "name":"mismatche-path","input":"/a/any.txt", "result":false}
]
}
,
@@ -89,7 +360,7 @@
{ "name":"exact-path","input":"/", "result":true},
{ "name":"child-path","input":"/path1", "result":false},
{ "name":"grand-child-path","input":"/path1/path2", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -108,7 +379,7 @@
{ "name":"exact-path","input":"/path1", "result":true},
{ "name":"child-path","input":"/path1/path2", "result":false},
{ "name":"grand-child-path","input":"/path1/path2/path3", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -128,7 +399,7 @@
{ "name":"child-path","input":"/path1/path2", "result":true},
{ "name":"grand-child-path","input":"/path1/path2/path3", "result":true},
{ "name":"sibling-path","input":"/path2/path3/path4", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -146,7 +417,7 @@
{ "name":"exact-path","input":"/", "result":true},
{ "name":"child-path","input":"/path1", "result":true},
{ "name":"grand-child-path","input":"/path1/path2", "result":true},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -217,7 +488,7 @@
{ "name":"grand-child-path-camel-case","input":"/Path-To-Success/Is-Slow/And-Fun", "result":true},
{ "name":"unmatched-path","input":"/pat1ha", "result":false},
{ "name":"unmatched-child-path","input":"/pat1ha/path2b", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -242,7 +513,7 @@
{ "name":"grand-child-path-camel-case","input":"/Path1/Path2/Path3", "result":true},
{ "name":"unmatched-path","input":"/path1a", "result":false},
{ "name":"unmatched-child-path","input":"/path1a/path2b", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -266,7 +537,7 @@
{ "name":"grand-child-path","input":"/public/archive/2008/first-test", "result":true},
{ "name":"unmatched-path","input":"/pat1ha", "result":false},
{ "name":"unmatched-child-path","input":"/pat1ha/path2b", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -297,7 +568,7 @@
{ "name":"grand-child-path","input":"/public/last-test/best-result/details", "result":true},
{ "name":"unmatched-path","input":"/pat1ha", "result":false},
{ "name":"unmatched-child-path","input":"/pat1ha/path2b", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -328,7 +599,7 @@
{ "name":"grand-child-path","input":"/public/last-test/best-result/details", "result":true}, # TODO: should this be false since isRecursive=false?
{ "name":"unmatched-path","input":"/pat1ha", "result":false},
{ "name":"unmatched-child-path","input":"/pat1ha/path2b", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -346,7 +617,7 @@
{ "name":"exact-path","input":"root", "result":true},
{ "name":"child-path","input":"root.default", "result":false},
{ "name":"grand-child-path","input":"root.default.mycompany", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -368,7 +639,7 @@
{ "name":"child-path","input":"root.default.mycompany1.test", "result":true}, # TODO: should this be false since isRecursive=false
{ "name":"child-path","input":"root.default.mycompany1.dev", "result":true}, # TODO: should this be false since isRecursive=false
{ "name":"sibling-path","input":"root.default.othercompany1.dev", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -386,7 +657,7 @@
{ "name":"exact-path","input":"root", "result":true},
{ "name":"child-path","input":"root.default", "result":true},
{ "name":"grand-child-path","input":"root.default.mycompany", "result":true},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
,
@@ -408,8 +679,9 @@
{ "name":"child-path","input":"root.default.mycompany1.test", "result":true},
{ "name":"child-path","input":"root.default.mycompany1.dev", "result":true},
{ "name":"sibling-path","input":"root.default.othercompany1.dev", "result":false},
- { "name":"invalid-path","input":"path1", "result":false},
+ { "name":"invalid-path","input":"path1", "result":false}
]
}
+
]
}
