Merge pull request #204 from sagilio/fix#199

feat: Implement priority explicit deny override model for v2
casbin · Aug 8, 2021 · 86cc11e · 86cc11e
2 parents 0e26050 + 114fff2
commit 86cc11e
Show file tree

Hide file tree

Showing 17 changed files with 263 additions and 85 deletions.
diff --git a/NetCasbin.UnitTest/Casbin.UnitTests.csproj b/NetCasbin.UnitTest/Casbin.UnitTests.csproj
@@ -99,6 +99,12 @@
     <None Update="examples\multiple_type_policy.csv">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>
+    <None Update="examples\priority_explicit_deny_override_model.conf">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
+    <None Update="examples\priority_explicit_deny_override_policy.csv">
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </None>
     <None Update="examples\priority_explicit_model.conf">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </None>

diff --git a/NetCasbin.UnitTest/Fixtures/TestModelFixture.cs b/NetCasbin.UnitTest/Fixtures/TestModelFixture.cs
@@ -55,6 +55,10 @@ public class TestModelFixture
         internal readonly string _multipleTypeModelText = ReadTestFile("multiple_type_model.conf");
         internal readonly string _multipleTypePolicyText = ReadTestFile("multiple_type_policy.csv");
 
+        // https://github.com/casbin/Casbin.NET/issues/188
+        internal readonly string _priorityExplicitDenyOverrideModelText = ReadTestFile("priority_explicit_deny_override_model.conf");
+        internal readonly string _priorityExplicitDenyOverridePolicyText = ReadTestFile("priority_explicit_deny_override_policy.csv");
+
         public IModel GetNewAbacModel()
         {
             return GetNewTestModel(_abacModelText);
@@ -100,6 +104,11 @@ public IModel GetNewPriorityExplicitTestModel()
             return GetNewTestModel(_priorityExplicitModelText, _priorityExplicitPolicyText);
         }
 
+        public IModel GetNewPriorityExplicitDenyOverrideModel()
+        {
+            return GetNewTestModel(_priorityExplicitDenyOverrideModelText, _priorityExplicitDenyOverridePolicyText);
+        }
+
         public IModel GetNewRbacTestModel()
         {
             return GetNewTestModel(_rbacModelText, _rbacPolicyText);

diff --git a/NetCasbin.UnitTest/ModelTests/ModelTest.cs b/NetCasbin.UnitTest/ModelTests/ModelTest.cs
@@ -492,6 +492,45 @@ public void TestPriorityExplicitModel()
             TestEnforce(e, "data2_allow_group", "data2", "write", true);
         }
 
+        [Fact]
+        public void TestPriorityExplicitDenyOverrideModel()
+        {
+            var e = new Enforcer(_testModelFixture.GetNewPriorityExplicitDenyOverrideModel());
+            e.BuildRoleLinks();
+
+            TestEnforce(e, "alice", "data2", "write", true);
+            TestEnforce(e, "bob", "data2", "read", true);
+
+            // adding a new group, simulating behaviour when two different groups are added to the same person.
+            e.AddPolicy("10", "data2_deny_group_new", "data2", "write", "deny");
+            e.AddGroupingPolicy("alice", "data2_deny_group_new");
+
+            TestEnforce(e, "alice", "data2", "write", false);
+            TestEnforce(e, "bob", "data2", "read", true);
+
+            // expected enforcement result should be true,
+            // as there is a policy with a lower rank 10, that produces allow result.
+            e.AddPolicy("5", "alice", "data2", "write", "allow");
+            TestEnforce(e, "alice", "data2", "write", true);
+
+            // adding deny policy for alice for the same obj,
+            // to ensure that if there is at least one deny, final result will be deny.
+            e.AddPolicy("5", "alice", "data2", "write", "deny");
+            TestEnforce(e, "alice", "data2", "write", false);
+
+            // adding higher fake higher priority policy for alice,
+            // expected enforcement result should be true (ignore this policy).
+            e.AddPolicy("2", "alice", "data2", "write", "allow");
+            TestEnforce(e, "alice", "data2", "write", true);
+            e.AddPolicy("1", "fake-subject", "fake-object", "very-fake-action", "allow");
+            TestEnforce(e, "alice", "data2", "write", true);
+
+            // adding higher (less of 0) priority policy for alice,
+            // to override group policies again.
+            e.AddPolicy("-1", "alice", "data2", "write", "deny");
+            TestEnforce(e, "alice", "data2", "write", false);
+        }
+
         [Fact]
         public void TestKeyMatch2Model()
         {
@@ -552,7 +591,7 @@ public void TestMultipleTypeModel()
             Assert.True(e.Enforce(context, "bob", "domain1", "data1", "read"));
             Assert.False(e.Enforce(context, "bob", "domain1", "data1", "write"));
 
-            // Use r_custom p_custom and m_custom type
+            // Use r3 p3 and m3 type
             context = e.CreatContext
             (
                 PermConstants.RequestType3,

diff --git a/NetCasbin.UnitTest/examples/priority_explicit_deny_override_model.conf b/NetCasbin.UnitTest/examples/priority_explicit_deny_override_model.conf
@@ -0,0 +1,14 @@
+[request_definition]
+r = sub, obj, act
+
+[policy_definition]
+p = priority, sub, obj, act, eft
+
+[role_definition]
+g = _, _
+
+[policy_effect]
+e = priority(p_eft) && !some(where (p_eft == deny))
+
+[matchers]
+m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
diff --git a/NetCasbin.UnitTest/examples/priority_explicit_deny_override_policy.csv b/NetCasbin.UnitTest/examples/priority_explicit_deny_override_policy.csv
@@ -0,0 +1,10 @@
+p, 10, data1_deny_group, data1, read, deny
+p, 10, data1_deny_group, data1, write, deny
+
+p, 10, data2_allow_group, data2, read, allow
+p, 10, data2_allow_group, data2, write, allow
+
+
+g, bob, data2_allow_group
+g, alice, data2_allow_group
+
diff --git a/NetCasbin/Abstractions/Effect/IChainEffector.cs b/NetCasbin/Abstractions/Effect/IChainEffector.cs
@@ -8,6 +8,8 @@ public interface IChainEffector
 
         public bool HitPolicy { get; }
 
+        public int HitPolicyCount { get; }
+
         public string EffectExpression { get; }
 
         public EffectExpressionType EffectExpressionType { get; }

diff --git a/NetCasbin/Abstractions/Evaluation/IExpressionHandler.cs b/NetCasbin/Abstractions/Evaluation/IExpressionHandler.cs
@@ -10,7 +10,7 @@ public interface IExpressionHandler
 
         public IDictionary<string, Parameter> Parameters { get; }
 
-        public void SetEnforceContext(ref EnforceContext context);
+        public void SetEnforceContext(in EnforceContext context);
 
         public void SetFunction(string name, Delegate function);
 

diff --git a/NetCasbin/Abstractions/IEnforcer.cs b/NetCasbin/Abstractions/IEnforcer.cs
@@ -49,8 +49,8 @@ public interface IEnforcer
         /// <param name="requestValues">The request needs to be mediated, usually an array of strings, 
         /// can be class instances if ABAC is used.</param>
         /// <returns>Whether to allow the request.</returns>
-        public bool Enforce(EnforceContext context, params object[] requestValues);
-
+        public bool Enforce(in EnforceContext context, params object[] requestValues);
+         
         /// <summary>
         /// Decides whether a "subject" can access a "object" with the operation
         /// "action", input parameters are usually: (sub, obj, act).
@@ -59,6 +59,5 @@ public interface IEnforcer
         /// can be class instances if ABAC is used.</param>
         /// <returns>Whether to allow the request.</returns>
         public Task<bool> EnforceAsync(EnforceContext context, params object[] requestValues);
-
     }
 }
diff --git a/NetCasbin/Effect/DefaultEffector.cs b/NetCasbin/Effect/DefaultEffector.cs
@@ -66,6 +66,7 @@ public PolicyEffect MergeEffects(string effectExpression, IReadOnlyList<PolicyEf
             PermConstants.PolicyEffect.DenyOverride => EffectExpressionType.DenyOverride,
             PermConstants.PolicyEffect.AllowAndDeny => EffectExpressionType.AllowAndDeny,
             PermConstants.PolicyEffect.Priority => EffectExpressionType.Priority,
+            PermConstants.PolicyEffect.PriorityDenyOverride => EffectExpressionType.PriorityDenyOverride,
             _ => throw new NotSupportedException("Not supported policy effect.")
         };
 
@@ -75,6 +76,8 @@ public PolicyEffect MergeEffects(string effectExpression, IReadOnlyList<PolicyEf
 
         public bool HitPolicy { get; private set; }
 
+        public int HitPolicyCount { get; private set; }
+
         public bool CanChain { get; private set; }
 
         public string EffectExpression { get; private set; }
@@ -87,6 +90,7 @@ public void StartChain(string effectExpression)
             EffectExpressionType = ParseEffectExpressionType(EffectExpression);
             CanChain = true;
             Result = false;
+            HitPolicyCount = 0;
         }
 
         public bool Chain(PolicyEffect effect)
@@ -127,11 +131,19 @@ public bool TryChain(PolicyEffect effect)
                 CanChain = false;
                 Result = result;
                 HitPolicy = hitPolicy;
+                if (hitPolicy)
+                {
+                    HitPolicyCount++;
+                }
                 return true;
             }
 
             Result = result;
             HitPolicy = hitPolicy;
+            if (hitPolicy)
+            {
+                HitPolicyCount++;
+            }
             return true;
         }
 

diff --git a/NetCasbin/Effect/EffectExpressionType.cs b/NetCasbin/Effect/EffectExpressionType.cs
@@ -6,6 +6,7 @@ public enum EffectExpressionType
         AllowOverride,
         AllowAndDeny,
         DenyOverride,
-        Priority
+        Priority,
+        PriorityDenyOverride
     }
 }
diff --git a/NetCasbin/EnforceContext.cs b/NetCasbin/EnforceContext.cs
@@ -1,5 +1,4 @@
-using System;
-using System.Collections.Generic;
+using System.Collections.Generic;
 using Casbin.Model;
 using Casbin.Util;
 

diff --git a/NetCasbin/EnforceSession.cs b/NetCasbin/EnforceSession.cs
@@ -0,0 +1,36 @@
+using System.Collections.Generic;
+using Casbin.Effect;
+
+namespace Casbin
+{
+    internal struct EnforceSession
+    {
+        internal IReadOnlyList<object> RequestValues { get; set; }
+        internal IReadOnlyList<string> PolicyValues { get; set; }
+
+        internal string ExpressionString { get; set; }
+
+        internal int PolicyIndex { get; set; }
+        internal int PolicyCount { get; set; }
+
+        internal PolicyEffect PolicyEffect { get; set; }
+        internal PolicyEffect[] PolicyEffects { get; set; }
+
+        internal bool Determined { get; private set; }
+        internal bool EnforceResult { get; set; }
+
+        internal EffectExpressionType EffectExpressionType {  get; set; }
+        internal bool ExpressionResult { get; set; }
+
+        internal bool IsChainEffector { get; set; }
+        internal bool HasPriority { get; set; }
+        internal int PriorityIndex { get; set; }
+        internal int? Priority { get; set; }
+
+        internal void DetermineResult(bool result)
+        {
+            Determined = true;
+            EnforceResult = result;
+        }
+    }
+}