feat: fix the double quotes caused eval() bug

casbin · Dec 1, 2024 · c82e7c5 · c82e7c5
1 parent 8dd4940
commit c82e7c5
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 1 deletion.
diff --git a/examples/abac_rule_with_comma_model.conf b/examples/abac_rule_with_comma_model.conf
@@ -0,0 +1,11 @@
+[request_definition]
+r = sub, obj, act
+
+[policy_definition]
+p = sub_rule, obj_rule, act
+
+[policy_effect]
+e = some(where (p.eft == allow))
+
+[matchers]
+m = r.act == p.act && eval(p.sub_rule) && eval(p.obj_rule)
diff --git a/src/main/java/org/casbin/jcasbin/main/CoreEnforcer.java b/src/main/java/org/casbin/jcasbin/main/CoreEnforcer.java
@@ -42,6 +42,9 @@
 import java.util.function.BiPredicate;
 import java.util.function.Function;
 
+import static org.casbin.jcasbin.util.Util.hasEval;
+import static org.casbin.jcasbin.util.Util.splitCommaDelimitedList;
+
 /**
  * CoreEnforcer defines the core functionality of an enforcer.
  */
@@ -580,6 +583,7 @@ private EnforceResult enforce(String matcher, Object... rvals) {
         } else {
             expString = Util.removeComments(Util.escapeAssertion(matcher));
         }
+        boolean hasEval = hasEval(expString);
 
         // json process
         if (acceptJsonRequest) {
@@ -629,6 +633,9 @@ private EnforceResult enforce(String matcher, Object... rvals) {
 
             for (int i = 0; i < policy.size(); i++) {
                 List<String> pvals = policy.get(i);
+                if (hasEval) {
+                    pvals = splitCommaDelimitedList(pvals);
+                }
                 Map<String, Object> parameters = new HashMap<>(rvals.length + pTokens.length);
                 getPTokens(parameters, pType, pvals, pTokens);
                 getRTokens(parameters, rType, rvals);

diff --git a/src/main/java/org/casbin/jcasbin/util/Util.java b/src/main/java/org/casbin/jcasbin/util/Util.java
@@ -284,6 +284,21 @@ public static String[] splitCommaDelimited(String s) {
         return records;
     }
 
+    /**
+     * splits each string in the given list by commas according to CSV format
+     * and removes any extra double quotes
+     * @param rule the rule to be modified
+     * @return the modified rule
+     */
+    public static List<String> splitCommaDelimitedList(List<String> rule) {
+        List<String> modifiedRule = new ArrayList<>();
+        for (String s : rule) {
+            String[] strings = splitCommaDelimited(s);
+            modifiedRule.add(strings[0]);
+        }
+        return modifiedRule;
+    }
+
     /**
      * setEquals determines whether two string sets are identical.
      *
@@ -314,7 +329,7 @@ public static boolean setEquals(List<String> a, List<String> b) {
     }
 
     public static boolean hasEval(String exp) {
-        return evalReg.matcher(exp).matches();
+        return evalReg.matcher(exp).find();
     }
 
     public static String replaceEval(String s, String replacement) {

diff --git a/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java b/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java
@@ -16,11 +16,15 @@
 
 import org.casbin.jcasbin.util.Util;
 import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.HashMap;
 
 import static org.casbin.jcasbin.main.TestUtil.testDomainEnforce;
 import static org.casbin.jcasbin.main.TestUtil.testEnforce;
+import static org.junit.Assert.*;
 
 public class AbacAPIUnitTest {
     @Test
@@ -57,6 +61,34 @@ public void testEvalWithDomain() {
         testDomainEnforce(e, "bob", "domain2", "data2", "read", true);
     }
 
+    @Test
+    public void testEvalWithComma() {
+        Enforcer e = new Enforcer("examples/abac_rule_with_comma_model.conf");
+        List<String> rule = new ArrayList<>();
+        rule.add("true");
+        rule.add("\"let test=seq.set('alice','bob');include(test,r.sub.name)\"");
+        rule.add("read");
+        List<String> newRule = new ArrayList<>();
+        newRule.add("true");
+        newRule.add("\"let test=seq.set('bob');include(test,r.sub.name)\"");
+        newRule.add("read");
+        assertTrue(e.addPolicy(rule));
+        assertFalse(e.addPolicy(rule));
+
+        Map<String, Object> sub = new HashMap<>();
+        sub.put("name", "alice");
+
+        testEnforce(e, sub, "data1", "read", true);
+
+        assertTrue(e.updatePolicy("p", "p", rule, newRule));
+        testEnforce(e, sub, "data1", "read", false);
+        sub.put("name", "bob");
+        testEnforce(e, sub, "data1", "read", true);
+
+        assertTrue(e.removePolicy(newRule));
+        testEnforce(e, sub, "data1", "read", false);
+    }
+
     @Test
     public void testABACMapRequest() {
         Enforcer e = new Enforcer("examples/abac_rule_map_model.conf");

diff --git a/src/test/java/org/casbin/jcasbin/main/UtilTest.java b/src/test/java/org/casbin/jcasbin/main/UtilTest.java
@@ -27,6 +27,7 @@
 import java.io.IOException;
 import java.io.StringReader;
 
+import static org.casbin.jcasbin.util.Util.hasEval;
 import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.*;
 
@@ -84,6 +85,13 @@ public void testSplitCommaDelimited(){
     assertArrayEquals(new String[]{"a b", "c", "d"}, Util.splitCommaDelimited("\"a b\", c, d"));
   }
 
+  @Test
+  public void testHasEval() {
+      assertTrue(hasEval("eval(test)"));
+      assertTrue(hasEval("r_act == p_act && eval(p_sub_rule) && eval(p_obj_rule)"));
+      assertFalse(hasEval("evaltest"));
+  }
+
   @Test
   public void testReplaceEval() {
       Util.logPrint(Util.replaceEval("eval(test)", "testEval"));