This is an automated email from the ASF dual-hosted git repository.

rmani pushed a commit to branch RANGER-5130
in repository https://gitbox.apache.org/repos/asf/ranger.git


The following commit(s) were added to refs/heads/RANGER-5130 by this push:
     new c9482c4800 RANGER-5130: DatSet policies fail to authorize when 
condition expression is present -- review comments fixed
c9482c4800 is described below

commit c9482c480001dfd30c1dcfdc50d38354426ef1e6
Author: Ramesh Mani <[email protected]>
AuthorDate: Mon Feb 10 21:58:34 2025 -0800

    RANGER-5130: DatSet policies fail to authorize when condition expression is 
present -- review comments fixed
---
 .../apache/ranger/plugin/model/RangerGrant.java    | 131 +++++++++++++++++----
 .../main/java/org/apache/ranger/rest/GdsREST.java  |  53 ++++-----
 .../java/org/apache/ranger/rest/TestGdsREST.java   |  20 +++-
 3 files changed, 150 insertions(+), 54 deletions(-)

diff --git 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGrant.java 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGrant.java
index 0a3e3dc215..37a1ff6cc9 100644
--- 
a/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGrant.java
+++ 
b/agents-common/src/main/java/org/apache/ranger/plugin/model/RangerGrant.java
@@ -23,6 +23,7 @@
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonInclude;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 
@@ -34,18 +35,16 @@ public class RangerGrant implements java.io.Serializable {
 
     private RangerPrincipal principal;
     private List<String>    accessTypes;
-    private List<String>    conditions;
-    private List<String>    validitySchedules;
+    private List<Condition> conditions;
 
     public RangerGrant() {
-        this(null, null, null, null);
+        this(null, null, null);
     }
 
-    public RangerGrant(RangerPrincipal principal, List<String> accessTypes, 
List<String> conditions, List<String> validitySchedules) {
-        this.principal         = principal;
-        this.accessTypes       = accessTypes;
-        this.conditions        = conditions;
-        this.validitySchedules = validitySchedules;
+    public RangerGrant(RangerPrincipal principal, List<String> accessTypes, 
List<Condition> conditions) {
+        setPrincipal(principal);
+        setAccessTypes(accessTypes);
+        setConditions(conditions);
     }
 
     public RangerPrincipal getPrincipal() {
@@ -61,28 +60,44 @@ public List<String> getAccessTypes() {
     }
 
     public void setAccessTypes(List<String> accessTypes) {
-        this.accessTypes = accessTypes;
+        if (this.accessTypes == null) {
+            this.accessTypes = new ArrayList<>();
+        }
+
+        if (this.accessTypes == accessTypes) {
+            return;
+        }
+
+        this.accessTypes.clear();
+
+        if (accessTypes != null) {
+            this.accessTypes.addAll(accessTypes);
+        }
     }
 
-    public List<String> getConditions() {
+    public List<Condition> getConditions() {
         return conditions;
     }
 
-    public void setConditions(List<String> conditions) {
-        this.conditions = conditions;
-    }
+    public void setConditions(List<Condition> conditions) {
+        if (this.conditions == null) {
+            this.conditions = new ArrayList<>();
+        }
 
-    public List<String> getValiditySchedules() {
-        return validitySchedules;
-    }
+        if (this.conditions == conditions) {
+            return;
+        }
+
+        this.conditions.clear();
 
-    public void setValiditySchedules(List<String> validitySchedules) {
-        this.validitySchedules = validitySchedules;
+        if (conditions != null) {
+            this.conditions.addAll(conditions);
+        }
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(principal, accessTypes, conditions, 
validitySchedules);
+        return Objects.hash(principal, accessTypes, conditions);
     }
 
     @Override
@@ -99,8 +114,7 @@ public boolean equals(Object obj) {
 
         return Objects.equals(principal, other.principal) &&
                 Objects.equals(accessTypes, other.accessTypes) &&
-                Objects.equals(conditions, other.conditions) &&
-                Objects.equals(validitySchedules, other.validitySchedules);
+                Objects.equals(conditions, other.conditions);
     }
 
     @Override
@@ -109,7 +123,78 @@ public String toString() {
                 "principal='" + principal.toString() +
                 ", accessTypes=" + accessTypes +
                 ", conditions=" + conditions +
-                ", validitySchedules=" + validitySchedules +
                 '}';
     }
-}
+
+    @JsonAutoDetect(getterVisibility = JsonAutoDetect.Visibility.NONE, 
setterVisibility = JsonAutoDetect.Visibility.NONE, fieldVisibility = 
JsonAutoDetect.Visibility.ANY)
+    @JsonInclude(JsonInclude.Include.NON_EMPTY)
+    @JsonIgnoreProperties(ignoreUnknown = true)
+    public static class Condition implements java.io.Serializable {
+        private static final long serialVersionUID = 1L;
+
+        private String       type;
+        private List<String> values;
+
+        public Condition() {
+            this(null, null);
+        }
+
+        public Condition(String type, List<String> values) {
+            setType(type);
+            setValues(values);
+        }
+
+        public String getType() {
+            return type;
+        }
+
+        public void setType(String type) {
+            this.type = type;
+        }
+
+        public List<String> getValues() {
+            return values;
+        }
+
+        public void setValues(List<String> values) {
+            if (this.values == null) {
+                this.values = new ArrayList<>();
+            }
+
+            if (this.values == values) {
+                return;
+            }
+
+            this.values.clear();
+
+            if (values != null) {
+                this.values.addAll(values);
+            }
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+            Condition that = (Condition) o;
+            return Objects.equals(type, that.type) && Objects.equals(values, 
that.values);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(type, values);
+        }
+
+        @Override
+        public String toString() {
+            return "Conditions{" +
+                    "type='" + type + '\'' +
+                    ", values=" + values +
+                    '}';
+        }
+    }
+}
\ No newline at end of file
diff --git a/security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java 
b/security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java
index 30d9d20c53..98d642833f 100755
--- a/security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java
+++ b/security-admin/src/main/java/org/apache/ranger/rest/GdsREST.java
@@ -115,9 +115,6 @@ public class GdsREST {
     private static final RangerAdminConfig config                          = 
RangerAdminConfig.getInstance();
     private static final int               SHARED_RESOURCES_MAX_BATCH_SIZE = 
config.getInt("ranger.admin.rest.gds.shared.resources.max.batch.size", 100);
 
-    public static final String GDS_POLICY_EXPR_CONDITION                   = 
"expression";
-    public static final String GDS_POLICY_VALIDITY_SCHEDULE_CONDITION      = 
"validitySchedule";
-
     @Autowired
     GdsDBStore gdsStore;
 
@@ -1901,34 +1898,29 @@ List<RangerGrant> 
transformPolicyItemsToGrants(List<RangerPolicyItem> policyItem
             return null;
         }
 
-        List<RangerGrant> ret = new ArrayList<>();
+        List<RangerGrant>   ret         = new ArrayList<>();
 
         for (RangerPolicyItem policyItem : policyItems) {
             List<String> policyItemUsers  = policyItem.getUsers();
             List<String> policyItemGroups = policyItem.getGroups();
             List<String> policyItemRoles  = policyItem.getRoles();
 
-            List<RangerPolicyItemAccess>    policyItemAccesses   = 
policyItem.getAccesses();
-            List<RangerPolicyItemCondition> policyItemConditions = 
policyItem.getConditions();
-
-            List<String> policyItemAccessTypes     = 
policyItemAccesses.stream().map(RangerPolicyItemAccess::getType).collect(Collectors.toList());
-
-            List<RangerPolicy.RangerPolicyItemCondition> 
policyItemConditionExpressions = policyItemConditions.stream().filter(c -> 
c.getType().equals(GDS_POLICY_EXPR_CONDITION)).collect(Collectors.toList());
-            List<String> expressions = 
policyItemConditionExpressions.stream().flatMap(x -> 
x.getValues().stream()).collect(Collectors.toList());
+            List<RangerPolicyItemAccess>    policyItemAccesses    = 
policyItem.getAccesses();
+            List<RangerPolicyItemCondition> policyItemConditions  = 
policyItem.getConditions();
+            List<String>                    policyItemAccessTypes = 
policyItemAccesses.stream().map(RangerPolicyItemAccess::getType).collect(Collectors.toList());
 
-            List<RangerPolicy.RangerPolicyItemCondition> 
policyItemConditionValiditySchedules = policyItemConditions.stream().filter(c 
-> 
c.getType().equals(GDS_POLICY_VALIDITY_SCHEDULE_CONDITION)).collect(Collectors.toList());
-            List<String> validitySchedules = 
policyItemConditionValiditySchedules.stream().flatMap(x -> 
x.getValues().stream()).collect(Collectors.toList());
+            List<RangerGrant.Condition> conditions = 
getGrantConditions(policyItemConditions);
 
             if (CollectionUtils.isNotEmpty(policyItemUsers)) {
-                policyItemUsers.forEach(x -> ret.add(new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.USER, x), policyItemAccessTypes, 
expressions, validitySchedules)));
+                policyItemUsers.forEach(x -> ret.add(new RangerGrant(new 
RangerPrincipal(PrincipalType.USER, x), policyItemAccessTypes, conditions)));
             }
 
             if (CollectionUtils.isNotEmpty(policyItemGroups)) {
-                policyItemGroups.forEach(x -> ret.add(new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, x), policyItemAccessTypes, 
expressions, validitySchedules)));
+                policyItemGroups.forEach(x -> ret.add(new RangerGrant(new 
RangerPrincipal(PrincipalType.GROUP, x), policyItemAccessTypes, conditions)));
             }
 
             if (CollectionUtils.isNotEmpty(policyItemRoles)) {
-                policyItemRoles.forEach(x -> ret.add(new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.ROLE, x), policyItemAccessTypes, 
expressions, validitySchedules)));
+                policyItemRoles.forEach(x -> ret.add(new RangerGrant(new 
RangerPrincipal(PrincipalType.ROLE, x), policyItemAccessTypes, conditions)));
             }
         }
 
