#28020 closed defect (bug) (maybelater)
get_userdata and wp_get_current_user do not share WP_User instance
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | Priority: | normal | |
| Severity: | normal | Version: | |
| Component: | Users | Keywords: | has-patch has-unit-tests early |
| Focuses: | rest-api | Cc: |
Description
Just discovered this fun one.
Steps to reproduce:
$uid = 3;
// UID for this demo should be a subscriber
wp_set_current_user( $uid );
// $GLOBALS['current_user'] now contains a WP_User instance
$from_userdata = get_userdata( $uid );
// $from_userdata now also contains a WP_User instance, but not the same one
// Fun times with desynchronisation:
$from_userdata->set_role( 'administrator' );
assert(current_user_can('administrator'));
(I suspect you can also reproduce by using wp_update_user instead of get_userdata)
This is probably due to the fact that we don't use object caching for the user object instances, which would let us share the instances.
Attachments (3)
Change History (29)
#3
@
6 years ago
- Keywords has-patch has-unit-tests added; needs-patch removed
So here's how I approached the problem - see my attached patch 28020.diff.
- Check if what is being fetched inside the function
get_user_by()is the same as the current user. If it is, then return the current user object usingwp_get_current_user().
if ( get_current_user_id() == $userdata->ID ) {
return wp_get_current_user();
}
However, after I added the code above and ran the full test suite, bunch of failed tests emerged.
- After debugging, I found out that the failed tests' origin is from the function
clean_user_cache(). The code from no. 1 above returns the current user object instead of a new user object with updated data. So functions that transform the current state of a user object such aswp_set_password()fails if the user in context is the current user.
To solve this, the current user object should be re-setup / re-created if it is passed to clean_user_cache(). Hence the code.
if ( get_current_user_id() == $user->ID ) {
wp_set_current_user( $user->ID, '', true );
}
- Lastly, I added a new param -
$forcewith default value offalse- in the functionwp_set_current_user(). It is becausewp_set_current_user()returns the same current user object if the passed$idis the same as the current user. At first I thought of using
wp_set_current_user( 0 )
wp_set_current_user( $current_user_id )
The code above will first "remove" the current user effectively bypassing the check. However, I believe it's not good to go around like this. Another possible solution was to create another function which basically do the same as wp_set_current_user() just without the check so it will always re-setup the current user object. But then again, creating almost duplicate functions are just ticking bombs.
So ultimately, I settled on creating a new param $force and added it to the check.
Also, just to mention, I had to edit the test function getPluggableFunctionSignatures() and add the newly added $force param to pass the test.
'wp_set_current_user' => array(
'id',
'name' => '',
'force' => false // ticket 28020
),
I run the full test suite and it looks good.
OK, but incomplete, skipped, or risky tests!
Tests: 9640, Assertions: 73874, Skipped: 39
Any thoughts and suggestions are greatly appreciated.
References.
https://developer.wordpress.org/reference/functions/get_user_by/
https://developer.wordpress.org/reference/functions/wp_get_current_user/
https://developer.wordpress.org/reference/functions/clean_user_cache/
https://developer.wordpress.org/reference/functions/wp_set_current_user/
Just hit this again. Extremely annoying, and insane to debug.