Proposal - change Security object to accept instances instead of classes

[kuba-lilz]

Core Security object now accepts classes for various logic - registration, login, etc

flask_security.Security(
        app=application,
        datastore=user_datastore,
        register_form=MyCustomUserRegistrationForm,
        confirm_register_form=MyCustomConfirmUserRegistrationForm,
        login_form=MyCustomLoginForm)

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:

class MyCustomLoginForm

  def __init__(self, redis_service):

    self.redis_service = redis_service

  def validate(self, payload):

      # ...


flask_security.Security(
        app=application,
        datastore=user_datastore,
        ...
        login_form=MyCustomLoginForm(my_redis_service))

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?

[jwag956]

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), For the form classes - need to look into this more - from what I am aware, the accepted best-practice from WTForms is to instantiate the form on every request - which is what FS does in the view.

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.

[kuba-lilz]

Thank you for taking time to answer this

from what I am aware, the accepted best-practice from WTForms is to instantiate the form on every request

Aye, writing this out this proposal I also thought WTForms isn't aligned well with it :/

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).

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.

[jwag956]

I have some ideas around this that I think are pretty simple - give me a few weeks to work them out.

[jwag956]

@kuba-lilz if you have a chance - please check out #688 for a proof of concept. Not all forms have been converted - but key ones like login/register have been - the unit tests show a couple ways of working with this.

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.

[kuba-lilz]

@jwag956 Wow, thank you for putting this together!

I tried putting code similar to tests/test_misc.py:: test_custom_form_instantiator2 into my code, but can't get around RuntimeError: Working outside of request context. that happens in self.csrf_impl.generate_csrf_token. Happy to try again if you know how can I get around it.

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!

[jwag956]

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

with app.test_request_context():
  fi = MyLoginForm(xxxxx)

I have updated the PR and added a specific test for CSRF.

[kuba-lilz]

Using with app.test_request_context() I got my custom registration form builder to work :)