fix: make ecforceEx return a result which include `is (#308)

request allow` and `explain` Signed-off-by: imp2002 <imp07@qq.com> Signed-off-by: imp2002 <imp07@qq.com>
casbin · Oct 23, 2022 · 783030c · 783030c
1 parent c61f3e4
commit 783030c
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 50 deletions.
diff --git a/src/main/java/org/casbin/jcasbin/main/CoreEnforcer.java b/src/main/java/org/casbin/jcasbin/main/CoreEnforcer.java
@@ -434,14 +434,13 @@ public void buildRoleLinks() {
      * input parameters are usually: (matcher, explain, sub, obj, act), use model matcher by default when matcher is "" or null.
      *
      * @param matcher the custom matcher.
-     * @param explain to explain enforcement by informing matched rules
      * @param rvals   the request needs to be mediated, usually an array
      *                of strings, can be class instances if ABAC is used.
      * @return whether to allow the request.
      */
-    private boolean enforce(String matcher, List<String> explain, Object... rvals) {
+    private EnforceResult enforce(String matcher, Object... rvals) {
         if (!enabled) {
-            return true;
+            return new EnforceResult(true, new ArrayList<>(Collections.singletonList("The enforcer is not enable, allow all request")));
         }
 
         boolean compileCached = true;
@@ -608,12 +607,13 @@ private boolean enforce(String matcher, List<String> explain, Object... rvals) {
             result = eft.mergeEffects(model.model.get("e").get(eType).value, policyEffects, matcherResults);
         }
 
-        if (explain != null && explainIndex != -1) {
+        List<String> explain = new ArrayList<>();
+        if (explainIndex != -1) {
             explain.addAll(model.model.get("p").get(pType).policy.get(explainIndex));
         }
 
         Util.logEnforce(rvals, result, explain);
-        return result;
+        return new EnforceResult(result, explain);
     }
 
     /**
@@ -625,7 +625,7 @@ private boolean enforce(String matcher, List<String> explain, Object... rvals) {
      * @return whether to allow the request.
      */
     public boolean enforce(Object... rvals) {
-        return enforce(null, null, rvals);
+        return enforce(null, rvals).isAllow();
     }
 
     /**
@@ -638,7 +638,7 @@ public boolean enforce(Object... rvals) {
      * @return whether to allow the request.
      */
     public boolean enforceWithMatcher(String matcher, Object... rvals) {
-        return enforce(matcher, null, rvals);
+        return enforce(matcher, rvals).isAllow();
     }
 
     /**
@@ -650,9 +650,8 @@ public boolean enforceWithMatcher(String matcher, Object... rvals) {
      *              of strings, can be class instances if ABAC is used.
      * @return whether to allow the request.
      */
-    public boolean enforceEx(Object... rvals) {
-        List<String> explain = new ArrayList<>();
-        return enforce("", explain, rvals);
+    public EnforceResult enforceEx(Object... rvals) {
+        return enforce(null, rvals);
     }
 
     /**
@@ -665,9 +664,8 @@ public boolean enforceEx(Object... rvals) {
      *                of strings, can be class instances if ABAC is used.
      * @return whether to allow the request.
      */
-    public boolean enforceExWithMatcher(String matcher, Object... rvals) {
-        List<String> explain = new ArrayList<>();
-        return enforce(matcher, explain, rvals);
+    public EnforceResult enforceExWithMatcher(String matcher, Object... rvals) {
+        return enforce(matcher, rvals);
     }
 
     /**

diff --git a/src/main/java/org/casbin/jcasbin/main/EnforceResult.java b/src/main/java/org/casbin/jcasbin/main/EnforceResult.java
@@ -0,0 +1,54 @@
+// Copyright 2022 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package org.casbin.jcasbin.main;
+
+import java.util.List;
+
+public class EnforceResult {
+    private boolean allow;
+    private List<String> explain;
+
+    public boolean isAllow() {
+        return allow;
+    }
+
+    public void setAllow(boolean allow) {
+        this.allow = allow;
+    }
+
+    public List<String> getExplain() {
+        return explain;
+    }
+
+    public void setExplain(List<String> explain) {
+        this.explain = explain;
+    }
+
+    public EnforceResult() {
+    }
+
+    public EnforceResult(boolean allow, List<String> explain) {
+        this.allow = allow;
+        this.explain = explain;
+    }
+
+    @Override
+    public String toString() {
+        return "EnforceResult{" +
+            "allow=" + allow +
+            ", explain=" + explain +
+            '}';
+    }
+}
diff --git a/src/main/java/org/casbin/jcasbin/main/SyncedEnforcer.java b/src/main/java/org/casbin/jcasbin/main/SyncedEnforcer.java
@@ -190,7 +190,7 @@ public boolean enforceWithMatcher(String matcher, Object... rvals) {
      * @return whether to allow the request.
      */
     @Override
-    public boolean enforceEx(Object... rvals) {
+    public EnforceResult enforceEx(Object... rvals) {
         return runSynchronized(() -> super.enforceEx(rvals), READ_WRITE_LOCK.readLock());
     }
 
@@ -205,7 +205,7 @@ public boolean enforceEx(Object... rvals) {
      * @return whether to allow the request.
      */
     @Override
-    public boolean enforceExWithMatcher(String matcher, Object... rvals) {
+    public EnforceResult enforceExWithMatcher(String matcher, Object... rvals) {
         return runSynchronized(() -> super.enforceExWithMatcher(matcher, rvals), READ_WRITE_LOCK.readLock());
     }
 

diff --git a/src/test/java/org/casbin/jcasbin/main/EnforcerUnitTest.java b/src/test/java/org/casbin/jcasbin/main/EnforcerUnitTest.java
@@ -312,28 +312,28 @@ public void testEnforceExLog() {
 
         // the previous matcher is
         // m = r.sub == p.sub && r.obj == p.obj && r.act == p.act
-        testEnforceEx(e, "alice", "data1", "read", true);
-        testEnforceEx(e, "bob", "data2", "write", true);
-        testEnforceEx(e, "root", "data2", "read", false);
-        testEnforceEx(e, "root", "data3", "read", false);
-        testEnforceEx(e, "jack", "data3", "read", false);
+        testEnforceEx(e, "alice", "data1", "read", true, new String[]{"alice", "data1", "read"});
+        testEnforceEx(e, "bob", "data2", "write", true, new String[]{"bob", "data2", "write"});
+        testEnforceEx(e, "root", "data2", "read", false, new String[]{});
+        testEnforceEx(e, "root", "data3", "read", false, new String[]{});
+        testEnforceEx(e, "jack", "data3", "read", false, new String[]{});
 
         // custom matcher
         String matcher = "m = r.sub == 'root' || r.sub == p.sub && r.obj == p.obj && r.act == p.act";
-        TestUtil.testEnforceExWithMatcher(e, matcher, "alice", "data1", "read", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "bob", "data2", "write", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data2", "read", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data3", "read", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "jack", "data3", "read", false);
+        TestUtil.testEnforceExWithMatcher(e, matcher, "alice", "data1", "read", true, new String[]{"alice", "data1", "read"});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "bob", "data2", "write", true, new String[]{"bob", "data2", "write"});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data2", "read", true, new String[]{});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data3", "read", true, new String[]{});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "jack", "data3", "read", false, new String[]{});
 
         // the previous matcher is
         // m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
         e = new Enforcer("examples/rbac_model.conf", "examples/rbac_policy.csv", true);
-        testEnforceEx(e, "alice", "data1", "read", true);
-        testEnforceEx(e, "alice", "data2", "read", true);
-        testEnforceEx(e, "alice", "data2", "write", true);
-        testEnforceEx(e, "bob", "data1", "write", false);
-        testEnforceEx(e, "bob", "data2", "write", true);
+        testEnforceEx(e, "alice", "data1", "read", true, new String[]{"alice", "data1", "read"});
+        testEnforceEx(e, "alice", "data2", "read", true, new String[]{"data2_admin", "data2", "read"});
+        testEnforceEx(e, "alice", "data2", "write", true, new String[]{"data2_admin", "data2", "write"});
+        testEnforceEx(e, "bob", "data1", "write", false, new String[]{});
+        testEnforceEx(e, "bob", "data2", "write", true, new String[]{"bob", "data2", "write"});
     }
 
     @Test

diff --git a/src/test/java/org/casbin/jcasbin/main/SyncedEnforcerUnitTest.java b/src/test/java/org/casbin/jcasbin/main/SyncedEnforcerUnitTest.java
@@ -264,28 +264,28 @@ public void testEnforceExLog() {
 
         // the previous matcher is
         // m = r.sub == p.sub && r.obj == p.obj && r.act == p.act
-        testEnforceEx(e, "alice", "data1", "read", true);
-        testEnforceEx(e, "bob", "data2", "write", true);
-        testEnforceEx(e, "root", "data2", "read", false);
-        testEnforceEx(e, "root", "data3", "read", false);
-        testEnforceEx(e, "jack", "data3", "read", false);
+        testEnforceEx(e, "alice", "data1", "read", true, new String[]{"alice", "data1", "read"});
+        testEnforceEx(e, "bob", "data2", "write", true, new String[]{"bob", "data2", "write"});
+        testEnforceEx(e, "root", "data2", "read", false, new String[]{});
+        testEnforceEx(e, "root", "data3", "read", false, new String[]{});
+        testEnforceEx(e, "jack", "data3", "read", false, new String[]{});
 
         // custom matcher
         String matcher = "m = r.sub == 'root' || r.sub == p.sub && r.obj == p.obj && r.act == p.act";
-        TestUtil.testEnforceExWithMatcher(e, matcher, "alice", "data1", "read", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "bob", "data2", "write", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data2", "read", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data3", "read", true);
-        TestUtil.testEnforceExWithMatcher(e, matcher, "jack", "data3", "read", false);
+        TestUtil.testEnforceExWithMatcher(e, matcher, "alice", "data1", "read", true, new String[]{"alice", "data1", "read"});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "bob", "data2", "write", true, new String[]{"bob", "data2", "write"});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data2", "read", true, new String[]{});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "root", "data3", "read", true, new String[]{});
+        TestUtil.testEnforceExWithMatcher(e, matcher, "jack", "data3", "read", false, new String[]{});
 
         // the previous matcher is
         // m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
-        e = new Enforcer("examples/rbac_model.conf", "examples/rbac_policy.csv", true);
-        testEnforceEx(e, "alice", "data1", "read", true);
-        testEnforceEx(e, "alice", "data2", "read", true);
-        testEnforceEx(e, "alice", "data2", "write", true);
-        testEnforceEx(e, "bob", "data1", "write", false);
-        testEnforceEx(e, "bob", "data2", "write", true);
+        e = new SyncedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv", true);
+        testEnforceEx(e, "alice", "data1", "read", true, new String[]{"alice", "data1", "read"});
+        testEnforceEx(e, "alice", "data2", "read", true, new String[]{"data2_admin", "data2", "read"});
+        testEnforceEx(e, "alice", "data2", "write", true, new String[]{"data2_admin", "data2", "write"});
+        testEnforceEx(e, "bob", "data1", "write", false, new String[]{});
+        testEnforceEx(e, "bob", "data2", "write", true, new String[]{"bob", "data2", "write"});
     }
 
     @Test

diff --git a/src/test/java/org/casbin/jcasbin/main/TestUtil.java b/src/test/java/org/casbin/jcasbin/main/TestUtil.java
@@ -35,12 +35,20 @@ static void testEnforceWithMatcher(Enforcer e, String matcher, Object sub, Objec
         assertEquals(res, e.enforceWithMatcher(matcher, sub, obj, act));
     }
 
-    static void testEnforceEx(Enforcer e, Object sub, Object obj, String act, boolean res) {
-        assertEquals(res, e.enforceEx(sub, obj, act));
+    static void testEnforceEx(Enforcer e, Object sub, Object obj, String act, boolean res, String[] explain) {
+        EnforceResult enforceResult = e.enforceEx(sub, obj, act);
+        assertEquals(res, enforceResult.isAllow());
+        for (int i = 0; i < explain.length; i++) {
+            assertEquals(explain[i], enforceResult.getExplain().get(i));
+        }
     }
 
-    static void testEnforceExWithMatcher(Enforcer e, String matcher, Object sub, Object obj, String act, boolean res) {
-        assertEquals(res, e.enforceExWithMatcher(matcher, sub, obj, act));
+    static void testEnforceExWithMatcher(Enforcer e, String matcher, Object sub, Object obj, String act, boolean res, String[] explain) {
+        EnforceResult enforceResult = e.enforceExWithMatcher(matcher, sub, obj, act);
+        assertEquals(res, enforceResult.isAllow());
+        for (int i = 0; i < explain.length; i++) {
+            assertEquals(explain[i], enforceResult.getExplain().get(i));
+        }
     }
 
     static void testEnforceWithoutUsers(Enforcer e, String obj, String act, boolean res) {