RANGER-4403: security-zone validation updated to prevent duplicate re…

…source entries
apache · Sep 13, 2023 · 7f86e62 · 7f86e62
1 parent f3e8e0a
commit 7f86e62
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 0 deletions.
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
@@ -124,6 +124,7 @@ public enum ValidationErrorCode {
     SECURITY_ZONE_VALIDATION_ERR_INTERNAL_ERROR(3045, "Internal Error:[{0}]"),
     SECURITY_ZONE_VALIDATION_ERR_ZONE_RESOURCE_CONFLICT(3046, "Multiple zones:[{0}] match resource:[{1}]"),
     SECURITY_ZONE_VALIDATION_ERR_UNEXPECTED_RESOURCES(3047, "Tag service [{0}], with non-empty resources, is associated with security zone"),
+    SECURITY_ZONE_VALIDATION_ERR_DUPLICATE_RESOURCE_ENTRY(3052, "Resource [{0}] specified more than once in service [{1}]"),
 
     //RANGER ROLE Validations
     ROLE_VALIDATION_ERR_NULL_RANGER_ROLE_OBJECT(4001, "Internal error: RangerRole object passed in was null"),

diff --git a/...ts-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java b/...ts-common/src/main/java/org/apache/ranger/plugin/model/RangerPolicyResourceSignature.java
@@ -57,6 +57,11 @@ public RangerPolicyResourceSignature(RangerPolicy policy) {
 		}
 	}
 
+	public RangerPolicyResourceSignature(Map<String, List<String>> resources) {
+		this(toSignatureString(toPolicyResources(resources)));
+	}
+
+
 	/**
 	 * Only added for testability.  Do not make public
 	 * @param string
@@ -204,6 +209,16 @@ public static String toSignatureString(Map<String, RangerPolicyResource> resourc
 		return ret;
 	}
 
+	private static Map<String, RangerPolicyResource> toPolicyResources(Map<String, List<String>> resources) {
+		Map<String, RangerPolicyResource> ret = new TreeMap<>();
+
+		for (Map.Entry<String, List<String>> entry : resources.entrySet()) {
+			ret.put(entry.getKey(), new RangerPolicyResource(entry.getValue(), false, false));
+		}
+
+		return ret;
+	}
+
 	static public class ResourceSerializer {
 		final RangerPolicyResource _policyResource;
 

diff --git a/.../src/main/java/org/apache/ranger/plugin/model/validation/RangerSecurityZoneValidator.java b/.../src/main/java/org/apache/ranger/plugin/model/validation/RangerSecurityZoneValidator.java
@@ -24,6 +24,7 @@
 import org.apache.commons.lang.StringUtils;
 import org.apache.ranger.plugin.errors.ValidationErrorCode;
 import org.apache.ranger.plugin.model.RangerPolicy.RangerPolicyResource;
+import org.apache.ranger.plugin.model.RangerPolicyResourceSignature;
 import org.apache.ranger.plugin.model.RangerSecurityZone;
 import org.apache.ranger.plugin.model.RangerService;
 import org.apache.ranger.plugin.model.RangerServiceDef;
@@ -460,6 +461,8 @@ private boolean validateSecurityZoneService(String serviceName, RangerSecurityZo
             } else {
                 if (CollectionUtils.isNotEmpty(securityZoneService.getResources())) {
                     // For each resource-spec, verify that it forms valid hierarchy for some policy-type
+                    Set<String> resourceSignatures = new HashSet<>();
+
                     for (Map<String, List<String>> resource : securityZoneService.getResources()) {
                         Set<String>            resourceDefNames = resource.keySet();
                         RangerServiceDefHelper serviceDefHelper = new RangerServiceDefHelper(serviceDef);
@@ -503,6 +506,18 @@ private boolean validateSecurityZoneService(String serviceName, RangerSecurityZo
                                 ret = false;
                             }
                         }
+
+                        RangerPolicyResourceSignature resourceSignature = new RangerPolicyResourceSignature(resource);
+
+                        if (!resourceSignatures.add(resourceSignature.getSignature())) {
+                            ValidationErrorCode error = ValidationErrorCode.SECURITY_ZONE_VALIDATION_ERR_DUPLICATE_RESOURCE_ENTRY;
+
+                            failures.add(new ValidationFailureDetailsBuilder().field("security zone resources")
+                                                 .subField("resources")
+                                                 .becauseOf(error.getMessage(resource, serviceName))
+                                                 .errorCode(error.getErrorCode()).build());
+                            ret = false;
+                        }
                     }
                 }
             }

diff --git a/.../test/java/org/apache/ranger/plugin/model/validation/RangerSecurityZoneValidatorTest.java b/.../test/java/org/apache/ranger/plugin/model/validation/RangerSecurityZoneValidatorTest.java
@@ -481,6 +481,39 @@ public void test2ValidateHiveResourceInMultipleSecurityZones() throws Exception
 		}
 	}
 
+	@Test
+	public void testValidateDuplicateResourceEntries() throws Exception {
+		List<HashMap<String, List<String>>> zone1Resources = new ArrayList<>();
+
+		zone1Resources.add(new HashMap<String, List<String>>() {{ put("database", Arrays.asList("db1")); put("table", Arrays.asList("tbl1")); }});
+		zone1Resources.add(new HashMap<String, List<String>>() {{ put("database", Arrays.asList("db1")); put("table", Arrays.asList("tbl1")); }});
+
+		RangerServiceDef          svcDef       = getHiveServiceDef();
+		RangerService             svc          = getHiveService();
+		RangerSecurityZoneService zone1HiveSvc = new RangerSecurityZoneService(zone1Resources);
+
+		RangerSecurityZone zone1 = new RangerSecurityZone("zone1", Collections.singletonMap(svc.getName(), zone1HiveSvc), null, Arrays.asList("admin"), null, Arrays.asList("auditor"), null, "Zone 1");
+
+		zone1.setId(1L);
+
+		List<RangerSecurityZone> zones = new ArrayList<RangerSecurityZone>() {{ add(zone1); }};
+
+		Mockito.when(_store.getServiceByName(svc.getName())).thenReturn(svc);
+		Mockito.when(_store.getServiceDefByName(svc.getType())).thenReturn(svcDef);
+		Mockito.when(_store.getSecurityZone(zone1.getId())).thenReturn(zone1);
+
+		try {
+			rangerSecurityZoneValidator.validate(zone1, RangerValidator.Action.UPDATE);
+
+			Assert.assertFalse("security-zone update should have failed in validation", true);
+		} catch (Exception excp) {
+			String  failureMessage           = excp.getMessage();
+			boolean hasResourceConflictError = StringUtils.contains(failureMessage, ValidationErrorCode.SECURITY_ZONE_VALIDATION_ERR_DUPLICATE_RESOURCE_ENTRY.getErrorCode() + "");
+
+			Assert.assertTrue("validation failure message didn't include expected error code " + ValidationErrorCode.SECURITY_ZONE_VALIDATION_ERR_DUPLICATE_RESOURCE_ENTRY.getErrorCode() + ". Failure message: " + excp.getMessage(), hasResourceConflictError);
+		}
+	}
+
 	private RangerService getRangerService() {
 		Map<String, String> configs = new HashMap<String, String>();
 		configs.put("username", "servicemgr");