Allow admins to send users a 'Reset Password' link

[Ipstenu]

Following #24633 and #34180

As Pippin said:

it'd be really nice for admins to be able to manually generate a new password for user accounts and then automatically send that new password via email.

As an extension of that, should we be able to easily send users a link to reset their passwords? It would greatly improve supporting communities. Instead of explaining "Go to domain.com/wp-admin/ and click on lost password and put in your info..." you can just press a button and send a link. Done.

I was able to make it a plugin but I'm epically failing at getting the ajax/js stuff to work right as a core patch :(

Screenshot: ​https://cloudup.com/cWNmv6T0SXI
Code: ​https://cloudup.com/c2SpsmqXb14

This doesn't actually change the password or even force it. If you, as an admin, change the password of a user, they get an email anyway. This uses just emails them the reset link.

[mordauk]

I support this times 100000x.

[knutsp]

+1

This is really needed and was missed in the fix for the previous ticket.

As an admin we usually don't want to brutally change the password for a user. We want to have a simple way to enable our users to easily change their password themselves.

[adamsilverstein]

34281.diff​ is a first pass. Notes:

• Built off of code provided by @Ipstenu - thanks!
• I'm leveraging the existing retrieve_password function to avoid duplicating code and allow existing password reset hooks to work as expected. To accomplish this I moved the function from wp-login.php to wp-includes/functions.php - otherwise the function is not available in this context. Feedback appreciated.

Things that would be nice to add:

• The ability to trigger password resets as a bulk action from the user list screen.
• Extending retrieve_password to accept a user_login argument. Currently, I'm setting up $_POST['user_login'] because thats what the function expects. That feels wrong, it would be cleaner if I could pass the user_login to the function directly.
• Unit tests.

Would appreciate feedback on the UI as well, especially the success (or error) message inserted.

Screenshots:

Reset button:

After reset:

[DrewAPicture]

First impressions on 34281.diff​:

• Seems weird to display the notice inline like that, I'd expect some other kind of feedback, or a page refresh with the notice at the top
• Something doesn't feel great about the button text + the setting title + the description text. Maybe "Reset Password" for the button, with description text "The user will be emailed a password reset link.", but then the setting title is redundant. Hmm.

[adamsilverstein]

Replying to DrewAPicture:

Thanks for the feedback. Not sure about the notice either. I like inline because its an ajax action not a full submission of the page/form. Maybe more along the lines of shiny updates?

Thinking I should disable the button immediately on click to avoid double clicks.

Agree on the language, open to suggestions and will try to make it more concise.

[Ipstenu]

Drew the inline matches what we do for sessions. Since it's a click and done, it looks the same.

Sessions spits out "You are now logged out everywhere else." in the same success box.

The text needs better Englishing, though, yes.

Adam, the plugin did disable the button on click to prevent doubles :)

[adamsilverstein]

Replying to Ipstenu:

Drew the inline matches what we do for sessions. Since it's a click and done, it looks the same.

Good point, the inline message and behavior exactly matches the 'Log out everywhere else' button. Lets leave it that way and if we decide to change things, we can change both.

I'll work on the Englishing.

Heres what that 'log out' like after you click the button:

[Ipstenu]

• Title: Reset
• Button: Send Reset Link
• Sub-text: Send USER a link to reset their password. This will not change their password, nor will it force a change.

Pursuant to that, we should probably have a note under 'generate new password' that says 'Generate a new password for USER. This will email them an alert that their password has been changed.' I mean if we're doing this everywhere else, it would be good there as well, no?

(nb: No you can't send yourself a password reset link from the admin panel, that's just silly.)

[adamsilverstein]

In 34281.2.diff​:

