Make wp-login.php form pluggable

[misterbisson]

Allowing plugins to override functions like retrieve_password() and reset_password() would help me integrate WP (well, BuddyPress, really) into my environment (academic institution with existing identity framework).

[ryan]

There's another patch somewhere that makes mouch of wp-login pluggable and checks for a login.php type file in the current theme? Would that approach be better so we're not putting stuff in pluggable that is used only by wp-login.php?

[ryan]

#4478

[Denis-de-Bernardy]

while they don't need to be pluginable, they're probably missing a hook or two.

picture someone creating a membership plugin or a forum plugin that needs to override the retrieve_password() because it won't filter the message, for instance.

patch coming up in a few seconds.

[misterbisson]

My application requires they either be replaceable, or there be more sophisticated hooks.

I need to replace reset_password(), for instance, so that it sends the message to user_email as well as an alternate email address I'm managing via a plugin. (user_email is being set to a person's on campus address, but if they can't log in to WP, they probably also can't get a reset email there.) Additionally, I'm also sending a reset code to the user's cell phone.

I also need to replace retrieve_password() and make the switch( $action ) pluggable because:

1: the reset code sent to the user's phone is different (shorter) than the one sent by email and so needs to be verified differently
2: sending a password back by email is unacceptable in my environment, so I then need to re-verify the username and allow the user to immediately set a new password.

[misterbisson]

Makes the switch( $action ) part of wp-login.php hookable, moves functions defined in wp-login.php to wp-includes/pluggable.php so they can be overridden

[misterbisson]

My latest diff still moves the functions to pluggable.php, but the real point is the following addition just above the main switch( $action ):

global $wp_filter;
if( isset( $wp_filter[ 'wp_login_'. $action ] )):
	do_action( 'wp_login_'. $action );
else:
switch( $action ) {...

This allows plugins to hook into that and, perhaps, override the existing functionality. It has to check if there's an action attached to prevent leaving the user with a blank screen because somebody added an invalid action to the get vars.

That solution could eliminate my need to move the functions, as I could replace them with the action switch.

[Denis-de-Bernardy]

I've updated my patch based on your first feedback. looking into your patch and the rest of the feedback right away.

reset password is extended with:

do_action('password_reset', $user, $new_pass);

... so you can send the new password to wherever you want it to go.

both of the emails can be now cancelled by returning an empty message. in other words, you can disable the WP functionality in the sense that, WP will still do its stuff but it won't show to the user.

this hook seems valid to generate a cell phone key on the fly:

do_action('retrieve_password_key', $user_login, $key);

just ignore the $key and generate yours.

the last change is at the very end of the file: I've changed the default case in the switch statement, so as to make the entire thing pluggable.

it needs a little testing on that last point. please give it a roll quickly, so we know if anything else is needed. I'd love to see this one get into 2.8 as well, as a matter of fact, because the last point makes the entire screen overriddable.

[Denis-de-Bernardy]

the check on wp_filter isn't always valid. if a callback is added and then removed from that hook.

[Denis-de-Bernardy]

action validation, with filter on top

[Denis-de-Bernardy]

please give 9682.2.diff a whirl. it should even allow to replace the login form if you're not happy with it.

[Denis-de-Bernardy]

make login, rather than an empty string, the default action

[misterbisson]

Putting the do_action() at the end of the switch doesn't allow plugins to override the built-in action functions. This is important because the existing functions really don't do what they need to for my application, and the actions don't help.

problems:

The retrieve_password_key action only gets fired if the user_activation_key in the users table is empty.

The $key stored in the users table never expires (so far as I can tell).

The $key stored in the users table will be re-used for future password resets (as far as I can tell).

The password_reset action happens after the $key has been validated (preventing plugins from doing their own validation/expiration).

[misterbisson]

The solution in your last diffs is what I'd first considered, but that either limits the action to the built-in ones or requires another hook to set allowable actions.

Understanding your point about actions being set and then removed, the following should work:

global $wp_filter;
if( is_array( $wp_filter[ 'wp_login_'. $action ] ) && count( $wp_filter[ 'wp_login_'. $action ] )):

[misterbisson]

My mistake. I didn't read your last patch well enough. Yes, it allows any number of actions.

[misterbisson]

false !== has_filter('login_form_' . $action) )

Needs to be

false == has_filter('login_form_' . $action) )

[Denis-de-Bernardy]

Replying to misterbisson:

The retrieve_password_key action only gets fired if the user_activation_key in the users table is empty.

The $key stored in the users table never expires (so far as I can tell).

The $key stored in the users table will be re-used for future password resets (as far as I can tell).

actually... it's unset in wp_set_password(). so that it only gets fired once -- until used up. this is probably desirable, too, so as to avoid getting repeated email if the form is the target of a robot.

that it doesn't expire is a potential issue, but it's going to trigger such a debate in trac that it probably belongs in a separate ticket.

The password_reset action happens after the $key has been validated (preventing plugins from doing their own validation/expiration).

shouldn't this be the job of a separate action? that way you get to keep the best of both worlds: if the user is sent an email by WP and an SMS, the first would send him to ?action=rp&key=foo, and the second would send him to ?action=smsrp&smskey=foo.

you'd generate smskey on the fly on do_action(retrieve_password) or do_action(retrieve_password_key), whichever makes the most sense, and proceed as needed from there.

you're correct on the has_filter. fixing this right away.

[Denis-de-Bernardy]

fix the has_filter check

[Denis-de-Bernardy]

it needs to be ===, though, because has_filter can return 0 if the first filter has that priority.

[Denis-de-Bernardy]

yeah, it lets you do any action, and even override the default login screen and other actions with whatever you want as a bonus. :-)

[misterbisson]

Replying to Denis-de-Bernardy:

The password_reset action happens after the $key has been validated (preventing plugins from doing their own validation/expiration).

shouldn't this be the job of a separate action? that way you get to keep the best of both worlds: if the user is sent an email by WP and an SMS, the first would send him to ?action=rp&key=foo, and the second would send him to ?action=smsrp&smskey=foo.

If WP expired the reset codes, then maybe, but because expiring is a required feature for me....

I've actually developed a framework to handle this and similar actions: ​http://maisonbisson.com/blog/post/13862/wordpress-action-ticketing-api/. Since we seem to be working in such a similar space, I'd appreciate your feedback.

Your latest patch allows me to override the built in actions, though, so I'm good.

[ryan]

Patch not applying cleanly.

[Denis-de-Bernardy]

will get this fixed later today

[Denis-de-Bernardy]

against trunk @ 11219

[ryan]

(In [11291]) Make login more pluggable. Props Denis-de-Bernardy. fixes #9682