Skip to content
This repository has been archived by the owner on Feb 22, 2024. It is now read-only.

SQLAlchemyUserDatastore create_user creates user with plain text password #598

Closed
imaia opened this issue Feb 14, 2017 · 12 comments
Closed

Comments

@imaia
Copy link

imaia commented Feb 14, 2017

As in description. An call to create_user for SQLAlchemyUserDatastore creates a new user with password in plain text; regardless of config.

@jirikuncar
Copy link
Collaborator

@imaia you need to use flask_security.utils:encrypt_password if you create user manually.

--
see usage https://github.com/mattupstate/flask-security/search?utf8=%E2%9C%93&q=encrypt_password

@imaia
Copy link
Author

imaia commented Feb 14, 2017

Thanks for the suggestion, I was using encrypt_password to set the password. Wouldn't it make sense to set the password properly, when using create_user? An new user with "plaintext" password makes little use.

@kellyjanderson
Copy link

Binding the encryption algorithm to the call to create user would make transitioning to a new encryption algorithm difficult. It is best for the low level functions to be closer to CRUD and save what you pass in. For instance if you started out using SHA-1 and later decided to upgrade to HMAC-SHA-1, you would have to change the create_user in order to support it. The create_user should not be concerned about encryption algorithms, just creating a row in the user table.

@imaia
Copy link
Author

imaia commented Feb 15, 2017

@kellyjanderson There could be a encrypt_password method bound to the UserDatastore, were there further concerns. That encrypt_password method could be, then, overwritten on specific needs and create_user could then encrypt the password on call.

The create_user should not be concerned about encryption algorithms, just creating a row in the user table.

My concern is that create_user does not create "the right row". How useful is an user without a functional password value?

@jirikuncar
Copy link
Collaborator

@imaia can you maybe elaborate on why do you want to use the "low-level" model directly to create user? It might help us to find correct solution for this problem.

@italomaia
Copy link

I thought of user_datastore.create_user as a middle level method, as it prepares the arguments before persisting. Maybe it could also "prepare" the password, if provided, encrypting it properly. This behavior is most useful for user added through CLI (this is the problem that originated this issue).
Does it make sense?

@jirikuncar
Copy link
Collaborator

@italomaia there is an improved CLI in our fork (https://github.com/inveniosoftware/flask-security-fork/blob/master/flask_security/cli.py#L67) which is already released as Flask-Security-Fork==2.0.0.

@italomaia
Copy link

@jirikuncar That I did not know. I can live with that, thanks!
ps: when should I consider using the fork? It seems way healthier than the "mainstream" repo.

@jirikuncar
Copy link
Collaborator

@italomaia feel free to use it. We are running several production services on it without troubles 🤞 so far.

@italomaia
Copy link

By the way, is flask-security abandonware? @mattupstate

@jirikuncar
Copy link
Collaborator

@italomaia you last question is related to #559

@jirikuncar
Copy link
Collaborator

Binding hashing directly to datastore can bring problems when a migration needs to be done. If you know about any other elegant solution please propose a PR. Thank you!

jasco pushed a commit to jasco/flask-security that referenced this issue Oct 3, 2023
…lets-eco#598)

flask-mail has been abandoned - flask-mailman is a newer package that provides similar functionality to Django mail.

Use flask-mailmanas our default and in all the examples. flask-mail is still supported - and of course from a runtime perspective this all can be easily overwritten using the MailUtil class.

Most changes were around flask-mailman having a different way of capturing outbound emails during testing.

Closes: pallets-eco#531
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants