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

Casbin comma handling over JDBC Adapter #413

Closed m-ignatov closed 1 month ago

m-ignatov commented 1 month ago

Hi,

I am using JCasbin 1.55.0 and JDBC Adapter 2.7.0 with the standard RBAC model:

# Request definition
[request_definition]
r = sub, obj, act

# Policy definition
[policy_definition]
p = sub, obj, act

[role_definition]
# user-group mapping
g = _, _

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

# Matchers
[matchers]
m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act

Our policies contain data which might include commas (,).

As far as I understand from the docs, I need to enclose the data with double quotes to escape it. However, it took me much trial and error to set this everywhere - previously I had parsing issues, coming mainly from Util.splitCommaDelimited() method.

I observed the issue when the policy is saved with quotes to the in-memory casbin model and to the DB. However, if the application is restarted and casbin reads again the policies from the DB, the quotes get stripped away. This caused issues when deleting policies for example due to string mismatch (comparing same strings, but one with quotes, other without).

What I have done so far is to extend the org.casbin.adapter.JDBCAdapter class and override the following methods by quoting the rule list strings before persisting to the DB:

For now it seems to work. So my question/proposal would be if this comma/special symbol handling could be extended to the JDBC adapter or to JCasbin. It would be great if JCasbin could automatically handle the commas, so that consumers don't have to implement custom logic like this.

Looking forward to your thoughts on this.

Thanks and regards, Momchil

casbin-bot commented 1 month ago

@tangyang9464 @imp2002

hsluoyz commented 1 month ago

@m-ignatov can you make a PR?

m-ignatov commented 1 month ago

Hi @hsluoyz,

I think the above mentioned PR covers the minimum that fixes the comma handling, tested it locally and works good. If this proposal is okay for you, I guess quoteRule() can be moved to the org.casbin.jcasbin.util.Util class as well for code consistency.

Regards, Momchil