Opened 17 months ago
Last modified 2 weeks ago
#62996 new defect (bug)
register_new_user() unintentionally triggering 'profile_update' hook
| Reported by: | johnjamesjacoby | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Login and Registration | Version: | 5.3 |
| Severity: | normal | Keywords: | 2nd-opinion needs-patch |
| Cc: | Focuses: |
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
- Single site installation
- Enable debug logging via your preferred method
- Enable registration in Settings > General
- 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 ); - 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 (2)
#2
@
2 weeks ago
I traced this a bit and I think the issue is not really default_password_nag_edit_user() itself. That callback is just exposing a larger side effect.
During registration, the new user is created normally and user_register fires as expected. After that, the new-user notification email needs a password setup link. Generating that link saves a password reset key to user_activation_key.
Since [45714] / #45746, that reset-key save goes through the normal user update path instead of only updating the reset key. As a result, WordPress treats the internal reset-key write as a profile update, and profile_update fires even though no profile data was changed.
That also explains why this happens on wp-login.php?action=lostpassword: it uses the same reset-key generation path. So the regression is broader than registration. Any code path that generates and stores a password reset key can currently trigger profile_update.
I think the important distinction here is that user_activation_key is internal account state, not user profile data. Historically, plugins could use user_register for new accounts and profile_update for actual profile edits. This behavior blurs that separation and can cause profile-update integrations to run during signups/password resets.
I would avoid fully reverting [45714], because the cache/API consistency concern from #45746 still seems valid. A narrower fix may be to update only user_activation_key in the reset-key path and explicitly clean the user cache afterward, preserving the #45746 fix without firing profile_update.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
This is also happening when submitting a password reset via
wp-login.php?action=lostpassword.