From 114fff27ecc132bbdb577faa58029a5f3b77e4b8 Mon Sep 17 00:00:00 2001 From: Sagilio Date: Sun, 8 Aug 2021 07:13:01 +0800 Subject: [PATCH] feat: Implement priority explicit deny override model for v2 and add enforce session Signed-off-by: Sagilio --- NetCasbin.UnitTest/Casbin.UnitTests.csproj | 6 + .../Fixtures/TestModelFixture.cs | 9 + NetCasbin.UnitTest/ModelTests/ModelTest.cs | 41 +++- ...priority_explicit_deny_override_model.conf | 14 ++ ...priority_explicit_deny_override_policy.csv | 10 + .../Abstractions/Effect/IChainEffector.cs | 2 + .../Evaluation/IExpressionHandler.cs | 2 +- NetCasbin/Abstractions/IEnforcer.cs | 5 +- NetCasbin/Effect/DefaultEffector.cs | 12 ++ NetCasbin/Effect/EffectExpressionType.cs | 3 +- NetCasbin/EnforceContext.cs | 3 +- NetCasbin/EnforceSession.cs | 36 ++++ NetCasbin/Enforcer.cs | 184 +++++++++++------- NetCasbin/Evaluation/EffiectEvaluation.cs | 14 ++ NetCasbin/Evaluation/ExpressionHandler.cs | 3 +- NetCasbin/PermConstants.cs | 1 + NetCasbin/ThrowHelper.cs | 3 + 17 files changed, 263 insertions(+), 85 deletions(-) create mode 100644 NetCasbin.UnitTest/examples/priority_explicit_deny_override_model.conf create mode 100644 NetCasbin.UnitTest/examples/priority_explicit_deny_override_policy.csv create mode 100644 NetCasbin/EnforceSession.cs diff --git a/NetCasbin.UnitTest/Casbin.UnitTests.csproj b/NetCasbin.UnitTest/Casbin.UnitTests.csproj index 4e2eefb1..b81595c2 100644 --- a/NetCasbin.UnitTest/Casbin.UnitTests.csproj +++ b/NetCasbin.UnitTest/Casbin.UnitTests.csproj @@ -99,6 +99,12 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest + PreserveNewest diff --git a/NetCasbin.UnitTest/Fixtures/TestModelFixture.cs b/NetCasbin.UnitTest/Fixtures/TestModelFixture.cs index deb1c8d9..7d66bae8 100644 --- 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 index f0e3e08c..fc052ce4 100644 --- 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 new file mode 100644 index 00000000..01b66691 --- /dev/null +++ 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 new file mode 100644 index 00000000..99d9ab51 --- /dev/null +++ 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 index d04518f9..8fe6412b 100644 --- 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 index b52a5302..141d5fd9 100644 --- a/NetCasbin/Abstractions/Evaluation/IExpressionHandler.cs +++ b/NetCasbin/Abstractions/Evaluation/IExpressionHandler.cs @@ -10,7 +10,7 @@ public interface IExpressionHandler public IDictionary 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 index 3d4f645e..ac0c4b11 100644 --- a/NetCasbin/Abstractions/IEnforcer.cs +++ b/NetCasbin/Abstractions/IEnforcer.cs @@ -49,8 +49,8 @@ public interface IEnforcer /// The request needs to be mediated, usually an array of strings, /// can be class instances if ABAC is used. /// Whether to allow the request. - public bool Enforce(EnforceContext context, params object[] requestValues); - + public bool Enforce(in EnforceContext context, params object[] requestValues); + /// /// 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. /// Whether to allow the request. public Task EnforceAsync(EnforceContext context, params object[] requestValues); - } } diff --git a/NetCasbin/Effect/DefaultEffector.cs b/NetCasbin/Effect/DefaultEffector.cs index 766529f5..b05a9379 100644 --- a/NetCasbin/Effect/DefaultEffector.cs +++ b/NetCasbin/Effect/DefaultEffector.cs @@ -66,6 +66,7 @@ public PolicyEffect MergeEffects(string effectExpression, IReadOnlyList 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 RequestValues { get; set; } + internal IReadOnlyList 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; + } + } +} diff --git a/NetCasbin/Enforcer.cs b/NetCasbin/Enforcer.cs index d1d7b168..914060f1 100644 --- a/NetCasbin/Enforcer.cs +++ b/NetCasbin/Enforcer.cs @@ -14,7 +14,6 @@ #if !NET45 using Microsoft.Extensions.Logging; #endif -using DynamicExpresso; namespace Casbin { @@ -94,7 +93,7 @@ public IAdapter Adapter /// The request needs to be mediated, usually an array of strings, /// can be class instances if ABAC is used. /// Whether to allow the request. - public bool Enforce(EnforceContext context, params object[] requestValues) + public bool Enforce(in EnforceContext context, params object[] requestValues) { if (Enabled is false) { @@ -201,47 +200,33 @@ private bool InternalEnforce(in EnforceContext context, IPolicyManager policyMan } } - private bool InternalEnforce(EnforceContext context, IReadOnlyList requestValues) + private bool InternalEnforce(in EnforceContext context, IReadOnlyList requestValues) { - string expressionString = context.Matcher; + var session = new EnforceSession(); var policyList = context.PolicyAssertion.Policy; - int policyCount = policyList.Count; - int requestTokenCount = context.RequestAssertion.Tokens.Count; - if (requestTokenCount != requestValues.Count) - { - throw new ArgumentException($"Invalid request size: expected {requestTokenCount}, got {requestValues.Count}."); - } - ExpressionHandler.SetEnforceContext(ref context); - ExpressionHandler.SetRequestParameters(requestValues); + session.RequestValues = requestValues; IChainEffector chainEffector = Effector as IChainEffector; - bool isChainEffector = chainEffector is not null; - bool expressionResult; - bool? nowResult = null; + session.IsChainEffector = chainEffector is not null; - PolicyEffect[] policyEffects = null; - if (isChainEffector) - { - chainEffector.StartChain(context.Effect); - } - else - { - policyEffects = new PolicyEffect[policyCount]; - } + session = HandleInitialRequest(in context, ref session, chainEffector); - if (policyCount != 0) + if (session.PolicyCount != 0) { - for (int policyIndex = 0; policyIndex < policyCount; policyIndex++) + for (int policyIndex = 0; policyIndex < session.PolicyCount; policyIndex++) { IReadOnlyList policyValues = policyList[policyIndex]; - string actualExpressionString = HandleBeforeExpression(context, expressionString, policyValues, policyCount); - expressionResult = ExpressionHandler.Invoke(actualExpressionString, requestValues); + session.PolicyValues = policyValues; + session.PolicyIndex = policyIndex; - bool determined = isChainEffector - ? HandleExpressionResult(context, expressionResult, chainEffector, policyValues, policyCount, out nowResult) - : HandleExpressionResult(context, expressionResult, Effector, policyEffects, policyValues, policyIndex, policyCount, out nowResult); + session = HandleBeforeExpression(in context, ref session, chainEffector); + session.ExpressionResult = ExpressionHandler.Invoke(session.ExpressionString, requestValues); - if (determined) + session = session.IsChainEffector + ? HandleExpressionResult(in context, ref session, chainEffector) + : HandleExpressionResult(in context, ref session, Effector); + + if (session.Determined) { break; } @@ -249,22 +234,51 @@ private bool InternalEnforce(EnforceContext context, IReadOnlyList reque } else { - string actualExpressionString = HandleBeforeExpression(context, expressionString, null, policyCount); - expressionResult = ExpressionHandler.Invoke(actualExpressionString, requestValues); + session = HandleBeforeExpression(in context,ref session, chainEffector); + session.ExpressionResult = ExpressionHandler.Invoke(session.ExpressionString, requestValues); + + session = session.IsChainEffector + ? HandleExpressionResult(in context, ref session, chainEffector) + : HandleExpressionResult(in context, ref session, Effector); + } + + return session.EnforceResult; + } + + private EnforceSession HandleInitialRequest(in EnforceContext context, ref EnforceSession session, IChainEffector chainEffector) + { + session.ExpressionString = context.Matcher; + session.PolicyCount = context.PolicyAssertion.Policy.Count; + + int requestTokenCount = context.RequestAssertion.Tokens.Count; + if (requestTokenCount != session.RequestValues.Count) + { + throw new ArgumentException($"Invalid request size: expected {requestTokenCount}, got {session.RequestValues.Count}."); + } - _ = isChainEffector - ? HandleExpressionResult(context, expressionResult, chainEffector, null, policyCount, out nowResult) - : HandleExpressionResult(context, expressionResult, Effector, policyEffects, null, 0, policyCount, out nowResult); + ExpressionHandler.SetEnforceContext(in context); + ExpressionHandler.SetRequestParameters(session.RequestValues); + + if (session.IsChainEffector) + { + chainEffector.StartChain(context.Effect); + } + else + { + session.PolicyEffects = new PolicyEffect[session.PolicyCount]; } - bool finalResult = nowResult ?? false; - return finalResult; + + session.EffectExpressionType = chainEffector?.EffectExpressionType ?? DefaultEffector.ParseEffectExpressionType(session.ExpressionString); + session.HasPriority = context.PolicyAssertion.TryGetTokenIndex("priority", out int priorityIndex); + session.PriorityIndex = priorityIndex; + return session; } - private string HandleBeforeExpression(in EnforceContext context, string expressionString, IReadOnlyList policyValues, int policyCount) + private EnforceSession HandleBeforeExpression(in EnforceContext context, ref EnforceSession session, IChainEffector chainEffector) { int policyTokenCount = context.PolicyAssertion.Tokens.Count; - if (policyCount is 0) + if (session.PolicyCount is 0) { if (context.HasEval) { @@ -273,37 +287,57 @@ private string HandleBeforeExpression(in EnforceContext context, string expressi IReadOnlyList tempPolicyValues = Enumerable.Repeat(string.Empty, policyTokenCount).ToArray(); ExpressionHandler.SetPolicyParameters(tempPolicyValues); - return expressionString; + return session; + } + + if (policyTokenCount != session.PolicyValues.Count) + { + throw new ArgumentException($"Invalid policy size: expected {policyTokenCount}, got {session.PolicyValues.Count}."); + } + + if (session.IsChainEffector is false && session.EffectExpressionType is EffectExpressionType.PriorityDenyOverride) + { + ThrowHelper.ThrowNotSupportException($"Only {nameof(IChainEffector)} support {nameof(EffectExpressionType.PriorityDenyOverride)} policy effect expression."); } - if (policyTokenCount != policyValues.Count) + if (session.HasPriority && session.EffectExpressionType is EffectExpressionType.PriorityDenyOverride) { - throw new ArgumentException($"Invalid policy size: expected {policyTokenCount}, got {policyValues.Count}."); + if (int.TryParse(session.PolicyValues[session.PriorityIndex], out int nowPriority)) + { + if (session.Priority.HasValue && nowPriority != session.Priority.Value + && chainEffector.HitPolicyCount > 0) + { + session.DetermineResult(chainEffector.Result); + } + session.Priority = nowPriority; + } } - ExpressionHandler.SetPolicyParameters(policyValues); + ExpressionHandler.SetPolicyParameters(session.PolicyValues); if (context.HasEval) { - expressionString = RewriteEval(expressionString, context.PolicyAssertion.Tokens, policyValues); + session.ExpressionString = context.Matcher; + session.ExpressionString = RewriteEval(session.ExpressionString, context.PolicyAssertion.Tokens, session.PolicyValues); } - return expressionString; + + return session; } - private static bool HandleExpressionResult(in EnforceContext context, bool expressionResult, IEffector effector, PolicyEffect[] policyEffects, - IReadOnlyList policyValues, int policyIndex, int policyCount, out bool? nowResult) + private static EnforceSession HandleExpressionResult(in EnforceContext context, ref EnforceSession session, IEffector effector) { PolicyEffect nowEffect; - if (policyCount is 0) + if (session.PolicyCount is 0) { - nowEffect = GetEffect(expressionResult); - nowEffect = effector.MergeEffects(context.Effect, new[] { nowEffect }, null, policyIndex, policyCount, out _); + nowEffect = GetEffect(session.ExpressionResult); + nowEffect = effector.MergeEffects(context.Effect, new[] { nowEffect }, null, session.PolicyIndex, session.PolicyCount, out _); bool finalResult = nowEffect.ToNullableBool() ?? false; - nowResult = finalResult; - return true; + session.DetermineResult(finalResult); + return session; } - nowEffect = GetEffect(expressionResult); + IReadOnlyList policyValues = session.PolicyValues; + nowEffect = GetEffect(session.ExpressionResult); if (nowEffect is not PolicyEffect.Indeterminate && context.PolicyAssertion.TryGetTokenIndex("eft", out int index)) { @@ -316,43 +350,43 @@ private static bool HandleExpressionResult(in EnforceContext context, bool expre }; } - policyEffects[policyIndex] = nowEffect; - nowEffect = effector.MergeEffects(context.Effect, policyEffects, null, policyIndex, policyCount, out int hitPolicyIndex); - nowResult = nowEffect.ToNullableBool(); + session.PolicyEffects[session.PolicyIndex] = nowEffect; + nowEffect = effector.MergeEffects(context.Effect, session.PolicyEffects, null, session.PolicyIndex, session.PolicyCount, out int hitPolicyIndex); if (context.Explain && hitPolicyIndex is not -1) { context.Explanations.Add(policyValues); } - if (nowResult is not null) + if (nowEffect is not PolicyEffect.Indeterminate) { - return true; + session.DetermineResult(nowEffect.ToNullableBool() ?? false); + return session; } - nowResult = false; - return false; + session.EnforceResult = false; + return session; } - private static bool HandleExpressionResult(in EnforceContext context, bool expressionResult, IChainEffector chainEffector, - IReadOnlyList policyValues, int policyCount, out bool? nowResult) + private static EnforceSession HandleExpressionResult(in EnforceContext context, ref EnforceSession session, IChainEffector chainEffector) { PolicyEffect nowEffect; - if (policyCount is 0) + if (session.PolicyCount is 0) { - nowEffect = GetEffect(expressionResult); + nowEffect = GetEffect(session.ExpressionResult); if (chainEffector.TryChain(nowEffect)) { - nowResult = chainEffector.Result; - return true; + session.DetermineResult(chainEffector.Result); + return session; } - nowResult = false; - return true; + session.DetermineResult(false); + return session; } - nowEffect = GetEffect(expressionResult); + IReadOnlyList policyValues = session.PolicyValues; + nowEffect = GetEffect(session.ExpressionResult); if (nowEffect is not PolicyEffect.Indeterminate && context.PolicyAssertion.TryGetTokenIndex("eft", out int index)) { @@ -374,12 +408,12 @@ private static bool HandleExpressionResult(in EnforceContext context, bool expre if (chainResult is false || chainEffector.CanChain is false) { - nowResult = chainEffector.Result; - return true; + session.DetermineResult(chainEffector.Result); + return session; } - nowResult = chainEffector.Result; - return false; + session.EnforceResult = chainEffector.Result; + return session; } #if !NET45 diff --git a/NetCasbin/Evaluation/EffiectEvaluation.cs b/NetCasbin/Evaluation/EffiectEvaluation.cs index e4afa44c..0dc6d295 100644 --- a/NetCasbin/Evaluation/EffiectEvaluation.cs +++ b/NetCasbin/Evaluation/EffiectEvaluation.cs @@ -64,6 +64,20 @@ internal static bool TryEvaluate(PolicyEffect effect, EffectExpressionType effec } break; + case EffectExpressionType.PriorityDenyOverride: + switch (effect) + { + case PolicyEffect.Allow: + result = true; + hitPolicy = true; + return false; + case PolicyEffect.Deny: + result = false; + hitPolicy = true; + return true; + } + break; + case EffectExpressionType.Custom: // TODO: Support custom policy effect. break; diff --git a/NetCasbin/Evaluation/ExpressionHandler.cs b/NetCasbin/Evaluation/ExpressionHandler.cs index 37032dc4..9bad86ff 100644 --- a/NetCasbin/Evaluation/ExpressionHandler.cs +++ b/NetCasbin/Evaluation/ExpressionHandler.cs @@ -51,8 +51,7 @@ public void SetGFunctions() SetGFunctions(interpreter); } - - public void SetEnforceContext(ref EnforceContext context) + public void SetEnforceContext(in EnforceContext context) { EnforceContext = context; int parametersCount = 0; diff --git a/NetCasbin/PermConstants.cs b/NetCasbin/PermConstants.cs index 2cf8146c..4cf1b8bf 100644 --- a/NetCasbin/PermConstants.cs +++ b/NetCasbin/PermConstants.cs @@ -60,6 +60,7 @@ public static class PolicyEffect public const string DenyOverride = "!some(where (p_eft == deny))"; public const string AllowAndDeny = "some(where (p_eft == allow)) && !some(where (p_eft == deny))"; public const string Priority = "priority(p_eft) || deny"; + public const string PriorityDenyOverride = "priority(p_eft) && !some(where (p_eft == deny))"; } } } diff --git a/NetCasbin/ThrowHelper.cs b/NetCasbin/ThrowHelper.cs index 9b214a65..871efb20 100644 --- a/NetCasbin/ThrowHelper.cs +++ b/NetCasbin/ThrowHelper.cs @@ -9,5 +9,8 @@ internal static void ThrowNameNotFoundException() internal static void ThrowOneOfNamesNotFoundException() => throw new ArgumentException("The name1 or name2 parameter does not exist."); + + internal static void ThrowNotSupportException(string message) + => throw new NotSupportedException(message); } }