WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 weeks ago

#38334 closed enhancement (fixed)

Login: Pass the `$user_data` object as a parameter to the `lostpassword_post` hook

Reported by: pagewidth Owned by: johnbillion
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: has-patch
Focuses: Cc:
PR Number:

Description

In function 'retrieve_password()' in 'wp-login.php', the 'lostpassword_post' action hook does not allow access to the $user_data object to do any validation/error checking or make any changes to the user information.

In my use case I store an account/membership id in the user's profile email address field in order to allow the same email address to be used for different account/membership ids, then remove the account/membership id from the email address before sending the email.

In the 'retrieve_password()' function, the email address is saved in the local variable:

$user_email = $user_data->user_email;


Then this local variable is used to send the email later in:

if ( $message && !wp_mail( $user_email, wp_specialchars_decode( $title ), $message ) )


There is currently no way to change the local variable copy of the email address '$user_email' either via the above action hook, or the two filter hooks before sending the email:

$title = apply_filters( 'retrieve_password_title', $title, $user_login, $user_data );
$message = apply_filters( 'retrieve_password_message', $message, $key, $user_login, $user_data );


I am requesting changing the line:

do_action( 'lostpassword_post', $errors );

to:

do_action( 'lostpassword_post', $errors, $user_data );


This will allow any changes to the $user_data information and/or further validation/error checks using the user information, which will also allow any changes to be copied to the $user_email local variable.

Attachments (2)

38334.diff (488 bytes) - added by lukecavanagh 3 years ago.
Basic patch.
38334.2.diff (1.3 KB) - added by wpgurudev 3 weeks ago.
Patch with updated docblock. Also handled variable initialisation.

Download all attachments as: .zip

Change History (9)

@lukecavanagh
3 years ago

Basic patch.

#1 @johnbillion
3 years ago

  • Keywords needs-patch added
  • Version 4.6.1 deleted

Thanks for the ticket, @pagewidth .

@lukecavanagh The inline docs for the action will need updating (bearing in mind that $user_data can be boolean false or a WP_User object. In addition, if the first condition in the function is true (an empty value for $_POST['user_login']) then the $user_data variable won't be defined.

#2 @desrosj
9 months ago

  • Keywords good-first-bug has-patch needs-refresh added; needs-patch removed
  • Milestone changed from Awaiting Review to Future Release

@lukecavanagh or @pagewidth are you able to refresh the patch to address the feedback above?

#3 @kkarpieszuk
8 months ago

I think nothing has to be changed here at all (the ticket could be closed without applying any patch).

Please see that inside of the function retrieve_password(), the $user_data is generated (if it is really generated, as @johnbillion correctly pointed out) from $_POST data and nothing else.

So, whoever would utilize this lostpassword_post action in his plugin or theme, has complete access to the same $_POST values and can run get_user_by() on them to get user data.

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


3 weeks ago

@wpgurudev
3 weeks ago

Patch with updated docblock. Also handled variable initialisation.

#5 @wpgurudev
3 weeks ago

@kkarpieszuk While your suggestion is valid, it is not very handy way to handle the mentioned use case and others. We are anyways passing $user_data variable to other hooks so we should pass variable for lostpassword_post hook as well; instead of determining user from $_POST variable.

Submitting the patch with updated doc.

#6 @johnbillion
3 weeks ago

  • Keywords good-first-bug needs-refresh removed
  • Milestone changed from Future Release to 5.4
  • Owner set to johnbillion
  • Status changed from new to reviewing

Agreed, contextual parameters passed to hooks are always preferred to superglobals.

#7 @SergeyBiryukov
3 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 46749:

Login and Registration: Pass $user_data parameter to the lostpassword_post action in retrieve_password().

Props wpgurudev, pagewidth, lukecavanagh, johnbillion, kkarpieszuk.
Fixes #38334.

Note: See TracTickets for help on using tickets.