Conflict with Jetpack when the wp_login hook option is enabled

[aaronware]

The following error is displayed when both wordpress-saml and jetpack are both enabled and the wp_login hook is also checked off. The user is still actually logged in w/ the appropriate role if they directly visit wp-admin.

`Fatal error: Uncaught Error: Too few arguments to function Automattic\Jetpack\Sync\Modules\Users::wp_login_handler(), 1 passed in /wp-includes/class-wp-hook.php on line 287 and exactly 2 expected in /wp-content/plugins/jetpack/vendor/automattic/jetpack-sync/src/modules/class-users.php on line 330

Call stack:

Automattic\J\S\M\Users::wp_login_handler() wp-includes/class-wp-hook.php:287 WP_Hook::apply_filters() wp-includes/class-wp-hook.php:311 WP_Hook::do_action() wp-includes/plugin.php:484 do_action() wp-content/plugins/onelogin-saml-sso/php/functions.php:465 saml_acs() wp-content/plugins/onelogin-saml-sso/php/functions.php:20 saml_checker() wp-includes/class-wp-hook.php:287 WP_Hook::apply_filters() wp-includes/class-wp-hook.php:311 WP_Hook::do_action() wp-includes/plugin.php:484 do_action() wp-settings.php:557 require_once() wp-config.php:141 require_once() wp-load.php:37 require() wp-login.php:12`

[pitbulk]

There is an option at the SAML Settings that triggers the wp_login action https://github.com/onelogin/wordpress-saml/blob/master/onelogin-saml-sso/php/functions.php#L465

    $triggerWPLoginHook = get_site_option('onelogin_saml_trigger_login_hook');
    if ($triggerWPLoginHook) {
        do_action( 'wp_login', $user->user_login, $user );
    }

I guess that disabling that setting you avoid that conflict

The same call is executed by the wp_signon method at the WP core https://github.com/WordPress/WordPress/blob/7ced0efbf4afa3c0eab3289596bcea9a4e367fca/wp-includes/user.php#L110 so assume the issue is on JetPack side

[aaronware]

Thanks for that. I saw that in the code as well. I can confirm that the option is disabled and the error no longer happens.

However, that creates other issues. Basically anything that DOES rely on the wp_login hook will not be fired. It appears that you are not setting the $user object in your function in a similar way that WordPress is.

https://github.com/WordPress/WordPress/blob/7ced0efbf4afa3c0eab3289596bcea9a4e367fca/wp-includes/user.php#L95

This will have issues with any plugin that is trying to hook into wp_login. For example, with the Plugin Stream, no records of users logging in will be recorded unless your hook is enabled.

I can always fork and implement the authenticate if you'd like provide a pull request.

[pitbulk]

I can't authenticate the user with wp_authenticate because I don't have the password.

The wp_authenticate uses authenticate which will trigger the method wp_authenticate_username_password which retrieve the $user with:

$user = get_user_by( 'login', $username );

I see now that I forgot to define the $user object, can you try adding:

            $user = get_user_by( 'id', $user_id );

so at https://github.com/onelogin/wordpress-saml/blob/master/onelogin-saml-sso/php/functions.php#L465

    if ($triggerWPLoginHook) {
            $user = get_user_by( 'id', $user_id );
        do_action( 'wp_login', $user->user_login, $user );
    }

I believe that should fix the issue.

[aaronware]

Yep, that's exactly what I was thinking as well. I already made a fork and was going to test that out. I'll confirm and let you know shortly.