From c82e7c5ac643ed8aab55d085cee8bb2dcf352a00 Mon Sep 17 00:00:00 2001 From: TX <1428427011@qq.com> Date: Sun, 1 Dec 2024 18:57:52 +0800 Subject: [PATCH 1/3] feat: fix the double quotes caused eval() bug --- examples/abac_rule_with_comma_model.conf | 11 +++++++ .../org/casbin/jcasbin/main/CoreEnforcer.java | 7 ++++ .../java/org/casbin/jcasbin/util/Util.java | 17 +++++++++- .../casbin/jcasbin/main/AbacAPIUnitTest.java | 32 +++++++++++++++++++ .../org/casbin/jcasbin/main/UtilTest.java | 8 +++++ 5 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 examples/abac_rule_with_comma_model.conf diff --git a/examples/abac_rule_with_comma_model.conf b/examples/abac_rule_with_comma_model.conf new file mode 100644 index 00000000..654519cf --- /dev/null +++ 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 index 60d88e9a..ec3c7ace 100644 --- 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 pvals = policy.get(i); + if (hasEval) { + pvals = splitCommaDelimitedList(pvals); + } Map 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 index 4077ee18..6f739025 100644 --- 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 splitCommaDelimitedList(List rule) { + List 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 a, List 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 index d0f8a65b..57e82c19 100644 --- 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 rule = new ArrayList<>(); + rule.add("true"); + rule.add("\"let test=seq.set('alice','bob');include(test,r.sub.name)\""); + rule.add("read"); + List 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 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 index 545bc373..187951fd 100644 --- 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")); From d3113c8737c58442fce19bd5356c2623486ac036 Mon Sep 17 00:00:00 2001 From: TX <1428427011@qq.com> Date: Sun, 1 Dec 2024 22:53:17 +0800 Subject: [PATCH 2/3] feat: fix the conf file --- examples/abac_rule_with_comma_model.conf | 11 ----------- .../java/org/casbin/jcasbin/main/AbacAPIUnitTest.java | 6 +++--- 2 files changed, 3 insertions(+), 14 deletions(-) delete mode 100644 examples/abac_rule_with_comma_model.conf diff --git a/examples/abac_rule_with_comma_model.conf b/examples/abac_rule_with_comma_model.conf deleted file mode 100644 index 654519cf..00000000 --- a/examples/abac_rule_with_comma_model.conf +++ /dev/null @@ -1,11 +0,0 @@ -[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/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java b/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java index 57e82c19..3ddc7bfa 100644 --- a/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java +++ b/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java @@ -63,14 +63,14 @@ public void testEvalWithDomain() { @Test public void testEvalWithComma() { - Enforcer e = new Enforcer("examples/abac_rule_with_comma_model.conf"); + Enforcer e = new Enforcer("examples/abac_rule_model.conf"); List rule = new ArrayList<>(); - rule.add("true"); rule.add("\"let test=seq.set('alice','bob');include(test,r.sub.name)\""); + rule.add("data1"); rule.add("read"); List newRule = new ArrayList<>(); - newRule.add("true"); newRule.add("\"let test=seq.set('bob');include(test,r.sub.name)\""); + newRule.add("data1"); newRule.add("read"); assertTrue(e.addPolicy(rule)); assertFalse(e.addPolicy(rule)); From 55d1efacd88e7d3ccadda7ac4dc4d9ed06b9095c Mon Sep 17 00:00:00 2001 From: TX <1428427011@qq.com> Date: Mon, 2 Dec 2024 19:55:44 +0800 Subject: [PATCH 3/3] feat: fix the test case --- .../casbin/jcasbin/main/AbacAPIUnitTest.java | 39 +++++-------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java b/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java index 3ddc7bfa..7a252eb0 100644 --- a/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java +++ b/src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java @@ -14,7 +14,6 @@ package org.casbin.jcasbin.main; -import org.casbin.jcasbin.util.Util; import org.junit.Test; import java.util.ArrayList; @@ -24,7 +23,6 @@ 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 @@ -46,6 +44,15 @@ public void testEval() { alice.setAge(60); testEnforce(e, alice, "/data2", "read", false); testEnforce(e, alice, "/data2", "write", false); + + List rule = new ArrayList<>(); + rule.add("\"r.sub.name == 'alice,green'\""); + rule.add("data1"); + rule.add("read"); + e.addPolicy(rule); + + TestEvalRule aliceGreen = new TestEvalRule("alice,green", 18); + testEnforce(e, aliceGreen, "data1", "read", true); } @Test @@ -61,34 +68,6 @@ public void testEvalWithDomain() { testDomainEnforce(e, "bob", "domain2", "data2", "read", true); } - @Test - public void testEvalWithComma() { - Enforcer e = new Enforcer("examples/abac_rule_model.conf"); - List rule = new ArrayList<>(); - rule.add("\"let test=seq.set('alice','bob');include(test,r.sub.name)\""); - rule.add("data1"); - rule.add("read"); - List newRule = new ArrayList<>(); - newRule.add("\"let test=seq.set('bob');include(test,r.sub.name)\""); - newRule.add("data1"); - newRule.add("read"); - assertTrue(e.addPolicy(rule)); - assertFalse(e.addPolicy(rule)); - - Map 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");