Issue in update password From admin side and login ith same password

[ronakganatra]

Issue details:
-> login to admin area:
-> go to user tab
-> Click on your admin user
-> Go to password field click on generate password
-> Remove generated password
-> keep that password box empty or fill it with any number of space.
-> Click on update profile.
-> You will get a success message that your profile was successfully updated.
-> logout
-> But try using that same password or previous password you can't login.

[adamsilverstein]

Hi @ronakganatra - thanks for the bug report.

I ran thru these steps in mac/chrome dev channel and was able to reproduce by entering spaces into the field. if I leave the field blank, my password is unchanged. We probably shouldn't touch the password if your new password is all spaces, and when i tried logging in with the spaces as my password, it didn't work.

[1naveengiri]

Good Catch @ronakganatra Adding fix patch for it.

[ronakganatra]

Thnaks for appreciation
Replying to adamsilverstein:

Hi @ronakganatra - thanks for the bug report.

I ran thru these steps in mac/chrome dev channel and was able to reproduce by entering spaces into the field. if I leave the field blank, my password is unchanged. We probably shouldn't touch the password if your new password is all spaces, and when i tried logging in with the spaces as my password, it didn't work.

[adamsilverstein]

@1naveengiri Thanks for the patch! this looks like it will resolve the issue, I will give it a test.

Did you consider checking for empty vs. trimming? i guess trimming is better, spaces at the beginning or end of passwords are likely copy/paste or user entry errors.

I'm not sure we need the error message, would users get that when submitting a blank password field? they might expect empty to indicate no change (and not an error).

Additionally, it would be good to add a unit test here validating that a blank password will not be saved.

[1naveengiri]

@adamsilverstein trimming is better here, as you know spaces are not considered as empty. I have removed that error, your point seems meaningful to me here.

[1naveengiri]

@adamsilverstein we already have a unit test for empty password check.
​https://develop.svn.wordpress.org/trunk/tests/phpunit/tests/user.php
function test_edit_user_blank_pw()

[ronakganatra]

Perfect patch Naveen.

[ronakganatra]

As pe my testing this code is working fine.

[adamsilverstein]

@ronakganatra thanks for testing.

@1naveengiri Ah right - thanks for tracking down the function test_edit_user_blank_pw test, which seems like it should test for this. However, in this case we were sending not a blank password but one consisting only of spaces, right?

We can expand the unit test to catch that case so the test fails before this patch, but passes after it.

[ajensen]

I was able to reproduce the bug when entering spaces into the field. After applying the patch however I was able to log in succesfully. Although it still lets me update my profile with blank and spaces only passwords, I can only login with my original password. I also tested trimming after applying the patch and found it to work succesfully.

[oolleegg55]

Add unit test

[oolleegg55]

@adamsilverstein I added a patch containing unit test.
I am confused a little with file 42766.patch. It contains code which doesn't regard this ticket in my opinion. I didn't use the file 42766.patch for new file 42766.3.diff.

[afercia]

Related: #42852 which aims to make the interaction a bit more transparent to users.

[bookdude13]

Passwords with no length still don't allow for updating the profile (disabled button), but the issue remains for passwords just made of spaces. You can update the password to " " successfully, but then not log in with either " " or the old password.

Like @ajensen, after the patch I still see the following:

• When creating a user with two spaces as a password (and weak checkbox checked) I can click Add User, although I get the empty password error message which stops me from setting it
• When updating a user with two spaces as a password (and weak checkbox checked) I can click Update Profile and it will appear to be successful. However, when logging in I cannot use the two space password. The old password (the one before " ") works to log in.

Right now I think we need the profile update page to not show a success if the password ends up trimmed (and thus not updated). A more descriptive error message would be nice as well, since the password technically isn't empty for the user.

Note that I haven't checked out the unit test, since my current Windows environment isn't set up for that atm :)

[SergeyBiryukov]

#49433 was marked as a duplicate.

[bookdude13]

Design questions on this:

• Do we want a password of only spaces/space-like characters (e.g. tabs maybe if pasted in) to be allowed? Or should we forbid passwords being set if they would be "empty" after being trimmed?
• If the second case, then when creating the user, should the feedback to the user change at all, since they did enter a non-empty password and the error message would be "password empty"?
• Should the updating user page be able to update (e.g. click the button) if the only thing updated is the password, and it's an invalid password (e.g. will be trimmed to nothing, not updated)?

[nrqsnchz]

Discussed today during design triage meeting in Slack: ​https://wordpress.slack.com/archives/C02S78ZAL/p1581961322460700

Do we want a password of only spaces/space-like characters (e.g. tabs maybe if pasted in) to be allowed?

Not sure if this can be answered by the design team. This seems more like a security or compliance issue.

If the second case, then when creating the user, should the feedback to the user change at all, since they did enter a non-empty password and the error message would be "password empty"?

If the password is rejected because it contains empty spaces, and these are forbidden, then yes, the error messages should be clear and indicate why the password is being rejected.

Should the updating user page be able to update (e.g. click the button) if the only thing updated is the password, and it's an invalid password

I don't think so. If an error has occurred, this should be communicated to the user instead of falsely making them believe the operation was a success.

[afercia]

Related, for the UI part: #42749 and #42852.

[adamsilverstein]

Punting this to 5.5 as we aren't quite ready with a solution here.

[aristath]

@noudwordpress, @hansjovisyoast and myself tried tackling this one today.
After reading the comments above, we started wondering if the problem is the fact that the user can set a whilespace-only (spaces, tabs etc) password, or the fact that after setting it they can't login with it.
After a lot of backtracing and debugging we came to the conclusion that allowing whitespace-only characters as a password would introduce a lot more issues than it would solve. It is possible to allow them, but not without weakening security or implementing "hacky" solutions.

Do we want a password of only spaces/space-like characters (e.g. tabs maybe if pasted in) to be allowed?

According to our findings, though technically possible, it would be inadvisable to allow whitespaces-only passwords.

[paaljoachim]

We looked at this during Monday's Design triage and agreed on that this ticket does not need design. I am thinking that the password field needs to limit what can be entered.

[hellofromTonya]

Per @paaljoachim comment above, this ticket does not need design.

We looked at this during Monday's Design triage and agreed on that this ticket does not need design.

[adamsilverstein]

This is pretty close, although after the last patch some additional concerns were raised. I think we can address these by adding the same 'trim' logic for passwords that are all spaces to the JavaScript side, so we can make the action button disabled (just like when the field is blank). I'll plan to get back to this soon unless someone else beats me to it :)

Trac ticket: https://core.trac.wordpress.org/ticket/42766

[adamsilverstein]

In 42766.4.diff​:

• Disable submit button when trimmed password empty.

The Submit button now remains disabled if the password field contains only spaces.

[adamsilverstein]

42766.5.diff​ - fixes for php linter.

[adamsilverstein]

In 49118:

Users: prevent saving empty passwords, trim space from password ends on save.

Fix an issue where users could save a password with only spaces, or spaces at the beginning or end of their password, preventing them from logging in.

Props ronakganatra, 1naveengiri, ajensen, oolleegg55, bookdude13, nrqsnchz, aristath.
Fixes #42766.

[SergeyBiryukov]

In 49126:

Tests: Use assertSame() in test_edit_user_blank_password(), for consistency with other assertions.

Follow-up to [49118].

See #42766, #38266.

[prbot]

​dream-encode commented on ​PR #583:

Merged into WP Core in https://core.trac.wordpress.org/changeset/49118