WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 4 months ago

#42766 assigned defect (bug)

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

Reported by: ronakganatra Owned by: adamsilverstein
Milestone: Awaiting Review Priority: normal
Severity: normal Version: trunk
Component: Administration Keywords: good-first-bug has-patch has-unit-tests
Focuses: ui, javascript, administration, performance Cc:

Description

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.

Attachments (4)

42766.diff (1010 bytes) - added by 1naveengiri 6 months ago.
Good Catch @ronakganatra Adding fix patch for it.
42766.2.diff (539 bytes) - added by 1naveengiri 6 months ago.
@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.
42766.patch (3.6 KB) - added by ronakganatra 6 months ago.
As pe my testing this code is working fine.
42766.3.diff (1.8 KB) - added by oolleegg55 4 months ago.
Add unit test

Download all attachments as: .zip

Change History (16)

#1 follow-up: @adamsilverstein
6 months ago

  • Keywords needs-patch good-first-bug added
  • Owner set to adamsilverstein
  • Status changed from new to assigned

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.

Last edited 6 months ago by adamsilverstein (previous) (diff)

@1naveengiri
6 months ago

Good Catch @ronakganatra Adding fix patch for it.

#2 @1naveengiri
6 months ago

  • Keywords has-patch added; needs-patch removed

#3 in reply to: ↑ 1 @ronakganatra
6 months ago

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.

#4 @adamsilverstein
6 months ago

  • Keywords needs-unit-tests added

@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.

#5 @adamsilverstein
6 months ago

  • Keywords needs-testing added

@1naveengiri
6 months ago

@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.

#6 @1naveengiri
6 months ago

@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()

#7 @ronakganatra
6 months ago

Perfect patch Naveen.

@ronakganatra
6 months ago

As pe my testing this code is working fine.

#8 @adamsilverstein
6 months ago

  • Keywords needs-testing removed

@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.

#9 @ajensen
5 months ago

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
4 months ago

Add unit test

#10 @oolleegg55
4 months ago

  • Keywords has-unit-tests added; needs-unit-tests removed

@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.

This ticket was mentioned in Slack in #core by oolleegg55. View the logs.


4 months ago

#12 @afercia
4 months ago

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

Note: See TracTickets for help on using tickets.