Include submitter IP details in password reset emails?

[cefiar]

Could WP password reset emails include the IP of requester when someone asks for a password to be reset?

I've been seeing a lot of bots that seem to spam the password reset link (they find a username from a post, then hit the password reset link using that username), and this would make it easier to pick up and block that IP/range if it was in the reset email already, rather than having to dig through the webserver logs looking for which IP submitted the password reset request.

Note: From looking over wp-login.php this seems like it'd be fairly trivial to implement, but I wasn't sure what the best method for determining the clients IP address to use in the email template (no use creating a security hole or providing useless info), otherwise I might have included a patch.

FWIW: Google and various other sites usually report which IP either asked for the reset, or after a reset happened report that someone from that IP changed/reset the password, so basically I'm asking for similar sorts of detail from WP.

[iandunn]

Replying to cefiar:

wasn't sure what the best method for determining the clients IP address to use in the email template (no use creating a security hole or providing useless info)

WP_Community_events::get_unsafe_client_ip() might be useful there.

Adding the gdpr keyword since this could be considered sharing "personal data" with an external system.

[desrosj]

Moving to the new Privacy component.

[allendav]

I like it. If we do this, the patch should also add something like this to wordpress' wp_add_privacy_policy_content call:

“If you request a reset of your password, your IP address will be included in an email to the site administrator."

[desrosj]

Removing the GDPR keyword. This has been replaced by the new Privacy component and privacy focuses in Trac.

[isharis]

Hello,

This is my first patch and I'd like to be involved in coming up with a solution to make it through the core. I think that the simple solution of adding IP of the form submitted in the email is one solution but what I think should happen is this:

Like twitter, password reset emails should include the device and location. This is enough information for a user.

For the admin, do admins need to get a password reset email for each user? In the case where a site admin is not getting attacked by bots, this can be annoying.

[desrosj]

At first glance, 43856.diff​ needs the since annotation updated for wp_get_unsafe_client_ip() and the IP address: %s string is missing a `/* translators: */ comment.

@isharis, are you able to address that and refresh the patch?

[garrett-eclipse]

Applying needs-refresh due to @desrosj feedback.

[garrett-eclipse]

Moving this off the milestone until I've had a chance to test and refresh

[garrett-eclipse]

Refreshed patch and included text for default privacy policy content

[garrett-eclipse]

New suggested text for the default privacy policy content

[garrett-eclipse]

Thanks for the initial patch @isharis I've refreshed it in 43856.2.diff to apply to trunk and make the following amendments;

1. Added If you request a reset of your password, your IP address will be included in the reset email. to the default privacy policy content as suggested by @allendav.
2. Addressed the comments by @desrosj updating to 5.6.0 and adding translator comment.
3. Updated the verbiage in the email as just 'IP Address' felt like it could be confused with the website IP. Verbiage used This password reset request originated from the IP address %s.
4. I made it conditional so if wp_get_unsafe_client_ip returns false the string isn't added.

Adding dev-feedback/needs-privacy-review as I feel we don't need to anonymize the IP in this context as this is a security measure so would fall into section f of the GDPR. The full IP is more useful in ensuring identity in this case. I didn't remove the anon_ip portion yet as I'd like some input on that.

[garrett-eclipse]

Email w/ updated verbiage to include IP address

[bridgetwillard]

Hi, @garrett-eclipse

I'm very happy to help edit the copy in the screenshot. Are you working in a Google Doc?

Replying to garrett-eclipse:

[garrett-eclipse]

Hi @bridgetwillard we're just looking at two strings here so any suggestions can just be made on the ticket as a comment.

Specifically we're looking at introducing this string to the Privacy Policy Guide;

'If you request a reset of your password, your IP address will be included in the reset email.'

And the password reset email;

'This password reset request originated from the IP address %s.'

Note: The %s is the IP address of password reset requestor.

I greatly appreciate the review/input.

[bridgetwillard]

Thank you so much @garrett-eclipse. I saw the .diff files but it's not GitHub so I wasn't sure how to make the suggestion.

Suggested Edit

"Note: if you request a password reset, your IP Address will be included in the reset email."

I think the second phrase is succinct and well-said.

PS About The Word "Just"

I would eliminate the word "just" in the screenshot (not this string) that says "If this was a mistake, just ignore it..."

Replying to garrett-eclipse:

Hi @bridgetwillard we're just looking at two strings here so any suggestions can just be made on the ticket as a comment.

Specifically we're looking at introducing this string to the Privacy Policy Guide;

'If you request a reset of your password, your IP address will be included in the reset email.'

And the password reset email;

'This password reset request originated from the IP address %s.'

Note: The %s is the IP address of password reset requestor.

I greatly appreciate the review/input.

[garrett-eclipse]

Excellent, thanks @bridgetwillard for the copy review, great points, will refresh the patch when I have a chance.

[garrett-eclipse]

Refresh to address copy review by @bridgetwillard

[garrett-eclipse]

Removing needs privacy review as I feel I gave that initially. We're sending the users own IP back to them, and if it's not their then it's a flag that it's a potentially malicious action. As this is a improvement on the security of the action I see that falling under section (f) 'Legitimate Interests' of the GDPR.

[garrett-eclipse]

Aside from a little testing this is ready to go.

[SergeyBiryukov]

If this was a mistake ignore this email and nothing will happen.

Just noting that it seems better to me with a comma:

If this was a mistake, ignore this email and nothing will happen.

[garrett-eclipse]

Replying to SergeyBiryukov:

If this was a mistake ignore this email and nothing will happen.

Just noting that it seems better to me with a comma:

If this was a mistake, ignore this email and nothing will happen.

Thanks @SergeyBiryukov I was on the fence about that, do you want me to refresh or will you handle as part of commit?

[bridgetwillard]

Generally a comma is used when the phrase can be taken out and the meaning remains the same.

In this case, I think the comma isn’t needed.

Just my two cents.

[carike]

Tested on a single site.
The IP is included in the password reset e-mail.

[ocean90]

As discussed on Slack, 43856.3.diff​ should be simplified to only use $_SERVER['REMOTE_ADDR'] instead of introducing wp_get_unsafe_client_ip().

[garrett-eclipse]

Refresh patch to us $_SERVER['REMOTE_ADDR']

[garrett-eclipse]

I've refreshed in 43856.4.diff to use $_SERVER['REMOTE_ADDR']. Please retest.

[helen]

In 49255:

Privacy: Add requester IP to password reset emails.

Props garrett-eclipse, bridgetwillard, isharis, ocean90.
Fixes #43856.