casbin / jcasbin

An authorization library that supports access control models like ACL, RBAC, ABAC in Java
https://casbin.org
Apache License 2.0
2.38k stars 461 forks source link

eval() doesn't work for java.util functions, need to switch from bsh to aviator #183

Closed rachnaaggarwal closed 3 years ago

rachnaaggarwal commented 3 years ago

Hello, I am trying to do a POC on using ABAC via "jcasbin" and I am facing an issue related to the "eval" function. The eval function in the jcasbin library prior to the 1.7.1 version allowed using java expressions whereas a change was added in version 1.7.1 (https://github.com/casbin/jcasbin/compare/v1.7.0...v1.7.1) which broke this functionality. When I used "casbin-spring-boot-starter" dependency, the jcasbin library included was of version 1.6.4 and I was able to evaluate the following policy definition:

"\"test\".equals(r.sub) && java.util.regex.Pattern.compile(\"/*/url1/url2/*\", java.util.regex.Pattern.CASE_INSENSITIVE).matcher(r.obj).find()","r.obj","PUT"

with the below model definition:

[request_definition]
r = sub, obj, act

[policy_definition]
p = sub_rule, obj, act

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = eval(p.sub_rule) && r.act == p.act

Please note that I had to use the enforcer.addPolicy() method for adding the above policy definition because the csv parser with the file adapter was splitting the definition with comma. Then I found that the comma issue was fixed in version 1.8.1 (https://github.com/casbin/jcasbin/issues/158). So, I upgraded the jcasbin library to 1.8.1 but then the above stopped working because the eval function is changing the above expression to:

"test1".getEquals(r_sub)() && java.getUtil().getRegex().getgetPattern()().getCompile("/*/approve/expense/*",() java.getUtil().getRegex().getgetPattern()().getCASE_INSENSITIVE)().getMatcher(r_obj)().find()

i.e., its appending get for all the attributes and is not working as java expression.

I know that we can use functions (and even write our own customized function) in the model definition. But my requirement is to define a function in the policy definition. The reason being, I don't want to use the function for all the use cases and so the use case for which I need, I can define that in the policy definition.

Could you please suggest how can we achieve that?

hsluoyz commented 3 years ago

@shy1st @shink

shy1st commented 3 years ago

Do you mind providing entity class code related to it? This may be more convenient to debug for me. @rachnaaggarwal

hsluoyz commented 3 years ago

@shy1st I think @rachnaaggarwal means:

\"test\".equals(r.sub) && java.util.regex.Pattern.compile(\"/*/url1/url2/*\", java.util.regex.Pattern.CASE_INSENSITIVE).matcher(r.obj).find()

will be processed into:

"test1".getEquals(r_sub)() && java.getUtil().getRegex().getgetPattern()().getCompile("/*/approve/expense/*",() java.getUtil().getRegex().getgetPattern()().getCASE_INSENSITIVE)().getMatcher(r_obj)().find()

in latest jCasbin. You can see a lot of getXxx() there. I guess some of our escaping rules are functioning wrongly, so abc() is changed to getAbc(). Can you check this bug?

rachnaaggarwal commented 3 years ago

@shy1st Please find attached the zip file "abacPOCWithCasbin.zip" for the Spring Boot POC using jcasbin. The build.gradle in the zip includes the jcasbin 1.6.4 library since it defaults to the one included by casbin-spring-boot-starter. When you will run the application and will access the following "GET" URL: http://localhost:8080/test/approve/expense/2?subject=test1

You would see the result as "Authorized".

If you will now replace the build.gradle in the zip with the attached "build.gradle" file from the "build.gradle.zip" which includes the jcasbin 1.8.1 version, and will run the application and access the above URL, you will get the error and "Not Authorized".

abacPOCWithCasbin.zip build.gradle.zip

Please let me know if any further information is required.

Regards, Rachna

hsluoyz commented 3 years ago

@shy1st plz take a look.

rachnaaggarwal commented 3 years ago

I wanted to add here that I have created another issue (#184) for the original issue that I was facing with CSV parsing since I thought they are separate ones.

hsluoyz commented 3 years ago

@shink

shink commented 3 years ago

@shink

I'll do it.

shink commented 3 years ago

@hsluoyz @shy1st @rachnaaggarwal

I think this problem is caused by the pattern. Through this pattern, equals, util, regex etc. are recognized as attributes and changed to getXXX().

For example, we need convert user.age to user.getAge() instead of converting java.util to java.getUtil(), so we need distinguish between user and java. But it is impossible to exhaust all situations like java.util. Do you have any good suggestions?

hsluoyz commented 3 years ago

@shink it's not hard to solve I think. You can first check if it's starting with with policy or request definition. For example, r.sub.age should be converted to r.sub.getAge(), because you know we have the following in model:

[request_definition]
r = sub, obj, act

Similar for p.sub.xxx().

If it's not the case, then it's Java's package path.

shink commented 3 years ago

@hsluoyz Got it! I will do it.

hsluoyz commented 3 years ago

We introduced eval() in PR: https://github.com/casbin/jcasbin/pull/85 , but it used bsh. But then we switched to janino. Now we decided to use aviator, just like how matcher is evaluated.

shink commented 3 years ago

We fixed this bug and implemented the eval function using aviator. However, aviator is powerful but does not support executing non-static Java methods.

System.out.println(AviatorEvaluator.execute("Integer.toString(10)"));    // 10

// an exception (com.googlecode.aviator.exception.ExpressionSyntaxErrorException) will be thrown because the matches and find functions are not static
System.out.println(AviatorEvaluator.execute("Pattern.compile(\"/*/url1/url2/*\", Pattern.CASE_INSENSITIVE).matcher(\"/test/url1/url2/2\").find()"));

So, I think we can add support for custom function, then the complex policy can be handled by function customed by user. What do you think? @hsluoyz @rachnaaggarwal

rachnaaggarwal commented 3 years ago

@shink To clarify, have you added the support for executing a custom function (including java functions) from within a policy, or have you added the support for executing java functions from within eval?

So, for example, can we now do the following from either eval or within policy?

java.util.regex.Pattern.compile(\"//url1/url2/\", java.util.regex.Pattern.CASE_INSENSITIVE).matcher(r.obj).find()

shink commented 3 years ago

@rachnaaggarwal

Now, I have added support for custom function. You can take a look at this PR.

Unfortunately, in this PR, the java code in policies will no longer be supported. Because I don't think there should be verbose java code in policies.

However, you can create a custom function in this way and use it in the policy, which is more powerful and recommended. But in this way, you need to modify your previous policy. Looking forward to any your suggestions.

hsluoyz commented 3 years ago

@rachnaaggarwal actually java.util.regex.Pattern.compile("//url1/url2/", java.util.regex.Pattern.CASE_INSENSITIVE).matcher(r.obj).find() this code is so tedious and it is not supposed to be Casbin grammar. And it will not be supported by other Casbin implementations. If we can support it, that's fine. But it's totally fine I think if we don't support it.

rachnaaggarwal commented 3 years ago

Got it, thank you!