@@ -1980,6 +1972,17 @@ RangerPolicy updatePolicyWithModifiedGrants(RangerPolicy 
policy, List<RangerGran
         return policy;
     }
 
+    private List<RangerGrant.Condition> 
getGrantConditions(List<RangerPolicy.RangerPolicyItemCondition> 
policyItemConditions) {
+        List<RangerGrant.Condition> ret = new ArrayList<>();
+
+        if (CollectionUtils.isNotEmpty(policyItemConditions)) {
+            
policyItemConditions.stream().collect(Collectors.toMap(RangerPolicyItemCondition::getType,
 RangerPolicyItemCondition::getValues))
+                    .entrySet().stream().map(expr -> new 
RangerGrant.Condition(expr.getKey(), expr.getValue())).forEach(ret::add);
+        }
+
+        return ret;
+    }
+
     private Long getOrCreateDataShare(Long datasetId, Long serviceId, Long 
zoneId, String serviceName) throws Exception {
         LOG.debug("==> GdsREST.getOrCreateDataShare(dataSetId={} serviceId={} 
zoneId={} serviceName={})", datasetId, serviceId, zoneId, serviceName);
 
@@ -2131,27 +2134,23 @@ private RangerPolicyItem 
transformGrantToPolicyItem(RangerGrant grant) {
             return null;
         }
 
-        RangerPolicyItem policyItem        = new RangerPolicyItem();
-        List<String>     permissions       = grant.getAccessTypes();
-        List<String>     conditions        = grant.getConditions();
-        List<String>     validitySchedules = grant.getValiditySchedules();
+        RangerPolicyItem            policyItem  = new RangerPolicyItem();
+        List<String>                permissions = grant.getAccessTypes();
+        List<RangerGrant.Condition> conditions  = grant.getConditions();
 
         if (CollectionUtils.isNotEmpty(permissions)) {
-            policyItem.setAccesses(permissions.stream().map(accessType -> new 
RangerPolicyItemAccess(accessType, true)).collect(Collectors.toList()));
+            policyItem.setAccesses(permissions.stream()
+                    .map(accessType -> new RangerPolicyItemAccess(accessType, 
true))
+                    .collect(Collectors.toList()));
         }
 
         List<RangerPolicyItemCondition> policyItemConditions = new 
ArrayList<>();
         if (CollectionUtils.isNotEmpty(conditions)) {
-            conditions.stream().map(expr -> new 
RangerPolicyItemCondition(GDS_POLICY_EXPR_CONDITION, 
Collections.singletonList(expr))).forEach(policyItemConditions::add);
-        }
-
-        if (CollectionUtils.isNotEmpty(validitySchedules)) {
-            validitySchedules.stream().map(valditySchedule -> new 
RangerPolicyItemCondition(GDS_POLICY_VALIDITY_SCHEDULE_CONDITION, 
Collections.singletonList(valditySchedule))).forEach(policyItemConditions::add);
+            conditions.stream().map(condition -> new 
RangerPolicyItemCondition(condition.getType(), 
condition.getValues())).forEach(policyItemConditions::add);
         }
 
         policyItem.setConditions(policyItemConditions);
 
-
         switch (grant.getPrincipal().getType()) {
             case USER:
                 
policyItem.setUsers(Collections.singletonList(grant.getPrincipal().getName()));
diff --git 
a/security-admin/src/test/java/org/apache/ranger/rest/TestGdsREST.java 
b/security-admin/src/test/java/org/apache/ranger/rest/TestGdsREST.java
index 0303c38af7..11df009a46 100644
--- a/security-admin/src/test/java/org/apache/ranger/rest/TestGdsREST.java
+++ b/security-admin/src/test/java/org/apache/ranger/rest/TestGdsREST.java
@@ -87,7 +87,7 @@ public void testUpdateDataSetGrants() {
 
         List<RangerPolicy.RangerPolicyItem> hdfsPolicyItems = new 
ArrayList<>(gdsREST.filterPolicyItemsByRequest(policy, request));
 
-        RangerGrant grant3 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, "hdfs"), 
Collections.singletonList("_READ"), Collections.emptyList(), 
Collections.emptyList());
+        RangerGrant grant3 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, "hdfs"), 
Collections.singletonList("_READ"), Collections.emptyList());
         policy = gdsREST.updatePolicyWithModifiedGrants(policy, 
Collections.singletonList(grant3));
 
         List<RangerPolicy.RangerPolicyItem> updatedHdfsPolicyItems = new 
