pallets-eco / flask-security

Quick and simple security for Flask applications
MIT License
649 stars 154 forks source link

Changing email address #956

Closed sr-verde closed 6 months ago

sr-verde commented 8 months ago

Hey,

I am currently rewriting my edit user view. Until now, when a user changes their email address, I’ll update the mail address, deactivate the user, remove the confirmed_at value from the database and send the confirmation link (using send_confirmation_instructions). The problem that I have is: When a user misspells his email address, he will never be able to login again (there is no recovery than manual intervention). I know that some frameworks handle this by not changing the email address until the new email address is confirmed.

So, I have a feature request: Is it possible and desirable to have a method in UserDatastore that handles this? It's not critical as I've never experienced this before, but it just came to mind.

Best :v:

stfnx commented 8 months ago

I hope this library will at some point provide a way to change an user's e-mail address.

I'm not sure how I should implement this by myself so that it works well with all the other logic from this library. I'm certainly sure I'd do something wrong - and this could be a security issue.

edit: I think there's already an issue for this: https://github.com/Flask-Middleware/flask-security/issues/317

jwag956 commented 8 months ago

dup of #317 I have always been reluctant to add 'admin' features like this - but I see the value. I am not certain it is hard - though as mentioned - need to be mindful of security issues.

stfnx commented 8 months ago

I see it exactly as you do. I would also refrain from integrating general admin stuff into this library. Rather focus on a core competence and do it well. (As you already do)

But I would exclude changing the e-mail and username from this. Like the password, these two pieces of data are an integral part of the registration/login process and are already handled by the library anyway.

Changing the password is already possible, so why not also allow this for e-mail and username?

And I think it would be simple to add aswell. All the pieces like validation, sending confirmation mail etc. are already here. They just need to get wired together in a new route.

But if you are strictly against adding this functionality, what about adding a section about this to the documentation? A section that explains how to properly implement this.

If this is wanted (either implementation or documentation) and someone could give me a quick rundown which steps are required, Im willing to look into it and if I manage to implement it, create a pull request.

sr-verde commented 8 months ago

dup of #317 I have always been reluctant to add 'admin' features like this - but I see the value

Ah, sorry. I agree to your point with 'admin' features. My problem is that Flask Security offers email confirmation but I can't use the current interface (without doing horrible things :wink:) to confirm mail addresses that haven't been written to the email field already.

Should we close this issue in favor of #317?

stfnx commented 8 months ago

@sr-verde would you mind sharing your route code for changing e-mail addresses?