Changeset 25696 breaks expected value of argument sent to filter 'retrieve_password_message'

[dcavins]

In changeset [25696] to wp-login.php, the function 'retrieve_password' was changed to hash the generated key about line 350:

$hashed = $wp_hasher->HashPassword( $key );

However, the filter 'retrieve_password_message' is still sending $key as an argument, line 385

$message = apply_filters( 'retrieve_password_message', $message, $key );

So any existing filters are no longer receiving the value stored in the database (which matters because filtering 'retrieve_password_message' almost has to include a search on that value to get the requestor's user_login, which is required for the password reset link to work).

A simple fix is changing line 385 to

$message = apply_filters( 'retrieve_password_message', $message, $hashed );

Thanks for the great software!

[shawnhooper]

I see a potential problem with this recommendation. Changing $key to $hashed resolves the suggested use case of looking up user's information from the database (which stores the hashed activation key).

However, any plugins that would use this filter to re-write the activation message need to include the key in plain-text for inclusion in the activation link. Since the key cannot be obtained from the stored hash, it has to be passed to the filter from here.

Recommend adding the hash as another parameter:

$message = apply_filters( 'retrieve_password_message', $message, $key, $hashed );

so that anyone using this filter can make use of both the hashed and unhashed keys as needed.

[dcavins]

Good point. I missed that the password reset uri includes the un-hashed key. Thanks for correcting my mistake.

[nacin]

Right, I figured that sending the existing plain-text key through the filter would break the least. Happy to add the hashed value as a new argument.

I don't think this actually broke anything. Did it? Anyone doing raw queries on the DB is going to have a bad time with this change no matter what. This was about the email message being sent, not any complex operations.

I'm OK with passing the hashed value, but it seems like passing the user's login (or rather, ID or a user object) might make more sense.

[dcavins]

Replying to nacin:
Yes, including the user_login or user object would be an improvement over the earlier version of the filter args. The two pieces of information that are necessary to create the password recovery link are user_login and the key value. So any combination of arguments that can make that happen are enough. But if you're only given the key value, you can't easily get the user_login.

Thanks!

Right, I figured that sending the existing plain-text key through the filter would break the least. Happy to add the hashed value as a new argument.

I don't think this actually broke anything. Did it? Anyone doing raw queries on the DB is going to have a bad time with this change no matter what. This was about the email message being sent, not any complex operations.

I'm OK with passing the hashed value, but it seems like passing the user's login (or rather, ID or a user object) might make more sense.

[ivankristianto]

I also having similar issue with this.
I would like to change the email message formatting, but i cannot find the $user_login because it is not passed.
And the password reset url need the $key and the $user_login.

Right now to fix this, i forcefully change it (at least for now to make it working, i'm sorry...):

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

i submit the patch so it is included in the future release.

[ivankristianto]

Patch 'retrieve_password_message' to include $user_login

[ivankristianto]

Anyone check on this?

[dashaluna]

Any news on this?

[johnbillion]

Needs an inline doc for the new parameter on the filter, then this can go in.

[dcavins]

Adds user_login and WP_User object to 'retrieve_password_message' filter

[dcavins]

I've attached a new patch. In it, I added $user_login and $user_data (the WP_User object) to the filter. It made sense to add the login for parity with the earlier filters in this function: retrieve_password and retrieve_password_key, but I also liked @nacin's suggestion of sending the WP_User object, because that covers all the bases.

Thanks!

[DrewAPicture]

@since

[johnbillion]

In 30357:

Add $user_login and $user_data parameters to the retrieve_password_message filter.

Props ivankristianto, dcavins
Fixes #25853