You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by ab...@apache.org on 2018/10/30 22:36:35 UTC
ranger git commit: RANGER-2272: Ensure that case of
resource-definition names and access-type names in Ranger policy is the same
as in service-definition after successful validation
Repository: ranger
Updated Branches:
refs/heads/master 1c32f55a0 -> ae781cc90
RANGER-2272: Ensure that case of resource-definition names and access-type names in Ranger policy is the same as in service-definition after successful validation
Project: http://git-wip-us.apache.org/repos/asf/ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/ranger/commit/ae781cc9
Tree: http://git-wip-us.apache.org/repos/asf/ranger/tree/ae781cc9
Diff: http://git-wip-us.apache.org/repos/asf/ranger/diff/ae781cc9
Branch: refs/heads/master
Commit: ae781cc90db322af48cbb88964e73e07df1fa357
Parents: 1c32f55
Author: Abhay Kulkarni <ak...@hortonworks.com>
Authored: Tue Oct 30 15:36:23 2018 -0700
Committer: Abhay Kulkarni <ak...@hortonworks.com>
Committed: Tue Oct 30 15:36:23 2018 -0700
----------------------------------------------------------------------
.../model/validation/RangerPolicyValidator.java | 37 ++++++++++++++------
.../validation/RangerServiceDefValidator.java | 2 +-
.../model/validation/RangerValidator.java | 26 ++++++++------
.../validation/TestRangerPolicyValidator.java | 4 +--
.../model/validation/TestRangerValidator.java | 37 ++------------------
.../org/apache/ranger/biz/ServiceDBStore.java | 17 ++++++++-
6 files changed, 63 insertions(+), 60 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ranger/blob/ae781cc9/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
index 9de860d..ddedf3e 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
@@ -500,7 +500,8 @@ public class RangerPolicyValidator extends RangerValidator {
}
boolean valid = true;
- Set<String> policyResources = getPolicyResources(policy);
+ convertPolicyResourceNamesToLower(policy);
+ Set<String> policyResources = policy.getResources().keySet();
RangerServiceDefHelper defHelper = new RangerServiceDefHelper(serviceDef);
Set<List<RangerResourceDef>> hierarchies = defHelper.getResourceHierarchies(policy.getPolicyType()); // this can be empty but not null!
@@ -935,15 +936,20 @@ public class RangerPolicyValidator extends RangerValidator {
.errorCode(error.getErrorCode())
.build());
valid = false;
- } else if (!accessTypes.contains(accessType.toLowerCase())) {
- ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID;
- failures.add(new ValidationFailureDetailsBuilder()
- .field("policy item access type")
- .isSemanticallyIncorrect()
- .becauseOf(error.getMessage(accessType, accessTypes))
- .errorCode(error.getErrorCode())
- .build());
- valid = false;
+ } else {
+ String matchedAccessType = getMatchedAccessType(accessType, accessTypes);
+ if (StringUtils.isEmpty(matchedAccessType)) {
+ ValidationErrorCode error = ValidationErrorCode.POLICY_VALIDATION_ERR_POLICY_ITEM_ACCESS_TYPE_INVALID;
+ failures.add(new ValidationFailureDetailsBuilder()
+ .field("policy item access type")
+ .isSemanticallyIncorrect()
+ .becauseOf(error.getMessage(accessType, accessTypes))
+ .errorCode(error.getErrorCode())
+ .build());
+ valid = false;
+ } else {
+ access.setType(matchedAccessType);
+ }
}
Boolean isAllowed = access.getIsAllowed();
// it can be null (which is treated as allowed) but not false
@@ -964,4 +970,15 @@ public class RangerPolicyValidator extends RangerValidator {
}
return valid;
}
+
+ String getMatchedAccessType(String accessType, Set<String> validAccessTypes) {
+ String ret = null;
+ for (String validType : validAccessTypes) {
+ if (StringUtils.equalsIgnoreCase(accessType, validType)) {
+ ret = validType;
+ break;
+ }
+ }
+ return ret;
+ }
}
http://git-wip-us.apache.org/repos/asf/ranger/blob/ae781cc9/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
index 6a1b3e1..f96fcfc 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefValidator.java
@@ -495,7 +495,7 @@ public class RangerServiceDefValidator extends RangerValidator {
return valid;
}
- boolean isValidResources(RangerServiceDef serviceDef, List<ValidationFailureDetails> failures, final Action action) {
+ public boolean isValidResources(RangerServiceDef serviceDef, List<ValidationFailureDetails> failures, final Action action) {
if(LOG.isDebugEnabled()) {
LOG.debug(String.format("==> RangerServiceDefValidator.isValidResources(%s, %s)", serviceDef, failures));
}
http://git-wip-us.apache.org/repos/asf/ranger/blob/ae781cc9/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java
index c7062dd..51324b0 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerValidator.java
@@ -303,7 +303,7 @@ public abstract class RangerValidator {
if (StringUtils.isBlank(accessType)) {
LOG.warn("Access type def name was null/empty/blank!");
} else {
- accessTypes.add(accessType.toLowerCase());
+ accessTypes.add(accessType);
}
}
}
@@ -409,22 +409,22 @@ public abstract class RangerValidator {
}
return resourceNames;
}
-
+
/**
- * Returns the resource-types defined on the policy converted to lowe-case
+ * Converts, in place, the resources defined in the policy to have lower-case resource-def-names
* @param policy
* @return
*/
- Set<String> getPolicyResources(RangerPolicy policy) {
- if (policy == null || policy.getResources() == null || policy.getResources().isEmpty()) {
- return new HashSet<>();
- } else {
- Set<String> result = new HashSet<>();
- for (String name : policy.getResources().keySet()) {
- result.add(name.toLowerCase());
+
+ void convertPolicyResourceNamesToLower(RangerPolicy policy) {
+ Map<String, RangerPolicyResource> lowerCasePolicyResources = new HashMap<>();
+ if (policy.getResources() != null) {
+ for (Map.Entry<String, RangerPolicyResource> entry : policy.getResources().entrySet()) {
+ String lowerCasekey = entry.getKey().toLowerCase();
+ lowerCasePolicyResources.put(lowerCasekey, entry.getValue());
}
- return result;
}
+ policy.setResources(lowerCasePolicyResources);
}
Map<String, String> getValidationRegExes(RangerServiceDef serviceDef) {
@@ -582,6 +582,10 @@ public abstract class RangerValidator {
return valid;
}
+ /*
+ * Important: Resource-names are required to be lowercase. This is used in validating policy create/update operations.
+ * Ref: RANGER-2272
+ */
boolean isValidResourceName(final String value, final String valueContext, final List<ValidationFailureDetails> failures) {
boolean ret = true;
http://git-wip-us.apache.org/repos/asf/ranger/blob/ae781cc9/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
index 140a9ed..8cdb9c3 100644
--- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
+++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
@@ -123,13 +123,13 @@ public class TestRangerPolicyValidator {
private final Object[][] policyResourceMap_good = new Object[][] {
// resource-name, values, excludes, recursive
{ "db", new String[] { "db1", "db2" }, null, null },
- { "TBL", new String[] { "tbl1", "tbl2" }, true, false } // case should not matter
+ { "tbl", new String[] { "tbl1", "tbl2" }, true, false } // case matters - use only lowercase characters
};
private final Object[][] policyResourceMap_goodMultipleHierarchies = new Object[][] {
// resource-name, values, excludes, recursive
{ "db", new String[] { "db1", "db2" }, null, null },
- { "UDF", new String[] { "udf1", "udf2" }, true, false } // case should not matter
+ { "udf", new String[] { "udf1", "udf2" }, true, false } // case matters - use only lowercase characters
};
private final Object[][] policyResourceMap_bad = new Object[][] {
http://git-wip-us.apache.org/repos/asf/ranger/blob/ae781cc9/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java
index 5bdffda..6de590b 100644
--- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java
+++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java
@@ -33,7 +33,6 @@ import java.util.Map;
import java.util.Set;
import org.apache.ranger.plugin.model.RangerPolicy;
-import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource;
import org.apache.ranger.plugin.model.RangerService;
import org.apache.ranger.plugin.model.RangerServiceDef;
import org.apache.ranger.plugin.model.RangerServiceDef.RangerAccessTypeDef;
@@ -46,8 +45,6 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
-import com.google.common.collect.Maps;
-
public class TestRangerValidator {
static class RangerValidatorForTest extends RangerValidator {
@@ -264,8 +261,8 @@ public class TestRangerValidator {
Assert.assertEquals(4, accessTypes.size());
Assert.assertTrue(accessTypes.contains("a"));
Assert.assertTrue(accessTypes.contains("b "));
- Assert.assertTrue(accessTypes.contains(" c"));
- Assert.assertTrue(accessTypes.contains(" d "));
+ Assert.assertTrue(accessTypes.contains(" C"));
+ Assert.assertTrue(accessTypes.contains(" D "));
}
@Test
@@ -353,36 +350,6 @@ public class TestRangerValidator {
}
@Test
- public void test_getPolicyResources() {
-
- Set<String> result;
- RangerPolicy policy = null;
- // null policy
- result = _validator.getPolicyResources(null);
- Assert.assertTrue(result != null);
- Assert.assertTrue(result.isEmpty());
- // null resource map
- policy = mock(RangerPolicy.class);
- when(policy.getResources()).thenReturn(null);
- result = _validator.getPolicyResources(null);
- Assert.assertTrue(result != null);
- Assert.assertTrue(result.isEmpty());
- // empty resource map
- Map<String, RangerPolicyResource> input = Maps.newHashMap();
- when(policy.getResources()).thenReturn(input);
- result = _validator.getPolicyResources(policy);
- Assert.assertTrue(result != null);
- Assert.assertTrue(result.isEmpty());
- // known resource map
- input.put("r1", mock(RangerPolicyResource.class));
- input.put("R2", mock(RangerPolicyResource.class));
- result = _validator.getPolicyResources(policy);
- Assert.assertEquals(2, result.size());
- Assert.assertTrue("r1", result.contains("r1"));
- Assert.assertTrue("R2", result.contains("r2")); // result should lowercase the resource-names
- }
-
- @Test
public void test_getIsAuditEnabled() {
// null policy
RangerPolicy policy = null;
http://git-wip-us.apache.org/repos/asf/ranger/blob/ae781cc9/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 9ee5fae..b40d4f0 100644
--- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
+++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java
@@ -65,6 +65,9 @@ import org.apache.ranger.common.MessageEnums;
import org.apache.ranger.common.RangerCommonEnums;
import org.apache.ranger.common.db.RangerTransactionSynchronizationAdapter;
import org.apache.ranger.entity.*;
+import org.apache.ranger.plugin.model.validation.RangerServiceDefValidator;
+import org.apache.ranger.plugin.model.validation.RangerValidator;
+import org.apache.ranger.plugin.model.validation.ValidationFailureDetails;
import org.apache.ranger.plugin.policyengine.RangerPolicyEngine;
import org.apache.ranger.plugin.policyresourcematcher.RangerDefaultPolicyResourceMatcher;
import org.apache.ranger.plugin.policyresourcematcher.RangerPolicyResourceMatcher;
@@ -351,9 +354,21 @@ public class ServiceDBStore extends AbstractServiceStore {
+ serviceDef.getName() + " already exists",
MessageEnums.ERROR_DUPLICATE_OBJECT);
}
-
+
List<RangerServiceConfigDef> configs = serviceDef.getConfigs();
List<RangerResourceDef> resources = serviceDef.getResources();
+
+ if (CollectionUtils.isNotEmpty(resources)) {
+ RangerServiceDefValidator validator = new RangerServiceDefValidator(this);
+ List<ValidationFailureDetails> failures = new ArrayList<>();
+ boolean isValidResources = validator.isValidResources(serviceDef, failures, RangerValidator.Action.CREATE);
+ if (!isValidResources) {
+ throw restErrorUtil.createRESTException("service-def with name: "
+ + serviceDef.getName() + " has invalid resources:[" + failures.toString() + "]",
+ MessageEnums.INVALID_INPUT_DATA);
+ }
+ }
+
List<RangerAccessTypeDef> accessTypes = serviceDef.getAccessTypes();
List<RangerPolicyConditionDef> policyConditions = serviceDef.getPolicyConditions();
List<RangerContextEnricherDef> contextEnrichers = serviceDef.getContextEnrichers();