diff --git a/projects/packages/connection/changelog/fix-sso-two-factor-bypass b/projects/packages/connection/changelog/fix-sso-two-factor-bypass new file mode 100644 index 000000000000..8fc84ec80ae6 --- /dev/null +++ b/projects/packages/connection/changelog/fix-sso-two-factor-bypass @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Fixed SSO login conflict with Two-Factor plugin 0.15.0+ that caused a redundant local 2FA prompt after completing WordPress.com 2FA. diff --git a/projects/packages/connection/src/sso/class-sso.php b/projects/packages/connection/src/sso/class-sso.php index a3bfc4c286b2..3bcb5a8eca62 100644 --- a/projects/packages/connection/src/sso/class-sso.php +++ b/projects/packages/connection/src/sso/class-sso.php @@ -40,6 +40,14 @@ class SSO { */ public static $instance = null; + /** + * Stores the WP_User being authenticated via SSO so the + * attach_session_information callback can tag the session. + * + * @var WP_User|null + */ + private static $sso_user_for_2fa = null; + /** * Automattic\Jetpack\Connection\SSO constructor. */ @@ -918,6 +926,46 @@ public function handle_login() { // Cache the user's details, so we can present it back to them on their user screen. update_user_meta( $user->ID, 'wpcom_user_data', $user_data ); + /* + * Two-Factor plugin 0.15.0+ unconditionally hooks wp_login at PHP_INT_MAX, + * which destroys the auth session and prompts for local 2FA — even for SSO + * logins that already completed 2FA on WordPress.com. + * + * When WP.com confirms the user has 2FA active, remove Two-Factor's wp_login + * hook so SSO can complete without a redundant local 2FA prompt. + * + * When WP.com 2FA is NOT active, the hook stays and Two-Factor can enforce + * local 2FA as a safety net. + * + * @see https://github.com/WordPress/two-factor/issues/811 + */ + /** + * Filter whether to accept WordPress.com 2FA in place of a local + * Two-Factor prompt during SSO login. + * + * Return false to always require the local Two-Factor prompt, + * even when the user has completed 2FA on WordPress.com. + * + * @since $$next-version$$ + * @module sso + * + * @param bool $accept Whether to accept WP.com 2FA. Default true. + * @param object $user_data WordPress.com user data from SSO validation. + * @param WP_User $user The local WordPress user. + */ + $accept_wpcom_2fa = apply_filters( 'jetpack_sso_accept_wpcom_2fa', true, $user_data, $user ); + + if ( + ! empty( $user_data->two_step_enabled ) + && class_exists( 'Two_Factor_Core' ) + && $accept_wpcom_2fa + ) { + self::$sso_user_for_2fa = $user; + add_filter( 'attach_session_information', array( static::class, 'add_two_factor_session_meta' ), 10, 2 ); + + remove_action( 'wp_login', array( 'Two_Factor_Core', 'wp_login' ), PHP_INT_MAX ); + } + add_filter( 'auth_cookie_expiration', array( Helpers::class, 'extend_auth_cookie_expiration_for_sso' ) ); wp_set_auth_cookie( $user->ID, true ); remove_filter( 'auth_cookie_expiration', array( Helpers::class, 'extend_auth_cookie_expiration_for_sso' ) ); @@ -1273,4 +1321,19 @@ public function is_user_connected( $user_id ) { public function get_user_data( $user_id ) { return get_user_meta( $user_id, 'wpcom_user_data', true ); } + + /** + * Marks a session as two-factor-authenticated when SSO handled 2FA via WP.com. + * + * @param array $session Session information array. + * @param int $user_id User ID for the session being created. + * @return array Modified session information. + */ + public static function add_two_factor_session_meta( $session, $user_id ) { + if ( self::$sso_user_for_2fa && self::$sso_user_for_2fa->ID === $user_id ) { + $session['two-factor-login'] = time(); + self::$sso_user_for_2fa = null; + } + return $session; + } } diff --git a/projects/packages/connection/tests/php/sso/SSO_Test.php b/projects/packages/connection/tests/php/sso/SSO_Test.php new file mode 100644 index 000000000000..dc07aaab6fe2 --- /dev/null +++ b/projects/packages/connection/tests/php/sso/SSO_Test.php @@ -0,0 +1,115 @@ +setAccessible( true ); + $ref->setValue( null, null ); + + parent::tear_down(); + } + + /** + * Helper to create a WP user and return the WP_User object. + * + * @return \WP_User + */ + private function create_test_user() { + $user_id = wp_insert_user( + array( + 'user_login' => 'sso_test_' . wp_generate_password( 6, false ), + 'user_pass' => wp_generate_password(), + 'user_email' => 'sso_test_' . wp_generate_password( 6, false ) . '@example.com', + ) + ); + return get_userdata( $user_id ); + } + + /** + * Helper to set the private static $sso_user_for_2fa property. + * + * @param \WP_User|null $user User to set. + */ + private function set_sso_user_for_2fa( $user ) { + $ref = new \ReflectionProperty( SSO::class, 'sso_user_for_2fa' ); + $ref->setAccessible( true ); + $ref->setValue( null, $user ); + } + + /** + * Helper to get the private static $sso_user_for_2fa property. + * + * @return \WP_User|null + */ + private function get_sso_user_for_2fa() { + $ref = new \ReflectionProperty( SSO::class, 'sso_user_for_2fa' ); + $ref->setAccessible( true ); + return $ref->getValue(); + } + + /** + * Test that session is tagged with two-factor-login when user ID matches. + */ + public function test_add_two_factor_session_meta_tags_session_for_matching_user() { + $user = $this->create_test_user(); + $session = array( 'expiration' => time() + 3600 ); + + $this->set_sso_user_for_2fa( $user ); + $result = SSO::add_two_factor_session_meta( $session, $user->ID ); + + $this->assertArrayHasKey( 'two-factor-login', $result ); + $this->assertIsInt( $result['two-factor-login'] ); + } + + /** + * Test that the stored user is cleared after tagging (one-shot). + */ + public function test_add_two_factor_session_meta_clears_stored_user() { + $user = $this->create_test_user(); + $session = array(); + + $this->set_sso_user_for_2fa( $user ); + SSO::add_two_factor_session_meta( $session, $user->ID ); + + $this->assertNull( $this->get_sso_user_for_2fa() ); + } + + /** + * Test that session is unchanged when user ID does not match. + */ + public function test_add_two_factor_session_meta_skips_non_matching_user() { + $user = $this->create_test_user(); + $session = array( 'expiration' => time() + 3600 ); + + $this->set_sso_user_for_2fa( $user ); + $result = SSO::add_two_factor_session_meta( $session, $user->ID + 999 ); + + $this->assertArrayNotHasKey( 'two-factor-login', $result ); + $this->assertNotNull( $this->get_sso_user_for_2fa() ); + } + + /** + * Test that session is unchanged when no SSO user is stored. + */ + public function test_add_two_factor_session_meta_noop_when_no_user_stored() { + $session = array( 'expiration' => time() + 3600 ); + $result = SSO::add_two_factor_session_meta( $session, 1 ); + + $this->assertArrayNotHasKey( 'two-factor-login', $result ); + $this->assertEquals( $session, $result ); + } +}