From 24b1f8277e8848d43ab0812798bf70ed444d96cb Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Wed, 18 Oct 2023 19:32:29 -0300 Subject: [PATCH] Use the determine_current_user filter to record activity; Use the wp_is_application_passwords_available_for_user filter to block REST requests with application passwords --- security/class-user-last-seen.php | 73 +++++++++------- tests/security/test-class-user-last-seen.php | 88 +++++++++++++------- 2 files changed, 104 insertions(+), 57 deletions(-) diff --git a/security/class-user-last-seen.php b/security/class-user-last-seen.php index 87908ff302..de4457a747 100644 --- a/security/class-user-last-seen.php +++ b/security/class-user-last-seen.php @@ -20,12 +20,11 @@ public function init() { // Use a global cache group since users are shared among network sites. wp_cache_add_global_groups( array( self::LAST_SEEN_CACHE_GROUP ) ); - add_action( 'set_current_user', array( $this, 'record_activity' ) ); - add_filter( 'rest_authentication_errors', array( $this, 'rest_authentication' ), PHP_INT_MAX, 1 ); + add_filter( 'determine_current_user', array( $this, 'record_activity' ), 30, 1 ); add_action( 'admin_init', array( $this, 'register_release_date' ) ); add_action( 'set_user_role', array( $this, 'user_promoted' ) ); - add_action( 'vip_support_user_added', function( $user_id ) { + add_action( 'vip_support_user_added', function ( $user_id ) { $ignore_inactivity_check_until = strtotime( '+2 hours' ); $this->ignore_inactivity_check_for_user( $user_id, $ignore_inactivity_check_until ); @@ -43,6 +42,8 @@ public function init() { if ( $this->is_block_action_enabled() ) { add_filter( 'authenticate', array( $this, 'authenticate' ), 20, 1 ); + add_filter( 'wp_is_application_passwords_available_for_user', array( $this, 'application_password_authentication' ), PHP_INT_MAX, 2 ); + add_filter( 'rest_authentication_errors', array( $this, 'rest_authentication_errors' ), PHP_INT_MAX, 1 ); add_filter( 'views_users', array( $this, 'add_blocked_users_filter' ) ); add_filter( 'views_users-network', array( $this, 'add_blocked_users_filter' ) ); @@ -52,28 +53,32 @@ public function init() { } } - public function record_activity( $user = null ) { - if ( $user === null ) { - $user = wp_get_current_user(); + public function record_activity( $user_id ) { + if ( ! $user_id ) { + return $user_id; } - if ( ! $user || ! $user->ID ) { - return; + $user = get_userdata( $user_id ); + if ( ! $user ) { + return $user_id; } - if ( ! $this->user_with_elevated_capabilities( $user ) ) { - return; + if ( $this->is_considered_inactive( $user_id ) ) { + // User needs to be unblocked first + return $user_id; } - if ( wp_cache_get( $user->ID, self::LAST_SEEN_CACHE_GROUP ) ) { + if ( wp_cache_get( $user_id, self::LAST_SEEN_CACHE_GROUP ) ) { // Last seen meta was checked recently - return; + return $user_id; } // phpcs:ignore WordPressVIPMinimum.Performance.LowExpiryCacheTime.CacheTimeUndetermined - if ( wp_cache_add( $user->ID, true, self::LAST_SEEN_CACHE_GROUP, self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { - update_user_meta( $user->ID, self::LAST_SEEN_META_KEY, time() ); + if ( wp_cache_add( $user_id, true, self::LAST_SEEN_CACHE_GROUP, self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { + update_user_meta( $user_id, self::LAST_SEEN_META_KEY, time() ); } + + return $user_id; } public function authenticate( $user ) { @@ -94,23 +99,35 @@ public function authenticate( $user ) { return $user; } - public function rest_authentication( $status ) { - if ( is_wp_error( $status ) || ! $status ) { - return $status; - } + public function rest_authentication_errors( $status ) { + global $wp_last_seen_application_password_error; - $user = wp_get_current_user(); - if ( ! $user->ID ) { - return $status; + if ( is_wp_error( $wp_last_seen_application_password_error ) ) { + return $wp_last_seen_application_password_error; } - if ( $this->is_block_action_enabled() && $this->is_considered_inactive( $user->ID ) ) { - return new \WP_Error( 'inactive_account', __( 'Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ), array( 'status' => 403 ) ); + return $status; + } + + /** + * @param bool $available True if application password is available, false otherwise. + * @param \WP_User $user The user to check. + * @return bool + */ + public function application_password_authentication( $available, $user ) { + global $wp_last_seen_application_password_error; + + if ( ! $available || ( $user && ! $user->exists() ) ) { + return false; } - $this->record_activity( $user ); + if ( $this->is_considered_inactive( $user->ID ) ) { + $wp_last_seen_application_password_error = new \WP_Error( 'inactive_account', __( 'Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ), array( 'status' => 403 ) ); - return $status; + return false; + } + + return $available; } public function add_last_seen_column_head( $columns ) { @@ -221,7 +238,7 @@ public function last_seen_unblock_action() { $admin_notices_hook_name = is_network_admin() ? 'network_admin_notices' : 'admin_notices'; if ( isset( $_GET['reset_last_seen_success'] ) && '1' === $_GET['reset_last_seen_success'] ) { - add_action( $admin_notices_hook_name, function() { + add_action( $admin_notices_hook_name, function () { $class = 'notice notice-success is-dismissible'; $error = __( 'User unblocked.', 'wpvip' ); @@ -254,7 +271,7 @@ public function last_seen_unblock_action() { } if ( $error ) { - add_action( $admin_notices_hook_name, function() use ( $error ) { + add_action( $admin_notices_hook_name, function () use ( $error ) { $class = 'notice notice-error is-dismissible'; printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); @@ -352,7 +369,7 @@ private function should_check_user_last_seen( $user_id ) { $user = get_userdata( $user_id ); if ( ! $user ) { - throw new \Exception( 'User not found' ); + throw new \Exception( sprintf( 'User #%d found', esc_html( $user_id ) ) ); } if ( $user->user_registered && strtotime( $user->user_registered ) > $this->get_inactivity_timestamp() ) { diff --git a/tests/security/test-class-user-last-seen.php b/tests/security/test-class-user-last-seen.php index 38dafec768..3a084eb326 100644 --- a/tests/security/test-class-user-last-seen.php +++ b/tests/security/test-class-user-last-seen.php @@ -87,6 +87,7 @@ public function test__is_considered_inactive__should_consider_user_registered() public function test__is_considered_inactive__add_extra_time_when_user_is_promoted() { Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); $user_id = $this->factory()->user->create( array( 'role' => 'subscriber', @@ -105,6 +106,7 @@ public function test__is_considered_inactive__add_extra_time_when_user_is_promot public function test__is_considered_inactive__should_consider_user_meta() { Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); $user_inactive_id = $this->factory()->user->create( array( 'role' => 'administrator', @@ -127,6 +129,7 @@ public function test__is_considered_inactive__should_consider_user_meta() { public function test__is_considered_inactive__should_return_false_if_user_meta_and_option_are_not_present() { Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); delete_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); @@ -161,6 +164,7 @@ public function test__is_considered_inactive__should_use_release_date_option_whe public function test__authenticate_should_not_return_error_when_user_is_active() { Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); remove_all_filters( 'authenticate' ); @@ -180,6 +184,7 @@ public function test__authenticate_should_not_return_error_when_user_is_active() public function test__authenticate_should_return_an_error_when_user_is_inactive() { Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); remove_all_filters( 'authenticate' ); @@ -199,31 +204,12 @@ public function test__authenticate_should_return_an_error_when_user_is_inactive( $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); } - public function test__rest_authentication_should_record_last_seen_meta() { - Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); - Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); - - remove_all_filters( 'rest_authentication_errors' ); - - $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); - - wp_set_current_user( $user_id ); - - $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); - $last_seen->init(); - - $result = apply_filters( 'rest_authentication_errors', true ); - - $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); - - $this->assertIsNumeric( $current_last_seen ); - $this->assertTrue( $result ); - } - public function test__rest_authentication_should_return_an_error_when_user_is_inactive() { Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); + remove_all_filters( 'wp_is_application_passwords_available_for_user' ); remove_all_filters( 'rest_authentication_errors' ); $user_id = $this->factory()->user->create( array( @@ -231,36 +217,49 @@ public function test__rest_authentication_should_return_an_error_when_user_is_in 'user_registered' => '2020-01-01', ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-100 days' ) ); + $user = get_user_by( 'id', $user_id ); wp_set_current_user( $user_id ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); - $result = apply_filters( 'rest_authentication_errors', true ); + $available = apply_filters( 'wp_is_application_passwords_available_for_user', true, $user ); + $this->assertFalse( $available ); + + $rest_authentication_errors = apply_filters( 'rest_authentication_errors', true ); - $this->assertWPError( $result, 'Expected WP_Error object to be returned' ); + $this->assertSame( 'inactive_account', $rest_authentication_errors->get_error_code() ); } public function test__rest_authentication_should_not_block_when_action_is_not_block() { - Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'REPORT' ); Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-100 days' ) ); + remove_all_filters( 'wp_is_application_passwords_available_for_user' ); remove_all_filters( 'rest_authentication_errors' ); + remove_all_filters( 'determine_current_user' ); - $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + $user_id = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-16 days' ) ); + + $user = get_user_by( 'id', $user_id ); wp_set_current_user( $user_id ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); - $result = apply_filters( 'rest_authentication_errors', true ); + $available = apply_filters( 'wp_is_application_passwords_available_for_user', true, $user ); + $this->assertTrue( $available ); - $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + $rest_authentication_errors = apply_filters( 'rest_authentication_errors', true ); - $this->assertIsNumeric( $current_last_seen ); - $this->assertTrue( $result ); + $this->assertNotWPError( $rest_authentication_errors ); } public function test__register_release_date_should_register_release_date_only_once() { @@ -350,4 +349,35 @@ public function test__should_check_user_last_seen_should_call_skip_users_filters $this->assertSame( $user, apply_filters( 'authenticate', $user, $user ) ); } + + public function test__record_activity_should_be_stored_only_once() { + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); + + remove_all_filters( 'determine_current_user' ); + + $user_id = $this->factory()->user->create( array( + 'role' => 'subscriber', + 'user_registered' => '2020-01-01', + ) ); + $first_last_seen = strtotime( '-100 days' ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $first_last_seen ); + + wp_set_current_user( $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + apply_filters( 'determine_current_user', $user_id ); + $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + $this->assertIsNumeric( $current_last_seen ); + $this->assertNotEquals( $first_last_seen, $current_last_seen ); + + $test_value = 12345; + update_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $test_value ); + + apply_filters( 'determine_current_user', $user_id ); + $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + $this->assertEquals( $test_value, $current_last_seen ); + } }