New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal - change Security object to accept instances instead of classes #667
Comments
Thanks for a detailed and interesting concept. Currently, there are 2 types of classes that can be sent to FS - the forms, and the utility classes (mail_util, password_util, webauthn_util, username_util). Those second set - utility classes seem very easy and a great idea to accept an instance (as well as a class), Not sure you would consider it 'clean' but of course you always could place services like that on the app context and be able to retrieve them from anywhere (e.g current_user, etc). Definitely need to think on this some more. |
Thank you for taking time to answer this
Aye, writing this out this proposal I also thought WTForms isn't aligned well with it :/
It's true this can be done. I don't really think it is clean, because it means I can't test the validation class in isolation without constructing objects global to logic I'm trying to test, but I appreciate it achieves the job without forcing a huge redesign. For me the goal in writing out this issue was to word what I think would make Flask Security Too easier to use in complex cases, and see if others agree. I understand the consensus might be to just continue relying on existing globals based solutions because they won't require changes to the library. |
I have some ideas around this that I think are pretty simple - give me a few weeks to work them out. |
@kuba-lilz if you have a chance - please check out #688 for a proof of concept. I am pretty happy with the implementation - it is backwards compatible, but offers advanced applications an easy way to get in the middle of form instantiation. It also nicely removes the intimate knowledge of form classes from views. |
@jwag956 I tried putting code similar to tests/test_misc.py:: test_custom_form_instantiator2 into my code, but can't get around Csrf issue aside, I get the idea behind new design, and it allows me to implement the factory pattern passing my own data to forms that I was hoping for! |
Ahh yes - forms really really want a request context - I was hoping to get away with an app context - but no luck. The answer is simply to do
I have updated the PR and added a specific test for CSRF. |
Using |
Allow for applications to set their own form instantiation callback. This allows an application to have complete control over the methods and services that validation (and instantiation) requires. This also de-couples form classes from Flask-Security views - the default classes can be completely re-defined (although of course the views require certain attributes!). The unit tests show 2 possible patterns - an application form factory and a self-cloning form. closes #667
Allow for applications to set their own form instantiation callback. This allows an application to have complete control over the methods and services that validation (and instantiation) requires. This also de-couples form classes from Flask-Security views - the default classes can be completely re-defined (although of course the views require certain attributes!). The unit tests show 2 possible patterns - an application form factory and a self-cloning form. closes #667
Core Security object now accepts classes for various logic - registration, login, etc
Passing custom logic as classes means we don't have a chance to seed objects performing validation with arguments outside of what FlaskSecurity envisioned in constructors for base/reference classes.
Say that for whatever reason I need to run some logic against Redis service in my login form - maybe I need to check if user isn't logged already from too many other machines because I run Netlifx and people are trying to share their accounts.
I don't see any non-hackish way of injecting redis service instance to validation performed by MyCustomLoginForm now.
If instead of classes, Security object would accept instances that need to comply with expected interface (say have
validation(payload)
function), then I can easily seed my object with whatever arguments I need to do my complex custom logic that FlaskSecurity never envisioned:In short - if Flask Security was accepting objects instead of classes, we would have flexibility to construct objects as we want, which would help with implementing arbitrary validation logic. With current setup we don't have this ability.
Or to phrase it a bit different: Flask Security doesn't allow me to pass arbitrary data to my own validation logic. Well, why?
I understand this is quite a large change, but it's a pattern I see over and over in various frameworks - ones that expect classes to configure them limit users in what they can do in hooks, while ones that accept instance give users much more freedom for complex logic in hooks.
Does the proposal look reasonable to others?
Or maybe there is already a clean method to achieve what I'm trying to do here that I don't realize?
The text was updated successfully, but these errors were encountered: