Set $current_user global in wp_signon() after successful authentication

[fjarrett]

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 );

[ocean90]

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

[dotancohen]

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).

[dotancohen]

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...
    }
}

[hellofromTonya]

Hello 👋,

Thank you for the ticket and everyone who has contributed.

As previously state, it's not a defect, but rather by design.

By design, wp_signon() does not handle setting the current user. It's job is to check the credentials and set the authentication cookies as SergeyBiryukov notes here.

#28116 / [40943] added a note into the function's DocBlock explaining why and providing further guidance if a custom requirement is needed:

 * Note: wp_signon() doesn't handle setting the current user. This means that if the
 * function is called before the {@see 'init'} hook is fired, is_user_logged_in() will
 * evaluate as false until that point. If is_user_logged_in() is needed in conjunction
 * with wp_signon(), wp_set_current_user() should be called explicitly.

​https://developer.wordpress.org/reference/functions/wp_signon/

Why by design?

After logging in, a redirect happens, which clears in-memory status (including the globals). After the redirect is done, the $current_user global is empty. It's populated after plugins are loaded, setup_theme, and auth_cookie_valid all run.

Setting the current user within wp_signon() does not retain the current user in-memory once the login and redirect are done. Adding that to wp_signon() adds more tasks to the process without a major benefit for the majority users.

While I understand the need, there are hooks available for customization.

Since this is not a defect, but by design, and it's been 4 years since last activity, I'm closing this ticket. The classification of invalid seems harsh (sorry about that), but it's used to denote tickets that are not a defect/bug (​see in the handbook).

Thank you again everyone!