-
Notifications
You must be signed in to change notification settings - Fork 175
Description
Describe the bug
Prior to 0.15.0, Jetpack SSO and Two Factor worked together without conflict. Users who authenticated via WordPress.com SSO (which has its own 2FA) were not prompted for local Two Factor verification. This worked through two mechanisms:
-
filter_authenticate_block_cookies(added in Block sending authentication cookies upon 2FA login #502 by dd32) only suppressed auth cookies whendid_action( 'login_init' )was true — the inline comment explicitly noted this was "to ensure we're within the regular login flow, rather than through an unsupported 3rd-party login process." SSO authenticates outsidewp-login.php, sologin_inithadn't fired. -
The
wp_loginaction (which triggers the 2FA prompt viashow_two_factor_login()) fired globally, but Jetpack SSO authenticates users and sets cookies through its own path without callingwp_signon(), so this hook never fired in that context.
PR #660 refactored this by consolidating the two authenticate filters into one and replacing the did_action('login_init') guard with a strlen($username) check — SSO doesn't pass a username through the authenticate filter, so the effect was the same. The wp_login hook was also moved to conditional registration inside filter_authenticate, so it only fired for username/password logins.
Then #793 reversed the key parts of #660: removed the $username parameter and strlen($username) guard, moved both the authenticate filter and wp_login hook back to unconditional registration in add_hooks(), and removed the argument count from the filter registration. The net result is that filter_authenticate now fires for any WP_User with 2FA enabled regardless of how they authenticated, and the wp_login hook fires unconditionally at PHP_INT_MAX.
For SSO users, this means the 2FA flow is triggered even though they've already completed authentication (including 2FA) on WordPress.com. I see the WP_Error data rendered as a raw object at the top of the page followed by a local 2FA prompt.
These two plugins have worked side-by-side for years, and I'd guess other plugins that authenticate users outside the username/password flow (OAuth, SAML, etc.) are probably affected the same way.
For reference, there's some prior history here: #84 and #122.
Steps to Reproduce
- Install and activate Jetpack (with SSO enabled) and Two Factor.
- Configure a user with a Two Factor method (e.g., TOTP) and connect their account to WordPress.com via Jetpack.
- Log out.
- Click "Log in with WordPress.com" on the login screen.
- Complete WordPress.com authentication (including WP.com's own 2FA if enabled).
- On redirect back, instead of being logged in, you see a raw object dump and/or the local Two Factor prompt.
Screenshots, screen recording, code snippet
The guard in #502 (2023, survived until 0.15.0):
public static function filter_authenticate_block_cookies( $user ) {
/*
* NOTE: The `login_init` action is checked for here to ensure we're within the regular login flow,
* rather than through an unsupported 3rd-party login process which this plugin doesn't support.
*/
if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && did_action( 'login_init' ) ) {
add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX );
}
return $user;
}The #660 replacement (also worked):
public static function filter_authenticate( $user, $username, $password ) {
if ( strlen( $username ) && $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) {Current code after #793 (no guard):
public static function filter_authenticate( $user ) {
if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) {I don't know what the right fix looks like from Two Factor's side — restoring strlen($username) might re-introduce whatever #793 was solving. A filter (something like two_factor_user_requires_authentication) that external auth plugins could hook to signal "2FA was already handled" might be a more durable solution, but that's just one idea. Open to whatever makes sense for the project.
Environment information
- WordPress trunk (6.8-alpha)
- Two Factor 0.15.0
- Jetpack (trunk)
Metadata
Metadata
Assignees
Labels
Type
Projects
Status