flask-security
flask-security copied to clipboard
Flask-Middleware
Option to avoid unnecessary queries for roles/permissions on every request
The _on_identity_loaded function in flask_security.core makes one query for roles, and then another query for each role, to load permissions for that role. My app doesn't even use flask-security for the roles and permissions support, though we do have roles and permissions relationships on our User model (we have our own roles & permissions checks in place). It would be nice if flask-security provided a config option similar to SECURITY_JOIN_USER_ROLES that allowed us to skip that part of _on_identity_loaded and get a little performance boost. Really we don't use the provides/needs stuff at all, so would be happy to skip all of the identity.provides loading. Proposed config option: SECURITY_LOAD_PROVIDES, default True.
If you're ok with this idea (or some version of it) I'm happy to make a PR for it.
So if I understand - you have a 'roles' column/relationship in your UserModel - but it isn't being used or is your own implementation? It looks like on_identity_loaded() doesn't require that column (i.e. 'roles' are optional). Possibly it would be better to fix any issues with FS2 that don't allow for applications that don't want to use FS2's roles feature (not sure how hard that would be). I guess another way of asking this - if you changed your user column name to 'myroles' would this issue go away?
Correct. We have a roles relationship in our User model that we use ourselves, but we don't want flask-security to care about it. Changing the name of the relationship in our model does work, but feels a little hacky, which is why I initially didn't go down that road. I took another look after your nudge, and we only use it a few places in our flask app, so I'm checking with teammates now about whether they're ok with that change. (It's a little messier than it otherwise would be because our User model inherits from one that's shared with other projects, and we can't easily rename 'roles' in that parent class.) If my team is cool with renaming the relationship, I can just close this issue.
p.s. - I have another workaround too, monkey-patching _on_identity_loaded to remove the roles/permissions loading, but we'd really like to avoid that if at all possible.
p.p.s. - Thanks so much for the super quick response!!
So I am not inclined to change FS2 to avoid name collisions - since that basically would mean eventually someone would want every column to be optional - FS2 is pretty clear in the documentation that it relies on specific naming in User and Role models.
Depending on your CI - a slightly better (IMHO) way to patch would be to source patch on download of the FS2 package. Possibly using something like https://pypi.org/project/pypatch/
Makes sense. I was originally thinking we wouldn't be alone in wanting some parts of FS2, but not needing the roles/permissions features, but I guess maybe most people in that boat wouldn't have 'roles' relationship defined at all. In any case, we ended up renaming our 'roles' relationship to avoid the name collision so we're ok now. Thanks for the good discussion!