Make WordPress Core

Opened 2 months ago

#62996 new defect (bug)

register_new_user() unintentionally triggering 'profile_update' hook

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.3
Component: Login and Registration Keywords: 2nd-opinion needs-patch
Focuses: Cc:

Description

When a new user account is registered, both the user_register hook (from 2.0.0) and the profile_update hook (from 1.5.0) are being executed (inside of wp_insert_user().)

Between versions 2.0.0 and 5.3.0, these two hooks were frequently used to differentiate between a new user being created and an existing user being updated.

After version 5.3.0, profile_update fires on both new & update, breaking the previous expectation of separation.


Replication

  1. Single site installation
  2. Enable debug logging via your preferred method
  3. Enable registration in Settings > General
  4. Activate this plugin:
    <?php
    /*
    Plugin Name: Profile Update Logger
    Description: Logs a message whenever a user's profile is updated.
    Version: 1.0
    Author: JJJ
    */
    
    function log_profile_update( $user_id, $old_user_data ) {
        error_log( 'User profile updated. User ID: ' . $user_id );
    }
    add_action( 'profile_update', 'log_profile_update', 10, 2 );
    
  5. Manually register a new user account via wp-login.php?action=register, or run the following WP-CLI eval:
    wp eval 'register_new_user( "jjj", "jjj@example.com" );'
    

In your debug log, you should see that the user profile was updated for the newly registered user, even though that new user did not actually update their own profile.


Results

WordPress core is calling default_password_nag_edit_user() on every sign-up, and it isn't useful in that context.

I confirmed that BuddyPress, bbPress, and Easy Digital Downloads use profile_update to target user profile updates only, and have quite a bit of extra code unintentionally running on user sign-ups.


Origin

r45714 specifically.

#45746, #45845, and tangentially #33587.

In summary, direct database queries were replaced with API equivalents, and hooks were put in place to abstract direct function calls (related to sending notification emails), which generally is good and makes sense, but has resulted in wp_insert_user() being called twice in the same request.

Below is an abbreviated call-stack of the relevant functions & hooks:

  • register_new_user()
    • wp_insert_user()
      • do_action( 'user_register' );
    • do_action( 'register_new_user' );
      • wp_new_user_notification()
        • get_password_reset_key()
          • wp_insert_user()
            • do_action( 'profile_update' );

Fix

Due to age & limited footprint, I'm not entirely sure.

We can accept this regression as inherited, since it's been multiple years without complaint. I don't love this idea, because it leaves WordPress without a way to hook into user updates & sign-ups for different purposes.

We can revert r45714, making sure to test that caches are behaving. This will technically break the hook behavior in this call-stack for WordPress 5.3.0 to present, so anyone expecting this second call to wp_insert_user() would experience breakage.

We can rethink the current order-of-operations and combine both wp_insert_user() calls into a single one. I think this is the ideal fix, but I can already imagine some back-compat issues with it, too.

Change History (0)

Note: See TracTickets for help on using tickets.