Keep email confirmation token on password update. #5951
Conversation
During `update_user_email` we have to clean up not only chagne email token, but also all tokens that were send to email previously. Such as "confirm" and "reset_password" tokens. This is because they were sent to previous email, which potentially can be compromised at this point, as it can be one of reasons to chagne email initially.
When user decides to change password, mail can still be waiting confirmation. Reason why user changes password is not important and does not lead to any security issue related to email confirmation. So related part of logic is the same for update and reset of password.
ShPakvel
changed the title
|
Thank you for the PR. Just one tiny correction:
I don't believe there is a security issue here because, when we reset and confirm password, we validate that user email matches the token email. This PR should be exclusively about improving the user experience by not deleting confirmation tokens. All other aspects are protected today. |
|
To be clear, I believe the PR should only change this line: https://github.com/phoenixframework/phoenix/pull/5951/files#diff-501abec1a66c387cce697c48f3c433e58c4fecd77bcd39765d0f3f982d96c39cL202 And update one of the existing tests. :) |
Previous email tokens are treated as invalid. Protected by email comparison in `verify_email_token_query`.
ShPakvel
changed the title
ShPakvel
changed the title
|
@josevalim, thanks! I see I missed protection we have by comparison of user emails in Regarding keeping email confirmation token. I think it should be done in both cases "password update" and "password reset". Removing "confirm" email token is inconvenience for user experience in both cases, and I don't see any security issue with keeping token. |
When user decides to change password, mail can still be waiting confirmation.
Reason why user changes password is not important and
does not lead to any security issue related to email confirmation.
Logic is the same for update and reset of password.