ranger-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ab...@apache.org
Subject 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
Date Tue, 30 Oct 2018 22:36:35 GMT
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 <akulkarni@hortonworks.com>
Authored: Tue Oct 30 15:36:23 2018 -0700
Committer: Abhay Kulkarni <akulkarni@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();


Mime
View raw message