• Adjust language for buttons, confirmation
• Adds a bulk action on the users screen (didn't test yet in multisite user screen)
• Screenshots incoming

[adamsilverstein]

Send reset button on profile screen:

After sending reset:

Users screen, send reset link for single user:

Users screen, confirmation after sending single user a reset:

Bulk action selection:

Bulk reset result confirmation:

[Thomas Vitale]

The patch 34281.2.diff can't be applied anymore.
This is what I got when I tried to apply it for testing it (using the latest available development version).

patch -p0 < 34281.2.diff
patching file src/wp-admin/admin-ajax.php
Hunk #1 FAILED at 62.
1 out of 1 hunk FAILED -- saving rejects to file src/wp-admin/admin-ajax.php.rej
patching file src/wp-admin/includes/ajax-actions.php
Hunk #1 succeeded at 3316 with fuzz 2 (offset 73 lines).
patching file src/wp-admin/includes/class-wp-users-list-table.php
Hunk #1 succeeded at 242 (offset 7 lines).
Hunk #2 succeeded at 416 (offset 7 lines).
patching file src/wp-admin/js/user-profile.js
patching file src/wp-admin/user-edit.php
patching file src/wp-admin/users.php
Hunk #1 succeeded at 192 (offset 1 line).
Hunk #2 succeeded at 473 (offset 1 line).
Hunk #3 succeeded at 513 (offset 1 line).
patching file src/wp-includes/functions.php
Hunk #1 succeeded at 5161 (offset 69 lines).
patching file src/wp-includes/script-loader.php
patching file src/wp-login.php
Reversed (or previously applied) patch detected!  Assume -R? [n]

[eventualo]

I refred the patch

[eventualo]

I refreshed the patch for latest version.

[wonderboymusic]

you should never set values on superglobals like $_POST - retrieve_password() should be altered to accept data passed to it, and not rely exclusively on $_POST

[adamsilverstein]

Replying to wonderboymusic:

you should never set values on superglobals like $_POST - retrieve_password() should be altered to accept data passed to it, and not rely exclusively on $_POST

Yea, that did feel wrong, thanks for the feedback! I will refresh the patch and update the function to accept data instead of relying on setting $_POST.

[adamsilverstein]

Replying to eventualo:

I refreshed the patch for latest version.

Thanks for refreshing!

[adamsilverstein]

In 34281.4.diff​:

• Extend retrieve_password to accept the user_login as an argument
• pass user_login to retrieve_password instead of using $_POST

[eventualo]

I tried the 34281.4.diff, but when I try to send the reset email using the Send reset link button in user profile I got a wrong message ([object Object]).

So I uploaded a new 34281.5.diff with these changes:

• inside retrieve_password function: if $user_login is empty and $_POST['user_login'] is set, $user_login gets $_POST['user_login'] value; then, I updated the following IF statement to check only the value of $user_login.
• inside user-profile.js file, I updated the addInlineNotice function: now the resultDiv prints html, because the output message can be html, and uses a fadeIn, useful if the user clicks the button again.

[adamsilverstein]

Replying to eventualo:

Thanks for the updated patch... I will re-test.

• inside user-profile.js file, I updated the addInlineNotice function: now the resultDiv prints html, because the output message can be html, and uses a fadeIn, useful if the user clicks the button again.

How would the response contain HTML? It should only be text unless I am missing something?

I'm concerned that using .html() opens a security hole because the response would be evaluated. If an attacker could alter the response, they could inject JavaScript into the admin page. I'd prefer to stick to text() which is safer.

Also I'm not sure fadein is appropriate, do we use that anywhere else in core?

In the previous version of the patch, we disable the button upon response, preventing duplicate clicks. That part is missing in your latest patch. I think this better matches what we are doing with the 'Log me out everywhere else' button right below this button. It might make more sense to fire right after clicking the button instead of on response.

[eventualo]

Replying to adamsilverstein:

Thanks a lot @adamsilverstein for your feedback.

How would the response contain HTML? It should only be text unless I am missing something?

I'm concerned that using .html() opens a security hole because the response would be evaluated. If an attacker could alter the response, they could inject JavaScript into the admin page. I'd prefer to stick to text() which is safer.

During my tests, I got also some error messages during sending emails. An error message is in html:
The email could not be sent.<br />Possible reason: your host may have disabled the mail() function.
for this reason I propose the .html(), but I undestand the security issue, and I agree.

Also I'm not sure fadein is appropriate, do we use that anywhere else in core?

In the previous version of the patch, we disable the button upon response, preventing duplicate clicks. That part is missing in your latest patch. I think this better matches what we are doing with the 'Log me out everywhere else' button right below this button. It might make more sense to fire right after clicking the button instead of on response.

I didn't disable the button upon response. With my patch, the button is properly disabled only on successfully sending, as in previous patch. But, if there is an error, the button is not disabled: this behaviour happened also in previous version, so I proposed the fadeIn. In my opinion it could be a quick way to solve the issue of a always active button, after having an error and trying another chance.

[adamsilverstein]

Replying to eventualo:

During my tests, I got also some error messages during sending emails. An error message is in html:
The email could not be sent.<br />Possible reason: your host may have disabled the mail() function.
for this reason I propose the .html(), but I undestand the security issue, and I agree.

Aha! Ok, I'll do some more testing and clarification on this and disabling the button. I think we may want to remove that html from the string. Thanks for keeping this fresh, I have a feeling its going to get punted until 4.5, I leave that up the the lead devs to decide.

[eventualo]

Replying to adamsilverstein:

Aha! Ok, I'll do some more testing and clarification on this and disabling the button. I think we may want to remove that html from the string. Thanks for keeping this fresh, I have a feeling its going to get punted until 4.5, I leave that up the the lead devs to decide.

Perfect, again thanks a lot for your code and tests.

[adamsilverstein]

In 34281.6.diff​ :

• hide any previous error before submitting new request (and remove fade)
• use a jQuery text transformation to strip HTML from returned messages, switch to .text() vs. .html()

[ocean90]

I'm curious why this ticket is labeled as a bug, looks like an enhancement and plugin material to me.

[adamsilverstein]

Replying to ocean90:

I'm curious why this ticket is labeled as a bug, looks like an enhancement and plugin material to me.

Definitely an enhancement, changing that. Could be a plugin, I agree. Lets discuss further.

[knutsp]

Pre 4.3, when a new password was requested, it was sent by email. When administrators wanted to change the password of a user, they usually emailed it.

As of 4.3 we discourage sending passwords by email by introducing a password reset link. This is about hardening WordPress security.

Still, if an admin wants to enable the user to log in after lost password, the old practice of sending a new password by email, manually, may persist.

Even if this could become a nice plugin, making the life of an administrator easier, it could also bee seen as part of a security hardening, following up the password changes in 4.3.

I think it goes along with the ability to log out of other sessions. That could also have been a plugin.

Apart from being nice and handy, I think it's an important enhancement.

[ocean90]

The slot for 4.4 enhancements was closed on Oct 22th.

[ericlewis]

@adamsilverstein anything else we need here, or ready for review?

[adamsilverstein]

Replying to ericlewis:

@adamsilverstein anything else we need here, or ready for review?

Generally ready for review, I am also open to reworking as a plugin.

[ericlewis]

Patch applies to trunk for review.

I don't think it needs to be in the user list table row and bulk actions dropdown. Leaving this in the user profile next to password feels like the right context.

@helen @michaelarestad @melchoyce any feedback on the flow?

[DrewAPicture]

Seems like some user testing could provide valuable data on the best approach here.

[afercia]

Thinking when it's AJAX it would be nice to send success/failure messages to wp.a11y.speak() :)

[adamsilverstein]

Replying to afercia:

Thinking when it's AJAX it would be nice to send success/failure messages to wp.a11y.speak() :)

Good point, thank you: I can add that. I'm still awaiting feedback on whether this belongs in core or should be offered as a plugin.

[melchoyce]

Replying to ericlewis:

Patch applies to trunk for review.

I don't think it needs to be in the user list table row and bulk actions dropdown. Leaving this in the user profile next to password feels like the right context.

@helen @michaelarestad @melchoyce any feedback on the flow?

The flow seems pretty straight-forward. What does the password reset email look like?

[Ipstenu]

It sends the default password reset email. The one that starts "Someone requested that the password be reset for the following account:..."

[jorbin]

There hasn't been any recent activity on this ticket and the beta 1 deadline for 4.5 is tomorrow, so punting.

[jazbek]

Not sure how useful this is, but I wanted to share that there is already a plugin that has pretty similar functionality to what's being requested here. I tried it last night and it needs to be updated to work with WP 4.3+ so I might do that, since I really need this functionality:

​https://github.com/atwellpub/resend-welcome-email

Alternatively, if any of the devs here want to turn their patches attached to this ticket into a plugin, I'd be happy to help.

[knutsp]

#38915 was marked as a duplicate.

[afercia]

Removing this from the accessibility focus to clean up the Trac reports, as the only potential a11y improvement here would be implementing audible feedbacks via wp.a11y.speak().

[JoshuaWold]

Just took a look over the UX solution provided for the profile screen. I think this works well and also like the idea of the bulk edit option! Could personally benefit from from this being added :)