ArrayList<>(gdsREST.filterPolicyItemsByRequest(policy, request));
@@ -111,7 +111,7 @@ public void testRemoveDataSetGrants() {
 
         List<RangerPolicy.RangerPolicyItem> existingHdfsPolicyItems = new 
ArrayList<>(gdsREST.filterPolicyItemsByRequest(policy, request));
 
-        RangerGrant grant4 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, "hdfs"), 
Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
+        RangerGrant grant4 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, "hdfs"), 
Collections.emptyList(), Collections.emptyList());
         policy = gdsREST.updatePolicyWithModifiedGrants(policy, 
Collections.singletonList(grant4));
 
         List<RangerPolicy.RangerPolicyItem> updatedHdfsPolicyItems = 
gdsREST.filterPolicyItemsByRequest(policy, request);
@@ -238,8 +238,20 @@ private RangerPolicy 
createPolicyForDataSet(RangerGds.RangerDataset dataset) {
     }
 
     private List<RangerGrant> createAndGetSampleGrantData() {
-        RangerGrant grant1 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.USER, "hive"), 
Collections.singletonList("_READ"), 
Collections.singletonList("IS_ACCESSED_BEFORE('2024/12/12')"), 
Collections.singletonList("{\"startTime\":\"1970/01/01 
00:00:00\",\"endTime\":\"2025/03/08 00:35:28\",\"timeZone\":\"UTC\"}"));
-        RangerGrant grant2 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, "hdfs"), 
Collections.singletonList("_MANAGE"), Collections.emptyList(), 
Collections.emptyList());
+        List<RangerGrant.Condition> conditions = new ArrayList<>();
+
+        RangerGrant.Condition condition1  = new RangerGrant.Condition(null, 
null);
+        condition1.setType("expression");
+        condition1.setValues(Arrays.asList("IS_ACCESSED_BEFORE('2024/12/12')", 
"_STATE == 'CA'"));
+        conditions.add(condition1);
+
+        RangerGrant.Condition condition2  = new RangerGrant.Condition(null, 
null);
+        condition2.setType("validitySchedule");
+        condition2.setValues(Arrays.asList("{\"startTime\":\"1970/01/01 
00:00:00\",\"endTime\":\"2025/03/08 00:35:28\",\"timeZone\":\"UTC\"}"));
+        conditions.add(condition2);
+
+        RangerGrant grant1 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.USER, "hive"), 
Collections.singletonList("_READ"), conditions);
+        RangerGrant grant2 = new RangerGrant(new 
RangerPrincipal(RangerPrincipal.PrincipalType.GROUP, "hdfs"), 
Collections.singletonList("_MANAGE"), Collections.emptyList());
 
         return Arrays.asList(grant1, grant2);
     }

Reply via email to