[User Accounts] Fix password bugs: "confirm password" must match; password cannot be email #5426
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johnsaigle
added
Bug
| @@ -1081,19 +1088,25 @@ class Edit_User extends \NDB_Form | |||
| = 'Please specify password or click Generate new password'; | |||
| } | |||
| } | |||
| // Ensure that the password and confirm password fields match. | |||
| // TODO This validation should be done on the front-end instead. | |||
There was a problem hiding this comment.
Suggested change
| // TODO This validation should be done on the front-end instead. | |
| // TODO This validation should also be done on the front-end. |
Never rely to front-end check!
There was a problem hiding this comment.
I don't think it's really an issue in this case. It's just a double-check so that someone doesn't make a typo while entering their password. Makes more sense to do that on the front-end and prevent the request from going through if they made a mistake.
| $plaintext = $values['Password_hash']; | ||
|
|
||
| // Ensure that the password and confirm password fields match. | ||
| // TODO This validation should be done on the front-end instead. |
There was a problem hiding this comment.
Suggested change
| // TODO This validation should be done on the front-end instead. | |
| // TODO This validation should also be done on the front-end. |
|
This should go to the release branch.. we want it fixed for 22.0, not 23.0, don't we? |
|
I've rebased it to |
…n error when they did not match
driusan
added
the
Critical to release
johnsaigle
changed the title
|
Also fixes #5439 |
|
I tested this locally and it worked but it wasn't manually tested. Hopefully any potential errors will come up during the next testing round |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Brief summary of changes
Password and Confirm Password on the front-end don't need to match on major. This check was removed by mistake during Password refactoring.
Testing instructions (if applicable)
Links to related tickets (GitHub, Redmine, ...)