Add "Didn't expect this?" message#728
Conversation
kasparsd
left a comment
kasparsd
left a comment
There was a problem hiding this comment.
Thanks for your contribution! This is great.
Left a couple of optional suggestions inline the pull request.
| public function generate_and_email_token( $user ) { | ||
| $token = $this->generate_token( $user->ID ); | ||
| $token = $this->generate_token( $user->ID ); | ||
| $remote_ip = preg_replace( '/[^0-9a-fA-F:., ]/', '', $_SERVER['REMOTE_ADDR'] ); |
There was a problem hiding this comment.
There is filter_var() with FILTER_VALIDATE_IP that should be able to handle this correctly for all IP types.
Secondly, this might return invalid IP if the site is behind a proxy. Unfortunately WP core doesn't have a helper function to retrieve this data consistently. The closest implementation is this https://developer.wordpress.org/reference/classes/wp_community_events/get_unsafe_client_ip/ but as explained in this article, we should probably stick to $_SERVER['REMOTE_ADDR'] and possibly add a filter to adjust as needed.
There was a problem hiding this comment.
FILTER_VALIDATE_IP only validates, doesn't sanitize.
That preg_replace() is lifted off from https://github.com/WordPress/wordpress-develop/blob/63b3a8f6c874c8dd7cec0400edf3135ad98f649d/src/wp-includes/class-wp-xmlrpc-server.php#L7022
kasparsd
left a comment
kasparsd
left a comment
There was a problem hiding this comment.
This great! Let’s ship it!
Thanks again!
3 participants
What?
Add's "Didn't expect this?" message, as suggested by @dd32 in #726 (comment)
Why?
If your credentials have been compromised and someone's been able to log in to your account with email 2FA active, you might be wondering why your site is sending you 2FA codes, when you haven't been logging in.
How?
Adds additional information to the sent email.
Changelog Entry
Not sure if we should use
PHP_EOL, since WordPress codebase doesn't seem to use that either and if we should add the message as whole instead of concating it line by line.Enter %s to log inhas already been translated to many languages, so that should stay separate?