WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 2 years ago

#22114 assigned feature request

Propagating password on change

Reported by: ChloeD Owned by: chriscct7
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.4.2
Component: Users Keywords: has-patch needs-refresh
Focuses: Cc:

Description

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.

Attachments (1)

patch-wordpress-users.patch (556 bytes) - added by ChloeD 5 years ago.
Patch (wp-includes/user.php) for propagating cleartext passwords through an action

Download all attachments as: .zip

Change History (13)

@ChloeD
5 years ago

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

#1 @scribu
5 years ago

  • Keywords close added

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

#2 @ChloeD
5 years ago

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.

#3 @scribu
5 years ago

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

#4 @ChloeD
5 years ago

  • Keywords 2nd-opinion added

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.

#5 follow-up: @scribu
5 years ago

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

#6 in reply to: ↑ 5 @bradparbs
5 years ago

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.

#7 @bradparbs
5 years ago

  • Cc brad@… added

#8 @dd32
3 years ago

  • Keywords close 2nd-opinion removed

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.

#9 follow-up: @pento
3 years ago

  • Keywords needs-patch added; has-patch removed

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.

#10 in reply to: ↑ 9 @chriscct7
2 years ago

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?

#11 @pento
2 years ago

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

#12 @chriscct7
2 years ago

  • Keywords has-patch needs-refresh added; needs-patch removed
  • Owner set to chriscct7
  • Status changed from new to assigned
Note: See TracTickets for help on using tickets.