[dainismichel]

apparently some ppl were using this plugin
​https://wordpress.org/plugins/re-send-welcome-email/
to accomplish "admin can send specific users a password reset link," but that plugin is now dated.

how is interest and development looking for this feature?

best,
dainis

[adamsilverstein]

34281.7.diff​ is a refresh against trunk, resolving several merge conflicts. Given the close proximity of 5.1 beta, milestoning this as 5.2 so we can test further and add some audible feedback for screen readers as suggested in https://core.trac.wordpress.org/timeline?from=2017-01-23T18%3A02%3A57Z&precision=second

[desrosj]

This still needs feedback and testing. Punting to 5.3 since 5.2 beta 1 is in less than 2 days.

[adamsilverstein]

Milestoning this as 'future release' until there is some more progress.

[adamsilverstein]

Going to try to tackle this issue in 5.4, milestoning as such.

[davidbaumwald]

Most recent patch fails against trunk. Marking for a refresh.

[bookdude13]

Refreshed patch

[bookdude13]

Patch refreshed. Had to merge the retrieve_password function in wp-login.php into the one added in functions.php by earlier patches. It seems to be needed in functions.php to have the scope needed for both the login screen and the edit-users screen. A quick round of testing and the patch seems to work well. Very clean code from earlier contribs!

