casbin / node-casbin

An authorization library that supports access control models like ACL, RBAC, ABAC in Node.js and Browser
https://casbin.org
Apache License 2.0
2.59k stars 216 forks source link

coreEnforcer.buildRoleLinksInternal calls model.buildRoleLinks(rmMap) on values of rmMap #331

Open richard-sim opened 2 years ago

richard-sim commented 2 years ago

https://github.com/casbin/node-casbin/blob/f859d105b31668a93f965f1e2d6c3c5cb7f22f3d/src/coreEnforcer.ts#L367

Due to a refactor ~6 months ago, model.buildRoleLinks(rmMap) is called within a loop over rmMap.values() in coreEnforcer.buildRoleLinksInternal(). Not only is this inefficient, but if rmMap is empty then model.buildRoleLinks() will never be called.

casbin-bot commented 2 years ago

@Gabriel-403 @Zxilly @kingiw @nodece

Shivansh-yadav13 commented 2 years ago

@richard-sim do you have any suggestions on how it can be improved?

hsluoyz commented 2 years ago

@fabian4 @nodece

richard-sim commented 2 years ago

@Shivansh-yadav13: It's been a while since I opened this issue, but IIRC it should not be in the loop at all (as it was prior to the refactor that I referred to).

Shivansh-yadav13 commented 2 years ago

Not only is this inefficient, but if rmMap is empty then model.buildRoleLinks() will never be called.

but we will have Default Role Manager? if we use role definition in the model