Propagating password on change

[ChloeD]

After creating an account or changing profile information, it is possible to intercept the changed data in a hook. However, this is not possible for the cleartext psasword, and this is a useful feature especially when propagating a password change over different accounts spanning across different systems (in contexts such as updating the password for phpBB, Prestashop, or any PAM thingie when the WordPress password is changed).

Since this feature is by no means possible to implement without core hacks, I am submitting a patch to include this feature in the WP core.

This patch proves useful if we are to integrate other software bricks without having to implement SSO using WordPress' architecture. In my context, I need to be able to log-in through WordPress or directly through the business specific back-end.

Proposed patch is attached.

[ChloeD]

Patch (wp-includes/user.php) for propagating cleartext passwords through an action

[scribu]

With this change, it would be possible for any plugin to record the user's password without telling them about it. -1.

[ChloeD]

scribu: pretty much like any plugin can insert its own crap using content filters (and capture the password using $_POST if it has an admin_init hook) :/ Not a problem per se IMO.

[scribu]

True. Still, this hook encourages devs to mess with plaintext passwords, when there are possibly better alternatives.

[ChloeD]

As Rob Miller (on wp-hackers) said, "any plugin could access a user's plaintext password even now and has always been able to, by hooking into wp_login and then examining the POST variables".

Hence, I'd rather go for implementing it clearly, instead of doing it through hacks. Maybe another opinion would be useful? Instead of keeping stuff dirty, pushing them into the API is a better option, as whatever dirty or unsafe things devs will want to do, they'll be able to do no matter how much you restrict them from trying to do so.

[scribu]

That's like saying "people are going to shoot themselves in the foot anyway, so we might as well give them some bullets."

[bradparbs]

Replying to scribu:

That's like saying "people are going to shoot themselves in the foot anyway, so we might as well give them some bullets."

+1 to this, I don't think we should encourage the ability to easily grab a plaintext password, ever.

[dd32]

The patch seems sane to me, so +1 from me (although a filter rename to user_password_updated wouldn't go astray IMHO)

There's no way around it - If you're implementing a SSO system where WordPress users exist elsewhere, you need access to the plaintext password, which you currently have by checking a variety of $_POST fields.

Adding an action, clearly intended as a way to perform an action upon user password updating (be it auditing, SSO, or invalidation) seems sane, and having the users password available on that hook seems appropriate.

Plugins have full reign over the environment already, it's not worth pretending that the password is protected data that plugins shouldn't see, we don't have the ability to hide it, or control what plugins do with it, so instead we trust plugins that a user has installed on their site.

[pento]

It would need to be implemented in both wp_insert_user() and wp_update_user(), so that we expose the password of new users, too.

I wonder if this would be better as a filter? That way, a plugin (say, a password strength enforcement plugin) could return a WP_Error if the password is unacceptable, which can be returned to the calling function.

[chriscct7]

Replying to pento:

It would need to be implemented in both wp_insert_user() and wp_update_user(), so that we expose the password of new users, too.

I wonder if this would be better as a filter? That way, a plugin (say, a password strength enforcement plugin) could return a WP_Error if the password is unacceptable, which can be returned to the calling function.

This could be a good idea, but what happens if 2 plugins both intercept the hook. 1 uses it to update remote site passwords, and the other does filtering on allowable passwords and returns an error. Then the remote sites would now be set to a new password which is not allowed, and hasn't been changed.
Thoughts?

[pento]

Good point, let's just go with the action.

[chriscct7]

Refreshed patch

[peterwilsoncc]

@chriscct7 Are you able to update the patch with a docblock? I can do so if you'd like to write one up in the ticket.

[johnbillion]

I think this should call the wp_set_password action which we've already got in wp_set_password(). No need for a new action.

@johnbillion Looking at Gary's comment in Trac:

It would need to be implemented in both wp_insert_user() and wp_update_user(), so that we expose the password of new users, too.

... should we call this action near ​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/user.php#L2416 as well?

[davidbaumwald]

This is very close, and has a path forward, but with one remaining question. With 6.7 Beta 1 releasing shortly, this is being moved to 6.8 given recent momentum towards a resolution.

[audrasjb]

As per today's bug scrub: since this ticket didn't move during the 6.8 cycle, I'm moving it to milestone 6.9.

[logicrays]

The use case is clear, and the patch could help with syncing password changes.
However, could you clarify how the patch securely handles the password during propagation, especially given that WordPress doesn’t store cleartext passwords? Would love to see how this maintains security best practices.

I agree this feature would be very helpful in multi-system environments. Propagating password changes securely without relying on SSO is a valid use case, especially for legacy systems.
Supporting this in core would enable better integrations for developers.

Fires a wp_set_password action whenever a user is created or updated.

[oglekler]

The code needs to be reviewed before testing, so I am adding the 2nd-opinion.

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

Supersedes #7464.

[davidbaumwald]

In 60634:

Users: Fire wp_set_password action when creating or updating a user's password.

Various filters and actions fire during user creation and editing, making available all manner of user data to be acted upon by custom code. However, a user's password was not included in the data that was made available.

This change now fires an existing action, wp_set_password, during initial user creation and when an existing user's password is updated.

Props ChloeD, scribu, dd32, pento, chriscct7, johnbillion, logicrays, nimeshatxecurify.
Fixes #22114.

[dd32]

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/user.php?marks=2504-2507&rev=60634#L2504

@davidbaumwald [60634] has caused wp_insert_user() to pass $old_user_data as an array instead of an object.

[davidbaumwald]

@dd32 Since there is no "old" data for a user on initial creation, I see 3 options:

1. Update the docblock here to accept either false(which would indicate there was no previous user data) or a WP_User.
2. Pass an empty WP_User object, or
3. Pass the exact same, new WP_User object from the line above.

LMK what you think and if you see another, better option.

[davidbaumwald]

After chatting with Dion, when it's a new insert, we'll just pass the current WP_User obj. Most likely use-case here is checking old_password !== new_password. Follow-up commit needed.

[davidbaumwald]

In 60712:

Users: Pass WP_User object to wp_set_password action in during new user creation.

After [60634], wp_set_password is now fired during new user creation and existing user updates when a password is changed. wp_set_password expects the third argument to be a WP_User object of the user's data prior to the update. This change simply passes the newly created WP_User object.

Follow-up to [60634].

Props dd32.
See #22114.