fix(email): on password reset, hash email with the emailToHashWith
value
#6579
fix(email): on password reset, hash email with the emailToHashWith
value
#6579
Conversation
cd70117
to
fbb6e39
Compare
* @returns {Promise} - resolves when complete | ||
*/ | ||
completePasswordReset (password, token, code, relier) { | ||
completePasswordReset (password, token, code, relier, emailToHashWith) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge fan of this variable name, but I used it in other parts of the code..so I'll be consistent for time being.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for consistency, at least until we get around to a cleanup like https://github.com/mozilla/fxa-auth-server/issues/2344
@@ -1173,6 +1174,47 @@ define(function (require, exports, module) { | |||
}); | |||
}); | |||
|
|||
describe('completeAccountPasswordResetWithRecoveryKey', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed I didn't add a test for this before.
@mozilla/fxa-devs I think this is ready for r! |
Looks like email timeout errors, will restart in a bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+, LGTM, thanks @vbudhram!
Verified as fixed on Train 122. |
Fixes #6571
After some investigating and head scratching, it turns out that the password reset is not using the correct email address to hash the password when the password is reset. I am not sure when this got regressed but suspect it might have been with account recovery.
This PR pdates the
completePasswordReset
andresetPasswordWithRecoveryKey
functions to correctly pass the email that should be hashed with password (emailToHashWith
). This value ensures that passwords are always hashed with the original email the account was created with.Unfortunately, with this bug, we can't have that guarantee anymore because there might be some users that reset their passwords (after performing a primary email change), so their password would be hashed with the new email.
In practice, I don't believe this is an issue because the auth-server will only compare the
authPW
that the content-server passes to it.