Note that the @since tags should be double checked; some of those were added in the original patches here back in about 4.4.

Side note, is the dev-feedback keyword still needed?

[davidbaumwald]

@adamsilverstein With the refreshed patch, do you think this can land for 5.4 Beta 1 today?

[adamsilverstein]

I will review today and give this a test to see if it ready to land.

[karmatosed]

With beta 1 coming today and expected in an hour or so, let's move this to future release just for now. This doesn't mean it won't get worked on, just right now it's going into next one.

[bookdude13]

Sounds good. Note that the @since tags will need updating then

[adamsilverstein]

Apologies, I wasn't able to spend time on this for 5.4, I'll work on it for 5.5.

Trac ticket:

How this looks currently:

"Send Reset Link" on profile page (user is "Theme Reviewer")

• Send password reset in the bulk action menu

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

"Send Reset Link" on profile page.

• Confirmation.

• Send password reset in the bulk action menu

User list confirmation.

User list reset password action.

[adamsilverstein]

34281.10.diff​ is a refresh against trunk. Everything looks good and tested well for me and this feels like a genuinely useful addition.

I have added some screenshots above.

Going to commit this as is before 5.7 beta1 so it can get some wider testing.

[johnbillion]

This looks good. The retrieve_password() function needs some work though. If you diff it with the current one in wp-login.php some of the recent changes have been lost, for example the docs updates, the IP address message, the lowercase ERROR strings, and the lostpassword_errors filter. Might be some more, I didn't check too thoroughly.

Also it will need a @since entry for the new parameter, and probably also one to mention that prior to 5.7.0 it only existed in wp-login.php.

My only concern about the functionality itself is whether the "Send password reset" link is needed on the Users list table. It adds a lot of visual noise. Is this action common enough to warrant that link?

[adamsilverstein]

@johnbillion Thanks for the review! In 34281.11.diff​

• I copied the retrieve_password function over fresh, reintroduced the change, and added your suggested @since and not annotations to the doc block.

My only concern about the functionality itself is whether the "Send password reset" link is needed on the Users list table. It adds a lot of visual noise. Is this action common enough to warrant that link?

Yea, I tend to agree: I was wondering about this myself. It feels like having it as an option in the bulk menu is sufficient? I have left it in for now, I would like to get some more design review before making a final decision.

