search

elasticio / casbin-mongoose-adapter

Mongoose adapter for Casbin
Apache License 2.0
15 stars 3 forks source link

FilteredAdapter should not dynamically change to ordinary Adapter #2

Open ulanmurzatayev opened 5 years ago

ulanmurzatayev commented 5 years ago

When creating a FilteredAdapter with MongooseAdapter.newFilteredAdapter(config.db) it is expected to stay as FilteredAdapter. However current behaviour is that it changes dynamically during execution https://github.com/elasticio/casbin-mongoose-adapter/blob/4a87b7ac0401e024016bee18145a0db9bddbda3a/src/adapter.js#L141

Now the problem is that when a filtered adapter is first created casbin internally calls loadPolicy(model) https://github.com/elasticio/casbin-mongoose-adapter/blob/4a87b7ac0401e024016bee18145a0db9bddbda3a/src/adapter.js#L126

which switches it back to an ordinary adapter. Then if a user calls enforcer.loadFilteredPolicy() casbin throws an error, as it does not recognize the adapter to be filtered anymore https://github.com/casbin/node-casbin/blob/40dd90a6dd7a023746bc3c15593d96d802c612ab/src/coreEnforcer.ts#L164

I think loadPolicy() should work without changing the adapter type.

ghaiklor commented 5 years ago

Strange, but MongoDB adapter for casbin allows to change it in runtime - https://github.com/casbin/mongodb-adapter/blob/master/adapter.go#L186-L190

Actually, these chunks of code you provided is a direct port from there.

ulanmurzatayev commented 5 years ago

well it could be that there is a bug in mongodb-adapter as well. @ghaiklor do you agree that this should not happen?

ghaiklor commented 5 years ago

after some investigation into internals - agreed.

we should set the filtered flag to true only when calling from newFilteredAdapter and do not change it in runtime.

If the adapter was created as filtered one, it should stay that way and otherwise.