casbin / Casbin.NET

An authorization library that supports access control models like ACL, RBAC, ABAC in .NET (C#)
https://casbin.org
Apache License 2.0
1.13k stars 110 forks source link

ParseException when pass a `RequestValue` to `Enforce` method #302

Closed Aya0wind closed 1 year ago

Aya0wind commented 1 year ago

I need to pass request attribute as a dynamic length array, so I'm trying to use a list of string to construct a RequestValue, and pass it as the second parameter of this method. public bool Enforce<TRequest>(EnforceContext context, TRequest requestValues) where TRequest : IRequestValues;
and it throws a ParseException with reason: No property or field 'Value1' exists in type 'RequestValues' (at index 14) And if i use Request<string,string,string>, this case runs ok. stack backtrace:

Unhandled exception. DynamicExpresso.Exceptions.ParseException: No property or field 'Value1' exists in type 'RequestValues' (at index 14).
   at DynamicExpresso.Parsing.Parser.GeneratePropertyOrFieldExpression(Type type, Expression instance, Int32 errorPos, String propertyOrFieldName)
   at DynamicExpresso.Parsing.Parser.ParseMemberAccess(Type type, Expression instance)
   at DynamicExpresso.Parsing.Parser.ParseMemberAccess(Expression instance)
   at DynamicExpresso.Parsing.Parser.ParsePrimary()
   at DynamicExpresso.Parsing.Parser.ParseUnary()
   at DynamicExpresso.Parsing.Parser.ParseMultiplicative()
   at DynamicExpresso.Parsing.Parser.ParseAdditive()
   at DynamicExpresso.Parsing.Parser.ParseShift()
   at DynamicExpresso.Parsing.Parser.ParseComparison()
   at DynamicExpresso.Parsing.Parser.ParseLogicalAnd()
   at DynamicExpresso.Parsing.Parser.ParseLogicalXor()
   at DynamicExpresso.Parsing.Parser.ParseLogicalOr()
   at DynamicExpresso.Parsing.Parser.ParseConditionalAnd()
   at DynamicExpresso.Parsing.Parser.ParseConditionalOr()
   at DynamicExpresso.Parsing.Parser.ParseConditional()
   at DynamicExpresso.Parsing.Parser.ParseAssignment()
   at DynamicExpresso.Parsing.Parser.ParseExpressionSegment()
   at DynamicExpresso.Parsing.Parser.ParseExpressionSegment(Type returnType)
   at DynamicExpresso.Parsing.Parser.Parse()
   at DynamicExpresso.Parsing.Parser.Parse(ParserArguments arguments)
   at DynamicExpresso.Interpreter.ParseAsLambda(String expressionText, Type expressionType, Parameter[] parameters)
   at DynamicExpresso.Interpreter.ParseAs(Type delegateType, String expressionText, String[] parametersNames)
   at DynamicExpresso.Interpreter.ParseAs[TDelegate](String expressionText, String[] parametersNames)
   at DynamicExpresso.Interpreter.ParseAsDelegate[TDelegate](String expressionText, String[] parametersNames)
   at Casbin.Evaluation.ExpressionHandler.CompileExpression[TRequest,TPolicy](EnforceContext& context, String expressionString)
   at Casbin.Evaluation.ExpressionHandler.Invoke[TRequest,TPolicy](EnforceContext& context, String expressionString, TRequest& request, TPolicy& policy)
   at Casbin.Evaluation.ExpressionHandler.Casbin.Evaluation.IExpressionHandler.Invoke[TRequest,TPolicy](EnforceContext& context, String expressionString, TRequest& request, TPolicy& policy)
   at Casbin.Enforcer.InternalEnforce[TRequest,TPolicy](EnforceContext& context, TRequest& requestValues)
   at Casbin.Enforcer.InternalEnforce[TRequest](EnforceContext& context, IPolicyManager& policyManager, TRequest& requestValues)
   at Casbin.Enforcer.Enforce[TRequest](EnforceContext context, TRequest requestValues)
   at Program.<Main>$(String[] args) in /Users/li/RiderProjects/DataManager/PolicyEngine/Program.cs:line 24

Full code here:

var model = DefaultModel.Create();
model.AddDef("r", "r", "sub, obj, act");
model.AddDef("p", "p", "sub, obj, act");
model.AddDef("e", "e", "some(where (p.eft == allow))");
model.AddDef("m", "m", "p.sub == r.sub && r.obj == p.obj && r.act == p.act");
var enforcer = DefaultEnforcer.Create(model);
FileAdapter adapter = new FileAdapter("test_policy.csv");
enforcer.SetAdapter(adapter);
enforcer.LoadPolicy();
// var request = Request.Create("alice", "data", "read");  //use this is ok
var request = new RequestValues(new []{"alice", "data", "read"}); // use this will cause ParseException
var result = enforcer.Enforce(enforcer.CreateContext(), request);
Console.WriteLine(result);

test_policy.csv:

p,alice,data,read

Did I do something wrong?

casbin-bot commented 1 year ago

@sagilio @sociometry @AsakusaRinne

AsakusaRinne commented 1 year ago

Could you please provide the version you used?

Aya0wind commented 1 year ago

Could you please provide the version you used?

casbin version: 2.0.0-preview.4 .NET version: 7.0.0-rc.2 platform: apple m1 macos 13.0

AsakusaRinne commented 1 year ago

Yes, in 2.0.0-preview.4 please use Request.Create("alice", "data", "read").

I confess it may be confusing due to some early designs. The code above returns the type Request<string, string, string>, while the other way you used has a return type of RequestValues. This difference will raise the error when enforcing.

Thank you for telling us and we'll optimize it in the next release. The current code has removed the constructor with parameters of RequestValues.

AsakusaRinne commented 1 year ago

@sagilio I noticed that the constructor of RequestValues may not need to be exposed to users (no such using in current unit test code). Shall we further hide the default constructor as internal?

Aya0wind commented 1 year ago

Yes, in 2.0.0-preview.4 please use Request.Create("alice", "data", "read").

I confess it may be confusing due to some early designs. The code above returns the type Request<string, string, string>, while the other way you used has a return type of RequestValues. This difference will raise the error when enforcing.

Thank you for telling us and we'll optimize it in the next release. The current code has removed the constructor with parameters of RequestValues.

Ok, Thanks for your help. By the way, if i really want to pass an array of object as a request, is there a best practice?
In 2.00-preview.1, this code can run correctly, because Enforce has a overloaded version which is
bool Enforce(params object[] requestValues);

var request = new Object[] {"alice", "data", "read"};
var result = enforcer.Enforce(request);

But in 2.00-preview.4, this method is disappeared, if i still use this code, it will call this unexpected
public static bool Enforce<T>(this IEnforcer enforcer, T value) overloaded version and cause error. Is there any way to pass request from a List<T>, T[],or other IEnumerable<T> types instead of use a fixed amount of parameters version such as
public static bool Enforce<T1, T2, T3>(this IEnforcer enforcer, T1 value1, T2 value2, T3 value3) in 2.0.0-preview.4?

AsakusaRinne commented 1 year ago

By the way, if i really want to pass an array of object as a request, is there a best practice?

I'm afraid not. I did not take part in the development of 2.00-preview.1 but I believe it removed such API later on account of performance. @sagilio What about adding an API like bool Enforce<T>(params T[] requestValues)?

sagilio commented 1 year ago

By the way, if i really want to pass an array of object as a request, is there a best practice?

I'm afraid not. I did not take part in the development of 2.00-preview.1 but I believe it removed such API later on account of performance. @sagilio What about adding an API like bool Enforce<T>(params T[] requestValues)?

The reason for the remove bool Enforce(params object[] requestValues); is because of the problem of calling priority. It may helpful to add the generic list API.

hsluoyz commented 1 year ago

Fixed by: https://github.com/casbin/Casbin.NET/pull/303