With these changes in place, do you feel like this is ready? If so, I would like to commit it before 5.7 beta 1 tomorrow.

[johnbillion]

34281.12.diff​ makes a couple of tweaks:

• Removed the formatting-only changes (mostly == to ===) that aren't related to the functional change, these should happen in a separate ticket so they can be checked properly.
• Adjusted the formatting of retrieve_password() to minimise the non-functional changes and make it easier to see the diff from its old version. Below is the actual diff.
• There are two noticeable changes to retrieve_password() as a result. These appear to create a functional change that might not be expected. The two sanitize_*() functions both strip octets and entities, for example. @adamsilverstein what's the reason to switching to using these functions?
  • sanitize_email(...) is used instead of trim( wp_unslash(...) )
  • sanitize_user(...) is used instead of trim( wp_unslash(...) )

.php

@@ -351 +351 @@
  * Handles sending a password retrieval email to a user.
  *
  * @since 2.5.0
+ * @since 5.7.0 Added `$user_login` parameter.
  *
+ * Note: prior to 5.7.0 this function was in wp_login.php.
+ *
+ * @global wpdb         $wpdb       WordPress database abstraction object.
+ * @global PasswordHash $wp_hasher  Portable PHP password hashing framework.
+ *
+ * @param  string       $user_login Optional user_login, default null. Uses
+ *                                  `$_POST['user_login']` if `$user_login` not set.
  * @return true|WP_Error True when finished, WP_Error object on error.
  */
