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

Question about "hasRole" method when using regex matching function #270

Closed JCereal closed 2 years ago

JCereal commented 2 years ago

Hi there,

I have a question about the "hasRole" method implementation here: https://github.com/casbin/jcasbin/blob/master/src/main/java/org/casbin/jcasbin/rbac/DomainRoles.java#L37-L43

Here is the example to cause this issue:

  1. Create an enforcer with regex matching function: enforcer.setRoleManager("g", new DefaultRoleManager(10, BuiltInFunctions::regexMatch, null));

  2. Add a "g" policy like: (g, ^E\d+$, role::employee), which is intended for all the users with id Exxxx have employee role.

  3. If I call the method enforcer.getRolesForUser("^E\\d+$");, it will return an empty list due to: https://github.com/casbin/jcasbin/blob/master/src/main/java/org/casbin/jcasbin/rbac/DefaultRoleManager.java#L232-L234 is using hasRole, which testifies the string ^E\d+$ against the regex expression ^E\d+$ (itself) and returns false.

  4. This issue also affects the enforcer.getImplicitRolesForUser() method. Assuming we have (g, ^E\d+$, role::employee) and (g, role::employee, role::all_user) and call enforcer.getImplicitRolesForUser("E2345"). The BFS search will terminate early due to cannot get role for ^E\d+$.

The root cause is the regex expression itself cannot pass the regex matching function test.

Any suggestions on this issue?

One idea from my side is, for https://github.com/casbin/jcasbin/blob/master/src/main/java/org/casbin/jcasbin/rbac/DomainRoles.java#L39, not only use the matching function, but also returns true if name==r. Since this is quite a fundamental method, I am not sure how the impact would be.

Thanks for your help in advance.

casbin-bot commented 2 years ago

@tangyang9464 @seriouszyx @elfisworking @fangzhengjin

hsluoyz commented 2 years ago

@tangyang9464

tangyang9464 commented 2 years ago

@JCereal You are right. Could you make a PR for this?

JCereal commented 2 years ago

@tangyang9464 Please see #274