Editing user in Dashboard and using "Send Reset Link " broken by retrieve_password()

[boblindner]

If you change the email of a user created with the username of an email address you are unable to send a reset link because retrieve_password() in wp-includes/user.php mistakenly thinks the username is an email address for a user (because it contains an @) so you get:

Error: There is no account with that username or email address.

Steps to reproduce:

• make a user with the same username AND email address. e.g. foo(@)example.com
• edit that user and change the email address (username not editable) to the email address. e.g. bar(@)example.com and save user
• Try to use the “Send Reset Link” button while editing that user again (/wp-admin/user-edit.php)

I think this is happening because the call to retrieve_password() passes in the username and retrieve_password() mistakenly believes everything with an "@" in must be an email address.

This PR takes into account that username can have @ char.

For backward-compatibility, we are still sending the WP_Error with error message invalid_email if the passed user login has @ character and no user data was found.

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

This PR takes into account that username can have @ char.

For backward-compatibility, we are still sending the WP_Error with error message invalid_email if the passed user login has @ character and no user data was found.

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

[daxelrod]

Hello, I ran across this same issue. The ​referenced PR seems to have the right idea (check email, but fallback to username).

This is related to the wider issue #22367, however I do not believe the patch there addresses this issue.

[azouamauriac]

Hello please ignore the previous attachement, it was mistake. This is the fix i proposed.

[azouamauriac]

@SergeyBiryukov @flixos90 hello can you take a look over here? I'm pinging you because i don't know who is component maintainer. thank you.

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

[pbearne]

I was able to fix this by just changing what (to the email from the username) we passed to the retrieve_password

But looking at this there an optimization to be done as we can pass the whole WP_User object to retrieve_password as this first action to get the object so I have done that in this ticket (which would fix this ticket as well) #56028

[SergeyBiryukov]

Hi there, welcome to WordPress Trac! Thanks for the ticket, patches and the PRs.

I think I would prefer ​PR 1536 here, as it attempts to fetch the user both by email and username, without adding a new parameter to the function.

​PR 2837 may look simpler, but I have a few concerns:

• It doesn't feel right to pass an email to the function if the parameter is specifically documented as a username. Should the documentation be updated in that case?

   * @param string $user_login Optional. Username to send a password retrieval email for.
   *                           Defaults to `$_POST['user_login']` if not set.
  

• This is not the only instance where core passes user_login to retrieve_password(), there is at least one other line in wp-admin/users.php that would need to be updated. In the future, we would also need to remember to pass an email to the function instead of a username.
• While passing an email instead of username fixes this particular ticket, it would be a good idea to confirm that it does not cause unintentional breaking changes in other cases. Some unit tests would be helpful here.

[SergeyBiryukov]

53634.2.diff​ is a slightly simplified version of ​PR 1536.

[audrasjb]

Thanks @pbearne, I tested your PR with the provided testing steps and it also fixes the issue on my side.

Steps to reproduce:

• make a user with the same username AND email address. e.g. foo(@)example.com
• edit that user and change the email address (username not editable) to the email address. e.g. bar(@)example.com and save user
• Try to use the “Send Reset Link” button while editing that user again (/wp-admin/user-edit.php)

After applying the patch, the email is sent.

[audrasjb]

Alright, just saw @SergeyBiryukov's comments.
Sounds legit to me 👍

[SergeyBiryukov]

53634.3.diff​ adds a unit test.

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

[pbearne]

I would still include the patch I did, as this passes the name, which is a safer value to pass

[SergeyBiryukov]

In 54477:

Users: Fetch user by login in retrieve_password() if not found by email.

This ensures that sending a password reset link works as expected if the user's login and email were initially the same, but the email address was subsequently updated and no longer matches the login, which is still set to the old address.

Follow-up to [6643], [18513], [19056], [37474], [50129], [50140].

Props donmhico, pbearne, azouamauriac, boblindner, daxelrod, audrasjb, SergeyBiryukov.
Fixes #53634.

Merged in r54477.

Thanks for the PR! Merged a slightly simplified version in r54477.

Thanks for the PR! This should be resolved in r54477.