-function retrieve_password() {
+function retrieve_password( $user_login = null ) {
         $errors    = new WP_Error();
         $user_data = false;
 
-        if ( empty( $_POST['user_login'] ) || ! is_string( $_POST['user_login'] ) ) {
+        // Use the passed $user_login if available, otherwise use $_POST['user_login'].
+        if ( ! $user_login && ! empty( $_POST['user_login'] ) ) {
+                $user_login = $_POST['user_login'];
+        }
+
+        if ( empty( $user_login ) ) {
                 $errors->add( 'empty_username', __( '<strong>Error</strong>: Please enter a username or email address.' ) );
-        } elseif ( strpos( $_POST['user_login'], '@' ) ) {
-                $user_data = get_user_by( 'email', trim( wp_unslash( $_POST['user_login'] ) ) );
+        } elseif ( strpos( $user_login, '@' ) ) {
+                $user_data = get_user_by( 'email', sanitize_email( $user_login ) );
                 if ( empty( $user_data ) ) {
                         $errors->add( 'invalid_email', __( '<strong>Error</strong>: There is no account with that username or email address.' ) );
                 }
         } else {
-                $login     = trim( wp_unslash( $_POST['user_login'] ) );
-                $user_data = get_user_by( 'login', $login );
+                $user_data = get_user_by( 'login', sanitize_user( $user_login ) );
         }
 
         /**

[adamsilverstein]

@johnbillion Excellent, thanks for reviewing.

Removed the formatting-only changes (mostly == to ===) that aren't related to the functional change, these should happen in a separate ticket so they can be checked properly.

Thanks. I had changed these to assuage the linter, better to leave them out!

Adjusted the formatting of retrieve_password() to minimise the non-functional changes and make it easier to see the diff from its old version. Below is the actual diff.

Super, thanks for providing that and reformatting.

There are two noticeable changes to retrieve_password() as a result. These appear to create a functional change that might not be expected. The two sanitize_*() functions both strip octets and entities, for example. @adamsilverstein what's the reason to switching to using these functions?

This looks like a mistake, likely from, the original patch work which we have studiously brought forward. I will revert these changes.

[johnbillion]

Great, so I think with a revert of those sanitize functions and a decision on whether the links are needed in the Users list table, this is good to go.

[adamsilverstein]

In 34281.13.diff​:

• Revert sanitization changes.

[paaljoachim]

Hi

I am testing the feature. Thank you for adding it!
Thank you for the general ping in the design channel @adamsilverstein

Disadvantage.
Hiding a feature in the Bulk actions is not very user friendly. People will likely not notice it.

Advantage.
It is easier to add multiple actions into the Bulk actions drop down than it is to list them all directly in the open when hovering the username. As @johnbillion mentioned it does feel crowded adding it to the users list table. I do agree on that.

I pinged both @kellychoffman and @hedgefield in the design channel. I do not know if they will be able to give feedback in time.

My conclusion would be to merge this feature now and iterate on it later.

[adamsilverstein]

My conclusion would be to merge this feature now and iterate on it later.

Good idea. We can always remove the hover link later if we decide it is better without it.

[hedgefield]

This looks good to me from a design perspective. Makes sense. Useful feature. I too say merge and iterate if needed.

[adamsilverstein]

This looks good to me from a design perspective. Makes sense. Useful feature. I too say merge and iterate if needed.

Thanks for the review @hedgefield, I will work on the commit.

Trac ticket:

[adamsilverstein]

34281.14.diff​ fixes a jslint error. (userProfileL10n was not defined, added it to globals comment).

[adamsilverstein]

Thinking when it's AJAX it would be nice to send success/failure messages to wp.a11y.speak() :)

@afercia I realized we don't have this in place yet. We have a single place we use an ajax action to sent the password reset:

var resetAction = wp.ajax.post( 'send-password-reset', data );

Would wp.a11y.speak be added immediately before calling the ajax post? eg

wp.a11y.speak( 'Sending password reset' );

or would we fire immediately after the action with
wp.a11y.speak( 'Password reset sent' );

Or both?

[paaljoachim]

Hey Adam.

@afercia is away.
I believe @joedolson will be able to reply to your questions.

[adamsilverstein]

Thanks @paaljoachim . Going to commit this as we have it and will re-open to add the accessibility improvements.

[adamsilverstein]

In 50129:

Users: enable admins to send users a reset password link.

Add a feature so Admins can send users a 'password reset' email. This doesn't change the password or force a password change. It only emails the user the password reset link.

The feature appears in several places:

• A "Send Reset Link" button on user profile screen.
• A "Send password reset" option in the user list bulk action dropdown.
• A "Send password reset" quick action when hovering over a username in the user list.

Props Ipstenu, DrewAPicture, eventualo, wonderboymusic, knutsp, ericlewis, afercia, JoshuaWold, johnbillion, paaljoachim, hedgefield.
Fixes #34281.

[adamsilverstein]

Re-opening to add accessibility improvements as discussed in this comment.

[SergeyBiryukov]

In 50139:

Users: Use consistent strings for error messages in wp-admin/users.php.

Use _n() for a string with plural forms.

Follow-up to [50129].

See #34281.

[SergeyBiryukov]

In 50140:

Users: Move retrieve_password() to wp-includes/user.php, for consistency with other user functions.

Follow-up to [25231], [50129].

Props jfarthing84, dimadin.
See #34281, #31039.

[SergeyBiryukov]

In 50141:

Docs: Update documentation for retrieve_password() per the documentation standards.

Follow-up to [50129], [50140].

See #34281.

[adamsilverstein]

Thanks for the follow/clean up commits @SergeyBiryukov!

https://core.trac.wordpress.org/changeset/50129

[desrosj]

In 50152:

Coding Standards: Fix several minor coding standards issues.

These are made by running composer format.

Follow up to [50124], [50129], [50143].

See #49961, #52192, #34281.

[adamsilverstein]

Thanks @desrosj!

[kellychoffman]

Bringing a design review here from Slack: +1 to only having this setting live in the Bulk Action dropdown and reserving the hover links for frequent actions.

[joedolson]

Replying to adamsilverstein:

Thinking when it's AJAX it would be nice to send success/failure messages to wp.a11y.speak() :)

@afercia I realized we don't have this in place yet. We have a single place we use an ajax action to sent the password reset:

var resetAction = wp.ajax.post( 'send-password-reset', data );

Would wp.a11y.speak be added immediately before calling the ajax post? eg

wp.a11y.speak( 'Sending password reset' );

or would we fire immediately after the action with
wp.a11y.speak( 'Password reset sent' );

Or both?

Ideally, you'd send the wp.a11y.speak notification after the ajax post; it should probably be inside the addInlineNotice helper function, so that it reflects the same message that's shown to users.

I don't think this behavior is something that will take long enough that an 'in process' message is required.

[joedolson]

If this ends up only in the bulk actions dropdown, the accessibility comments might be irrelevant. However, I question whether this should be primarily a bulk action; in my own experience, I almost *always* am dealing with a password reset for just one user. The bulk action could be useful in some cases, but for convenience, the main action is more useful.

[audrasjb]

Dev note published: ​https://make.wordpress.org/core/2021/02/22/send-reset-password-links-in-wordpress-5-7/

[carike]

I had a quick look at the P2 post. I agree that it would be preferable to remove the IP address entirely.
The reasoning for this is that when a user requests a change of their own password, they would have one of two legitimate interests in knowing the information:
1.) It is their own IP address;
2.) Someone is possibly trying to phish / hack them and they have a security-based interest.
Both of these are legitimate interests.

However, it is not desirable to give users access to the admin's IP address. I would not even want users to have access to my username (I would feel more comfortable if it included the display name, if any details had to be included at all).

I'm also concerned that with many WordPress users not being very tech savvy (and that is okay, that is the magic of crowd-sourcing code), this feature may make it easier for bad actors to phish.
Banks have spent billions trying to teach people not to click on any links in e-mails that you did not initiate yourself. :sunflower:

[audrasjb]

In that case, I think our best option is to add a parameter to the retrieve_password function so it’s possible to display the IP when it’s needed (lost password link) and to remove it in all other cases 🙂

[audrasjb]

Remove the IP Address from Admin generated password reset

[audrasjb]

In 34281.15.diff:

• Add a $context parameter to retrieve_password to avoid to send IP addresses when it's not needed.

send_password_reset_from_admin is probably not the best name… but you know… naming things is hard.

[SergeyBiryukov]

Looking at 34281.15.diff​:

Used to avoid sending the Admin IP address when the password reset was not asked by the user.

It seems like this could be done without adding a new parameter to the function, just by comparing the $user_login parameter to the current user's login instead.

[audrasjb]

Replying to SergeyBiryukov:

It seems like this could be done without adding a new parameter to the function, just by comparing the $user_login parameter to the current user's login instead.

Maybe I'm wrong @SergeyBiryukov, but I'm afraid it won't work for the Lost password link, as the user is not logged in in this context, by definition :) (and that's the main context where we'll want to provide the IP Address).

[SergeyBiryukov]

Replying to audrasjb:

I'm afraid it won't work for the Lost password link, as the user is not logged in in this context, by definition :) (and that's the main context where we'll want to provide the IP Address).

That was my point, only add the IP address if the login is not the same as the current user's login (which would be the case when requesting the password reset link from the admin). But it's possible that I'm missing something too :)

[audrasjb]

Replying to SergeyBiryukov:

That was my point, only add the IP address if the login is not the same as the current user's login (which would be the case when requesting the password reset link from the admin). But it's possible that I'm missing something too :)

Ah! I believe we want the opposite :)

• Lost password link on wp-login: send the IP address so the user can be prevented from request from other IPs
• New reset password methods on WP-Admin: don't send the IP address as the password reset is asked by a known user on the website (and it fixes some potential privacy issues)

[SergeyBiryukov]

Replying to audrasjb:

Ah! I believe we want the opposite :)

Right, my comment doesn't make much sense on second thought. Replying to too many things at a time, sorry :)

• Lost password link on wp-login: send the IP address so the user can be prevented from request from other IPs
• New reset password methods on WP-Admin: don't send the IP address as the password reset is asked by a known user on the website (and it fixes some potential privacy issues)

I think this can be done with one of two options:

• Only include the IP address if the user is not logged in (which would be the case when requesting the password reset link from wp-login.php).
• Only include the IP address when requesting the password reset link from wp-login.php specifically, by checking the $pagenow global, like we do in several other places in core.

Per your second point, it looks like the first option would preferable.

My concern with adding a new parameter this late in the release cycle is that it's hard to adjust later without breaking backward compatibility, which might lead to minor inconsistencies in the API in the future. This would require some careful thinking, so I'd like to avoid adding a new parameter for now if there's another way.

[audrasjb]

My concern with adding a new parameter this late in the release cycle is that it's hard to adjust later without breaking backward compatibility, which might lead to minor inconsistencies in the API in the future. This would require some careful thinking, so I'd like to avoid adding a new parameter for now if there's another way.

I understand. I'll update a new patch with the second option ;)

[audrasjb]

Remove the IP Address from Admin generated password reset

[audrasjb]

I tested this solution and it works fine on my side.

[gmariani405]

@audrasjb "Remove the IP Address from Admin generated password reset"

Not sure this is great idea either. The IP address (while fraught with privacy concerns) is the only thing validating that this email came from the website and is not a phishing email. Unless there is a better way to validate the authenticity of the email I'd say it would be worthwhile to keep it.

Scenario, user asks host or web designer for help. The host/web designer goes in and resets their password. User receives both the legitimate mail and by chance a phishing email. Both would look indistinguishable. The IP helps distinguish it as legitimate.

I realize it's not fool-proof, a savvy enough bad-actor could DNS lookup the IP before sending the phishing email but that is more labor intensive when you're mass-mailing. This also relies on the end-user being skilled enough to validate the website domain against what was displayed in the email, but I say to that, at-least it's better than nothing.

[Ipstenu]

The IP address (while fraught with privacy concerns) is the only thing validating that this email came from the website and is not a phishing email.

It is though? I could use my phone to send a reset, and I would have no idea what my IP was. And that can easily be faked. Omitting the IP actually reduces the data being sent out that could be used by bad-actors.

I think it's more likely we'd have a savvy bad actor than end users who would need to ask for a password reset but also know what a valid IP is and how to ask about it.

Not that we shouldn't look into something like logging or 'proving' ... off the top of my head, if when we use the password reset, it sets a key in the user's account, and the user has to enter that key to reset the password? That could work.

Edited to add: I feel this is a "release and iterate" case! We can improve this down the road, and I do not think my off the cuff idea is a blocker!

[juliobox]

This IP address is just useless, can we just remove it?
Who knows how to compare an IP with its own to be sure it's not a fake? Power user I guess.
Problems resolved ;)

[desrosj]

In 50415:

Users: Ensure reset password emails are in the receiving user’s locale.

This ensures that reset password emails initiated by an administrator are sent to the user in their preferred locale.

Follow up to [50129,50139-50141,50152].

Props chouby, davidbaumwald, audrasjb, johnbillion.
Fixes #52605. See #34281.

[gmariani405]

I agree that the IP would only be valuable in a power-user scenario and it's by no means fool-proof. But SOMETHING should be available to help prove the authenticity of the email.

"This IP address is just useless," - A bit of an overstatement there. If ANY user (even if just a power-user) is able to avoid a ransomware attack by this information proves it has some value.

[SergeyBiryukov]

In 50422:

Users: Only include the IP address in password reset email if the user is not logged in.

This avoids unnecessarily disclosing the IP address when sending a password reset link to another user from the admin.

Follow-up to [49255], [50129].

Props carike, audrasjb, gmariani405, Ipstenu.
See #34281.

[SergeyBiryukov]

Let's call this one fixed for 5.7. Please create new tickets for any follow-up issues.

Thanks everyone!

[Ipstenu]

FYI made a follow up at #52630

[peterwilsoncc]

In 50439:

Users: Use localized site title for password reset emails.

When sending password reset emails, switch to the user's locale prior to obtaining the site title to allow for plugins filtering based on locale.

Follow up to [50129,50139-50141,50152,50415].

Props chouby.
Fixes #52605. See #34281.

[SergeyBiryukov]

In 50440:

Users: Use localized site title for password reset emails.

When sending password reset emails, switch to the user's locale prior to obtaining the site title to allow for plugins filtering based on locale.

Follow up to [50129,50139-50141,50152,50415].

Props chouby.
Merges [50439] to the 5.7 branch.
Fixes #52605. See #34281.

[ocean90]

In 51077:

Users: Add user’s locale to password reset link to ensure login screen matches the language of the email.

Props walbo.
See #34281, #52605.
Fixes #53321.