Skip to content
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: flask_login should be vendored into flask_security and removed as a dependency #385

Closed
avilaton opened this issue Sep 7, 2020 · 7 comments

Comments

@avilaton
Copy link

avilaton commented Sep 7, 2020

On my work we have had more than a few issues with both packages and I would like to share with you that I think keeping flask_login as a dependency is not in the best interest of clarity. Both project define UserMixin, and so much of the functionality is either shared or overridden by flask_security that it has lead, at least us, to confusion.
I have also seen the need for hacks inside the flask_security code to work around issues in flask_login like for example

# N.B. this isn't great Flask-Login 0.5.0 made this protected

So I propose flask_login to be cloned inside flask_security, and slowly disentangle those issues making it safer to upgrade flask_security.
I would like to add that this is a proposal and not a complaint.

@avilaton avilaton changed the title Proposal> suggest flask_login should be vendored into flask_security and removed as a dependency Proposal: flask_login should be vendored into flask_security and removed as a dependency Sep 7, 2020
@jwag956
Copy link
Member

jwag956 commented Sep 7, 2020

Thanks for your insight - hadn't really thought about that - Flask-Login is mostly non-maintained - but not abandoned (yet).

I have been trying to reduce dependencies - so an interesting direction...

@jwag956
Copy link
Member

jwag956 commented Oct 28, 2020

The more I read - 'vendoring' seems an outdated concept - possibly fixing compatible versions?

@avilaton
Copy link
Author

A total headache to have such an important piece of the functionality in this package reside in another repository, having a different release cycle and maintainers. It is also a small enough to bring it in without much trouble, maybe.

@avilaton
Copy link
Author

Same as it happened to the origin of this project, flask login might slowly fall into oblivion and then the whole thing needs to be forked. We have burned ourselves already, I want to prevent that from happening again.

@jwag956
Copy link
Member

jwag956 commented Oct 28, 2020

I hear you - and had the same thoughts - and then they came out and updated it. If it does continue to fall into oblivion I will certainly pull that in. I guess my question is - is there anything different between placing a fixed version constraint in FS requirements and actually copying the code?

@hrishikeshrt
Copy link
Contributor

A potential difference between placing a fixed version constraint and copying the code is as follows,
If a vulnerability in the existing version gets discovered, and flask-login is no longer being maintained, that vulnerability now becomes part of this project as well and we would have to copy-fix or wait till someone else comes and fixes flask-login.

While, if it is part of this code-base, the "discovered" vulnerability will also be part of this project, and procedure of fixing it would be much more natural. All in all, it comes down to "trust" in flask-login

@jwag956
Copy link
Member

jwag956 commented Aug 28, 2021

Thanks for your comments. I reached out to Max Countryman and offered to either share or take over flask_login - he respectfully declined - however he has done a release when required - so I consider flask_login at least minimally maintained. So for right now I am going to close this and we'll see if we run into issues again.

@jwag956 jwag956 closed this as completed Aug 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants