flask-login

flask-login copied to clipboard

Currently, login_manager.session_protection is only considered if app.config does not have the SESSION_PROTECTION key. Session protection is the only config that works this way, and this is not a standard pattern for Flask config. A more common pattern would be to use the attribute if the config is set to some default value, but that doesn't work in this case because None has meaning (probably should be False instead).

As part of #696, I also wanted to set all available config to default values in init_app rather than using config.get with defaults spread out across the code. However, this doesn't work for session protection.

At minimum, I think init_app should do app.config.setdefault("SESSION_PROTECTION", self.session_protection). Requiring the extension to be configured before registering it on the app is a standard pattern in Flask, as you can't guarantee in general that config changed afterwards will take effect anyway.

An extra step would be to deprecate then remove the attribute and only use the config. I think this makes sense because session protection is very dependent on how the app is deployed, which is what app.config is for.

Somewhat interesting, the tests already never set the attribute, they always set the config.

Considering it again, I think it still makes sense to configure this at the extension level. Then when init_app is called, set the value in app.config with the attribute as a default, and from then on read app.config only. In other words, do what I described first, not the second part about deprecating it.