WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 14 months ago

#39385 new defect (bug)

Set $current_user global in wp_signon() after successful authentication

Reported by: fjarrett Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: needs-patch close
Focuses: Cc:
PR Number:

Description

The $current_user global should be set after successful authentication inside wp_signon() instead of waiting for the next load of WordPress.

Although the $user_login string and $user object are passed through the wp_login hook, there are some functions that don't allow user parameters and rely solely on get_current_user_id(), such as wp_destroy_other_sessions().

This is easy enough to work around, but it seems reasonable to expect that user-related function calls should "just work" at any point after authentication.

<?php

add_action( 'wp_login', function ( $user_login, $user ) {

        var_dump( get_current_user_id() ); // int(0)

        $GLOBALS['current_user'] = $user;

        var_dump( get_current_user_id() ); // int(1)

        exit;

}, 10, 2 );

Attachments (2)

39385.patch (396 bytes) - added by fjarrett 3 years ago.
39385.2.patch (399 bytes) - added by dotancohen 14 months ago.

Download all attachments as: .zip

Change History (6)

#1 @johnbillion
3 years ago

  • Keywords needs-patch added
  • Version trunk deleted

@fjarrett
3 years ago

#2 @ocean90
3 years ago

  • Keywords close added

See #28116 and #35488 for why this is more of a documentation issue than an actual bug.

#3 @dotancohen
14 months ago

This is not a documentation error for the simple reason that the wp_login action does not have the current user set, even though it ostensibly "Fires after the user has successfully logged in".

The current patches don't use the core Wordpress function for setting the current user, even though it pretty much does the same thing. I feel that the attached patch is better from a maintainability standpoint by using the appropriate WP API. It is also updated to the current WP code base (just different line numbers in the diff, though).

#4 @dotancohen
14 months ago

To preempt arguments that the wp_login does not need to have the current user set because one of its parameters is the logged-in user, I counter that the code in the hooked method may (as, _does_ on one system I'm writing) can a non-coupled class that itself _does_ require the current user to be set.

add_action('wp_login', 'actionWpLogin', 10, 2);

function actionWpLogin($user_login, $user)
{
    Foo::bar();
}


/**
 * A class that I don't maintain.
 */
class Foo {
    public static function bar()
    {
        $user = wp_get_current_user();
        // Some $user magic here...
    }
}
Note: See TracTickets for help on using tickets.