From 006ac0e93b1ca673ba9ffda15673e5cc258ef138 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Wed, 9 Aug 2023 17:48:48 -0300 Subject: [PATCH 01/27] Add user last seen information --- security.php | 4 +++ security/user-last-seen.php | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 security/user-last-seen.php diff --git a/security.php b/security.php index 05424695c8..1c47a4f107 100644 --- a/security.php +++ b/security.php @@ -16,6 +16,10 @@ require_once __DIR__ . '/security/login-error.php'; require_once __DIR__ . '/security/password.php'; +if ( defined( 'VIP_USER_LAST_SEEN_ENABLED' ) ) { + require_once __DIR__ . '/security/user-last-seen.php'; +} + use Automattic\VIP\Utils\Context; define( 'CACHE_GROUP_LOGIN_LIMIT', 'vip_login_limit' ); diff --git a/security/user-last-seen.php b/security/user-last-seen.php new file mode 100644 index 0000000000..8a3fbd2f49 --- /dev/null +++ b/security/user-last-seen.php @@ -0,0 +1,60 @@ +ID; + + try { + if ( wp_cache_get( $cache_key ) ) { + return; + } + + update_user_meta( $current_user->ID, 'vip_last_seen', time() ); + + wp_cache_set( $cache_key, true, null, LAST_SEEN_UPDATE_USER_META_CACHE_TTL ); + } catch ( \Exception $e ) { + trigger_error( + sprintf( 'failed to update user last seen meta', esc_html( $e->getMessage() ) ), + E_USER_WARNING + ); + } +} + +function users_columns ($cols) { + $cols['last_seen'] = __( 'Last seen' ) ; + return $cols; +} + +function users_custom_column ( $default, $column_name, $user_id ) { + if ( 'last_seen' == $column_name ) { + $last_seen_timestamp = get_user_meta( $user_id, 'vip_last_seen', true ); + + if ( $last_seen_timestamp ) { + $formatted_date = sprintf( + __( '%1$s at %2$s' ), + date_i18n( get_option('date_format'), $last_seen_timestamp ), + date_i18n( get_option('time_format'), $last_seen_timestamp ) + ); + + if ( defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + $diff = ( new DateTime( sprintf('@%s', $last_seen_timestamp ) ) )->diff( new DateTime() ); + + if ( $diff->days >= constant( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + return sprintf( '%s', esc_html__( $formatted_date ) ); + } + } + + return sprintf( '%s', esc_html__( $formatted_date ) ); + } + } + + return $default; +} From 5fadcf7d81b62cab69f393be499298616986be8b Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Wed, 9 Aug 2023 19:18:54 -0300 Subject: [PATCH 02/27] Linting fixes --- security/user-last-seen.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 8a3fbd2f49..a8ee44c798 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -4,8 +4,8 @@ const LAST_SEEN_UPDATE_USER_META_CACHE_KEY_PREFIX = 'vip_last_seen_update_user_meta_cache_key'; add_action( 'set_current_user', 'update_user_last_seen', 10, 0 ); -add_filter ('manage_users_columns', 'users_columns' ) ; -add_filter ('manage_users_custom_column', 'users_custom_column', 10, 3 ) ; +add_filter('manage_users_columns', 'users_columns' ) ; +add_filter('manage_users_custom_column', 'users_custom_column', 10, 3 ) ; function update_user_last_seen() { global $current_user; @@ -28,12 +28,12 @@ function update_user_last_seen() { } } -function users_columns ($cols) { +function users_columns($cols) { $cols['last_seen'] = __( 'Last seen' ) ; return $cols; } -function users_custom_column ( $default, $column_name, $user_id ) { +function users_custom_column( $default, $column_name, $user_id ) { if ( 'last_seen' == $column_name ) { $last_seen_timestamp = get_user_meta( $user_id, 'vip_last_seen', true ); From e1227255d3b0c48c62fbcce37e8159263d9f2d10 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Mon, 14 Aug 2023 16:23:26 -0300 Subject: [PATCH 03/27] Code review suggestions --- security/user-last-seen.php | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index a8ee44c798..3c7eb3aca5 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -1,30 +1,28 @@ ID; + if ( ! $current_user->ID ) { + return; + } - try { - if ( wp_cache_get( $cache_key ) ) { - return; - } + $cache_key = LAST_SEEN_UPDATE_USER_META_CACHE_KEY_PREFIX . $current_user->ID; - update_user_meta( $current_user->ID, 'vip_last_seen', time() ); + if ( wp_cache_get( $cache_key, 'wpvip' ) ) { + // Last seen meta was updated recently, no need to update it again + return; + } - wp_cache_set( $cache_key, true, null, LAST_SEEN_UPDATE_USER_META_CACHE_TTL ); - } catch ( \Exception $e ) { - trigger_error( - sprintf( 'failed to update user last seen meta', esc_html( $e->getMessage() ) ), - E_USER_WARNING - ); + if ( wp_cache_add( $cache_key, true, 'wpvip', LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { + update_user_meta( $current_user->ID, 'wpvip_last_seen', time() ); } } @@ -35,7 +33,7 @@ function users_columns($cols) { function users_custom_column( $default, $column_name, $user_id ) { if ( 'last_seen' == $column_name ) { - $last_seen_timestamp = get_user_meta( $user_id, 'vip_last_seen', true ); + $last_seen_timestamp = get_user_meta( $user_id, 'wpvip_last_seen', true ); if ( $last_seen_timestamp ) { $formatted_date = sprintf( From 6de9dd955b40029f1ee01aa2d9b1253ae3311a51 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Tue, 15 Aug 2023 16:41:54 -0300 Subject: [PATCH 04/27] Lockdown inactive users --- security/user-last-seen.php | 136 +++++++++++++++++++++++++++++------- 1 file changed, 112 insertions(+), 24 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 3c7eb3aca5..196322f1dd 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -1,39 +1,58 @@ ID ) { - return; + if ( wp_cache_get( $cache_key, GROUP ) ) { + // Last seen meta was checked recently + return $user_id; } - $cache_key = LAST_SEEN_UPDATE_USER_META_CACHE_KEY_PREFIX . $current_user->ID; + $is_api_request = ( ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) || ( defined( 'REST_REQUEST' ) && REST_REQUEST ) ); - if ( wp_cache_get( $cache_key, 'wpvip' ) ) { - // Last seen meta was updated recently, no need to update it again - return; + if ( $is_api_request && is_considered_inactive( $user_id ) ) { + // To block API requests for inactive requests, we need to return a WP_Error object here + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) ); } - if ( wp_cache_add( $cache_key, true, 'wpvip', LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { - update_user_meta( $current_user->ID, 'wpvip_last_seen', time() ); + if ( wp_cache_add( $cache_key, true, GROUP, LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { + update_user_meta( $user_id, LAST_SEEN_META_KEY, time() ); } -} -function users_columns($cols) { - $cols['last_seen'] = __( 'Last seen' ) ; + return $user_id; +}, 30, 1 ); + +add_filter( 'authenticate', function( $user, string $username, string $password ) { + if ( is_wp_error( $user ) ) { + return $user; + } + + if ( $user->ID && is_considered_inactive( $user->ID ) ) { + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) );; + } + + return $user; +}, 20, 3 ); + + +add_filter( 'manage_users_columns', function ( $cols ) { + $cols['last_seen'] = __( 'Last seen' ); return $cols; -} +} ); -function users_custom_column( $default, $column_name, $user_id ) { +add_filter( 'manage_users_custom_column', function ( $default, $column_name, $user_id ) { if ( 'last_seen' == $column_name ) { - $last_seen_timestamp = get_user_meta( $user_id, 'wpvip_last_seen', true ); + $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); if ( $last_seen_timestamp ) { $formatted_date = sprintf( @@ -42,12 +61,17 @@ function users_custom_column( $default, $column_name, $user_id ) { date_i18n( get_option('time_format'), $last_seen_timestamp ) ); - if ( defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { - $diff = ( new DateTime( sprintf('@%s', $last_seen_timestamp ) ) )->diff( new DateTime() ); + if ( is_considered_inactive( $user_id ) ) { + $unblock_link = ''; + if ( current_user_can( 'edit_user', array() ) ) { + $url = add_query_arg( array( + 'action' => 'reset_last_seen', + 'user_id' => $user_id, + ) ); - if ( $diff->days >= constant( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { - return sprintf( '%s', esc_html__( $formatted_date ) ); + $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; } + return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); } return sprintf( '%s', esc_html__( $formatted_date ) ); @@ -55,4 +79,68 @@ function users_custom_column( $default, $column_name, $user_id ) { } return $default; +}, 10, 3 ); + +add_action( 'user_row_actions', function ( $actions ) { + if( isset($_GET['action'] ) && $_GET['action'] === 'reset_last_seen' ){ + $user_id = $_GET['user_id']; + delete_user_meta( $user_id, LAST_SEEN_META_KEY ); + } + + return $actions; +}, 10, 1 ); + +add_filter( 'views_users', function ( $views ) { + global $wpdb; + + if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + return $views; + } + + $count = $wpdb->get_var( 'SELECT COUNT(meta_key) FROM ' . $wpdb->usermeta . ' WHERE meta_key = "' . LAST_SEEN_META_KEY . '" AND meta_value < ' . get_inactivity_timestamp() ); + + $view = __( 'Blocked Users' ); + if ( $count ) { + $class = $_REQUEST[ 'last_seen_filter' ] ? 'current' : ''; + $view = '' . $view . ''; + } + $views['blocked_users'] = $view . ' (' . $count . ')'; + + return $views; +} ); + +add_filter( 'users_list_table_query_args', function ( $args ) { + if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + return $args; + } + + if ( isset( $_REQUEST[ 'last_seen_filter' ] ) ) { + $args[ 'meta_key' ] = LAST_SEEN_META_KEY; + $args[ 'meta_value' ] = get_inactivity_timestamp(); + $args[ 'meta_type' ] = 'NUMERIC'; + $args[ 'meta_compare' ] = '<'; + } + + return $args; +}, 9999 ); + +function is_considered_inactive( $user_id ) { + if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + return false; + } + + $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); + if ( ! $last_seen_timestamp ) { + return false; + } + + return $last_seen_timestamp < get_inactivity_timestamp(); +} + +function get_inactivity_timestamp() { + if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + return 0; + } + + return strtotime( sprintf('-%d days', constant( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } From e920989d65b955f80dfb8ba10a85f71eb8b08d1a Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Tue, 15 Aug 2023 18:02:29 -0300 Subject: [PATCH 05/27] Fix priority --- security/user-last-seen.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 196322f1dd..03f24fdc95 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -122,7 +122,7 @@ } return $args; -}, 9999 ); +} ); function is_considered_inactive( $user_id ) { if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { From 5a4a93dde7f3b28fea167537da6b62cb1c13d97d Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Tue, 15 Aug 2023 18:04:49 -0300 Subject: [PATCH 06/27] Fix filter --- security/user-last-seen.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 03f24fdc95..9aae99a10e 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -101,7 +101,7 @@ $view = __( 'Blocked Users' ); if ( $count ) { - $class = $_REQUEST[ 'last_seen_filter' ] ? 'current' : ''; + $class = isset( $_REQUEST[ 'last_seen_filter' ] ) ? 'current' : ''; $view = '' . $view . ''; } $views['blocked_users'] = $view . ' (' . $count . ')'; From 2b5f8f63e400ca661cc6863b1f809ae6a6f9d6e4 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Tue, 15 Aug 2023 20:01:57 -0300 Subject: [PATCH 07/27] Simplify code execution flow --- security/user-last-seen.php | 58 +++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 9aae99a10e..ef65d2e923 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -51,34 +51,36 @@ } ); add_filter( 'manage_users_custom_column', function ( $default, $column_name, $user_id ) { - if ( 'last_seen' == $column_name ) { - $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); - - if ( $last_seen_timestamp ) { - $formatted_date = sprintf( - __( '%1$s at %2$s' ), - date_i18n( get_option('date_format'), $last_seen_timestamp ), - date_i18n( get_option('time_format'), $last_seen_timestamp ) - ); - - if ( is_considered_inactive( $user_id ) ) { - $unblock_link = ''; - if ( current_user_can( 'edit_user', array() ) ) { - $url = add_query_arg( array( - 'action' => 'reset_last_seen', - 'user_id' => $user_id, - ) ); - - $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; - } - return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); - } - - return sprintf( '%s', esc_html__( $formatted_date ) ); - } - } - - return $default; + if ( 'last_seen' !== $column_name ) { + return $default; + } + + $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); + + if ( ! $last_seen_timestamp ) { + return $default; + } + + $formatted_date = sprintf( + __( '%1$s at %2$s' ), + date_i18n( get_option('date_format'), $last_seen_timestamp ), + date_i18n( get_option('time_format'), $last_seen_timestamp ) + ); + + if ( ! is_considered_inactive( $user_id ) ) { + return sprintf( '%s', esc_html__( $formatted_date ) ); + } + + $unblock_link = ''; + if ( current_user_can( 'edit_user', array() ) ) { + $url = add_query_arg( array( + 'action' => 'reset_last_seen', + 'user_id' => $user_id, + ) ); + + $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; + } + return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); }, 10, 3 ); add_action( 'user_row_actions', function ( $actions ) { From 91fcbe10c08b9df7f69007fbf0168b32e17cd789 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Fri, 18 Aug 2023 11:31:53 -0300 Subject: [PATCH 08/27] Apply suggestions from code review Co-authored-by: Mohammad Jangda --- security/user-last-seen.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index ef65d2e923..7b97887489 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -18,7 +18,7 @@ return $user_id; } - $is_api_request = ( ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) || ( defined( 'REST_REQUEST' ) && REST_REQUEST ) ); + $is_api_request = Context::is_rest_api() || Context::is_xmlrpc_api(); if ( $is_api_request && is_considered_inactive( $user_id ) ) { // To block API requests for inactive requests, we need to return a WP_Error object here @@ -85,7 +85,7 @@ add_action( 'user_row_actions', function ( $actions ) { if( isset($_GET['action'] ) && $_GET['action'] === 'reset_last_seen' ){ - $user_id = $_GET['user_id']; + $user_id = absint( $_GET['user_id'] ); delete_user_meta( $user_id, LAST_SEEN_META_KEY ); } @@ -99,12 +99,12 @@ return $views; } - $count = $wpdb->get_var( 'SELECT COUNT(meta_key) FROM ' . $wpdb->usermeta . ' WHERE meta_key = "' . LAST_SEEN_META_KEY . '" AND meta_value < ' . get_inactivity_timestamp() ); + $count = $wpdb->get_var( $wpdb->prepare( 'SELECT COUNT(meta_key) FROM ' . $wpdb->usermeta . ' WHERE meta_key = %s AND meta_value < %s', LAST_SEEN_META_KEY, get_inactivity_timestamp() ) ); $view = __( 'Blocked Users' ); if ( $count ) { $class = isset( $_REQUEST[ 'last_seen_filter' ] ) ? 'current' : ''; - $view = '' . $view . ''; + $view = '' . esc_html( $view ) . ''; } $views['blocked_users'] = $view . ' (' . $count . ')'; From dd56cdd0a00934745eee12180e2f2299f57ce23f Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Fri, 18 Aug 2023 11:37:57 -0300 Subject: [PATCH 09/27] Apply suggestions from code review Co-authored-by: Mohammad Jangda --- security/user-last-seen.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 7b97887489..04aa7e6070 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -72,7 +72,7 @@ } $unblock_link = ''; - if ( current_user_can( 'edit_user', array() ) ) { + if ( current_user_can( 'edit_user', $user_id ) ) { $url = add_query_arg( array( 'action' => 'reset_last_seen', 'user_id' => $user_id, From d8ba2379a77a5d5dc260ea82b598e06ace14c8d1 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Mon, 21 Aug 2023 14:33:59 -0300 Subject: [PATCH 10/27] Add last seen to network users list --- security.php | 2 +- security/user-last-seen.php | 159 +++++++++++++++++++++++++----------- 2 files changed, 112 insertions(+), 49 deletions(-) diff --git a/security.php b/security.php index 1c47a4f107..32604985d4 100644 --- a/security.php +++ b/security.php @@ -16,7 +16,7 @@ require_once __DIR__ . '/security/login-error.php'; require_once __DIR__ . '/security/password.php'; -if ( defined( 'VIP_USER_LAST_SEEN_ENABLED' ) ) { +if ( defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) && constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) !== 'NO_ACTION' ) { require_once __DIR__ . '/security/user-last-seen.php'; } diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 04aa7e6070..ab32cec537 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -2,37 +2,41 @@ namespace Automattic\VIP\Security; const LAST_SEEN_META_KEY = 'wpvip_last_seen'; +const LAST_SEEN_CACHE_GROUP = 'wpvip_last_seen'; const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations -const LAST_SEEN_UPDATE_USER_META_CACHE_KEY_PREFIX = 'wpvip_last_seen_update_user_meta_cache_key'; -const GROUP = 'wpvip'; +const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; + +// Use a global cache group to avoid having to the data for each site +wp_cache_add_global_groups( array( LAST_SEEN_CACHE_GROUP ) ); + +// VIP_SECURITY_INACTIVE_USERS_ACTION= undefined || 'NO_ACTION', 'RECORD_LAST_SEEN', 'REPORT', 'BLOCK' +// VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS = undefined || number add_filter( 'determine_current_user', function ( $user_id ) { if ( ! $user_id ) { return $user_id; } - $cache_key = LAST_SEEN_UPDATE_USER_META_CACHE_KEY_PREFIX . $user_id; - - if ( wp_cache_get( $cache_key, GROUP ) ) { + if ( wp_cache_get( $user_id, LAST_SEEN_CACHE_GROUP ) ) { // Last seen meta was checked recently return $user_id; } - $is_api_request = Context::is_rest_api() || Context::is_xmlrpc_api(); + if ( is_considered_inactive( $user_id ) ) { + // Force current user to 0 to avoid recursive calls to this filter + wp_set_current_user( 0 ); - if ( $is_api_request && is_considered_inactive( $user_id ) ) { - // To block API requests for inactive requests, we need to return a WP_Error object here return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) ); } - if ( wp_cache_add( $cache_key, true, GROUP, LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { + if ( wp_cache_add( $user_id, true, LAST_SEEN_CACHE_GROUP, LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { update_user_meta( $user_id, LAST_SEEN_META_KEY, time() ); } return $user_id; }, 30, 1 ); -add_filter( 'authenticate', function( $user, string $username, string $password ) { +add_filter( 'authenticate', function( $user ) { if ( is_wp_error( $user ) ) { return $user; } @@ -42,12 +46,40 @@ } return $user; -}, 20, 3 ); +}, 20, 1 ); -add_filter( 'manage_users_columns', function ( $cols ) { - $cols['last_seen'] = __( 'Last seen' ); - return $cols; +add_filter( 'wpmu_users_columns', function ( $columns ) { + $columns['last_seen'] = __( 'Last seen' ); + return $columns; +} ); + +add_filter( 'manage_users_columns', function ( $columns ) { + $columns['last_seen'] = __( 'Last seen' ); + return $columns; +} ); + +add_filter( 'manage_users_sortable_columns', function ( $columns ) { + $columns['last_seen'] = 'last_seen'; + + return $columns; +} ); + +add_filter( 'manage_users-network_sortable_columns', function ( $columns ) { + $columns['last_seen'] = 'last_seen'; + + return $columns; +} ); + +add_filter( 'users_list_table_query_args', function ( $vars ) { + if ( isset( $vars['orderby'] ) && $vars['orderby'] === 'last_seen' ) { + $vars = array_merge( $vars, array( + 'meta_key' => LAST_SEEN_META_KEY, + 'orderby' => 'meta_value_num' + ) ); + } + + return $vars; } ); add_filter( 'manage_users_custom_column', function ( $default, $column_name, $user_id ) { @@ -76,6 +108,7 @@ $url = add_query_arg( array( 'action' => 'reset_last_seen', 'user_id' => $user_id, + 'reset_last_seen_nonce' => wp_create_nonce( 'reset_last_seen_action' ) ) ); $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; @@ -83,66 +116,96 @@ return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); }, 10, 3 ); -add_action( 'user_row_actions', function ( $actions ) { - if( isset($_GET['action'] ) && $_GET['action'] === 'reset_last_seen' ){ - $user_id = absint( $_GET['user_id'] ); - delete_user_meta( $user_id, LAST_SEEN_META_KEY ); - } - return $actions; -}, 10, 1 ); +add_action( 'admin_init', function () { + $admin_notices_hook_name = is_network_admin() ? 'network_admin_notices' : 'admin_notices'; -add_filter( 'views_users', function ( $views ) { - global $wpdb; + if ( isset( $_GET['reset_last_seen_success'] ) && $_GET['reset_last_seen_success'] === '1' ) { + add_action( $admin_notices_hook_name, function() { + $class = 'notice notice-success is-dismissible'; + $error = __( 'User unblocked.', 'inactive-account-lockdown' ); - if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { - return $views; + printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); + } ); } - $count = $wpdb->get_var( $wpdb->prepare( 'SELECT COUNT(meta_key) FROM ' . $wpdb->usermeta . ' WHERE meta_key = %s AND meta_value < %s', LAST_SEEN_META_KEY, get_inactivity_timestamp() ) ); + if ( ! isset( $_GET['action'] ) || $_GET['action'] !== 'reset_last_seen' ) { + return; + } + + $user_id = absint( $_GET['user_id'] ); - $view = __( 'Blocked Users' ); - if ( $count ) { - $class = isset( $_REQUEST[ 'last_seen_filter' ] ) ? 'current' : ''; - $view = '' . esc_html( $view ) . ''; + $error = null; + if ( ! wp_verify_nonce( $_GET['reset_last_seen_nonce'], 'reset_last_seen_action' ) ) { + $error = __( 'Unable to verify your request', 'inactive-account-lockdown' ); } - $views['blocked_users'] = $view . ' (' . $count . ')'; - return $views; -} ); + if ( ! get_userdata( $user_id) ) { + $error = __( 'User not found.', 'inactive-account-lockdown' ); + } -add_filter( 'users_list_table_query_args', function ( $args ) { - if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { - return $args; + if ( ! current_user_can( 'edit_user', $user_id ) ) { + $error = __( 'You do not have permission to unblock this user.', 'inactive-account-lockdown' ); } - if ( isset( $_REQUEST[ 'last_seen_filter' ] ) ) { - $args[ 'meta_key' ] = LAST_SEEN_META_KEY; - $args[ 'meta_value' ] = get_inactivity_timestamp(); - $args[ 'meta_type' ] = 'NUMERIC'; - $args[ 'meta_compare' ] = '<'; + if ( ! $error && ! delete_user_meta( $user_id, LAST_SEEN_META_KEY ) ) { + $error = __( 'Unable to unblock user.', 'inactive-account-lockdown' ); } - return $args; + if ( $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 ) ); + } ); + return; + } + + $url = remove_query_arg( array( + 'action', + 'user_id', + 'reset_last_seen_nonce', + ) ); + + $url = add_query_arg( array( + 'reset_last_seen_success' => 1, + ), $url ); + + wp_redirect( $url ); + exit(); +} ); + +add_action( 'admin_init', function () { + if ( ! get_option( LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { + // Right after the first admin_init, set the release date timestamp + // to be used as a fallback for users that never logged in before. + add_option( LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time() ); + } } ); function is_considered_inactive( $user_id ) { - if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) || ! constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) !== 'BLOCK' ) { return false; } $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); - if ( ! $last_seen_timestamp ) { - return false; + if ( $last_seen_timestamp ) { + return $last_seen_timestamp < get_inactivity_timestamp(); + } + + $release_date_timestamp = get_option( LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + if ( $release_date_timestamp ) { + return $release_date_timestamp < get_inactivity_timestamp(); } - return $last_seen_timestamp < get_inactivity_timestamp(); + // Release date is not defined yed, so we can't consider the user inactive. + return false; } function get_inactivity_timestamp() { - if ( ! defined( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { return 0; } - return strtotime( sprintf('-%d days', constant( 'VIP_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + LAST_SEEN_UPDATE_USER_META_CACHE_TTL; + return strtotime( sprintf('-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } From 8cf88fd5494db052a56decedd81e86dc296d52eb Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Mon, 21 Aug 2023 15:40:09 -0300 Subject: [PATCH 11/27] Move the logic to a class --- security.php | 5 +- security/user-last-seen.php | 358 ++++++++++++++++++++---------------- 2 files changed, 208 insertions(+), 155 deletions(-) diff --git a/security.php b/security.php index 32604985d4..89f77371c9 100644 --- a/security.php +++ b/security.php @@ -16,8 +16,11 @@ require_once __DIR__ . '/security/login-error.php'; require_once __DIR__ . '/security/password.php'; -if ( defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) && constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) !== 'NO_ACTION' ) { +if ( defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) ) { require_once __DIR__ . '/security/user-last-seen.php'; + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); } use Automattic\VIP\Utils\Context; diff --git a/security/user-last-seen.php b/security/user-last-seen.php index ab32cec537..4b8151ccc8 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -1,211 +1,261 @@ Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) ); - } + add_filter( 'views_users', array( $this, 'add_blocked_users_filter' ) ); + add_filter( 'views_users-network', array( $this, 'add_blocked_users_filter' ) ); + } - if ( wp_cache_add( $user_id, true, LAST_SEEN_CACHE_GROUP, LAST_SEEN_UPDATE_USER_META_CACHE_TTL ) ) { - update_user_meta( $user_id, LAST_SEEN_META_KEY, time() ); - } - - return $user_id; -}, 30, 1 ); + if ( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'BLOCK' ) { + add_action( 'admin_init', array( $this, 'last_seen_unblock_action' ) ); + } -add_filter( 'authenticate', function( $user ) { - if ( is_wp_error( $user ) ) { - return $user; + add_action( 'admin_init', array( $this, 'register_release_date' ) ); } - if ( $user->ID && is_considered_inactive( $user->ID ) ) { - return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) );; - } + public function determine_current_user( $user_id ) { + if ( ! $user_id ) { + return $user_id; + } - return $user; -}, 20, 1 ); + if ( wp_cache_get( $user_id, self::LAST_SEEN_CACHE_GROUP ) ) { + // Last seen meta was checked recently + return $user_id; + } + if ( $this->is_considered_inactive( $user_id ) ) { + // Force current user to 0 to avoid recursive calls to this filter + wp_set_current_user( 0 ); -add_filter( 'wpmu_users_columns', function ( $columns ) { - $columns['last_seen'] = __( 'Last seen' ); - return $columns; -} ); + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) ); + } -add_filter( 'manage_users_columns', function ( $columns ) { - $columns['last_seen'] = __( 'Last seen' ); - return $columns; -} ); + 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() ); + } -add_filter( 'manage_users_sortable_columns', function ( $columns ) { - $columns['last_seen'] = 'last_seen'; + return $user_id; + } - return $columns; -} ); + public function authenticate( $user ) { + if ( is_wp_error( $user ) ) { + return $user; + } -add_filter( 'manage_users-network_sortable_columns', function ( $columns ) { - $columns['last_seen'] = 'last_seen'; + if ( $user->ID && $this->is_considered_inactive( $user->ID ) ) { + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) );; + } - return $columns; -} ); + return $user; + } -add_filter( 'users_list_table_query_args', function ( $vars ) { - if ( isset( $vars['orderby'] ) && $vars['orderby'] === 'last_seen' ) { - $vars = array_merge( $vars, array( - 'meta_key' => LAST_SEEN_META_KEY, - 'orderby' => 'meta_value_num' - ) ); + function add_last_seen_column_head( $columns ) { + $columns[ 'last_seen' ] = __( 'Last seen' ); + return $columns; } - return $vars; -} ); + function add_last_seen_sortable_column( $columns ) { + $columns['last_seen'] = 'last_seen'; -add_filter( 'manage_users_custom_column', function ( $default, $column_name, $user_id ) { - if ( 'last_seen' !== $column_name ) { - return $default; + return $columns; } - $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); + function last_seen_query_args( $vars ) { + if ( isset( $vars['orderby'] ) && $vars['orderby'] === 'last_seen' ) { + $vars[ 'meta_key' ] = self::LAST_SEEN_META_KEY; + $vars[ 'orderby' ] = 'meta_value_num'; + } + + if ( isset( $_REQUEST[ 'last_seen_filter' ] ) && $_REQUEST[ 'last_seen_filter' ] === 'blocked' ) { + $vars[ 'meta_key' ] = self::LAST_SEEN_META_KEY; + $vars[ 'meta_value' ] = $this->get_inactivity_timestamp(); + $vars[ 'meta_type' ] = 'NUMERIC'; + $vars[ 'meta_compare' ] = '<'; + } - if ( ! $last_seen_timestamp ) { - return $default; + return $vars; } - $formatted_date = sprintf( - __( '%1$s at %2$s' ), - date_i18n( get_option('date_format'), $last_seen_timestamp ), - date_i18n( get_option('time_format'), $last_seen_timestamp ) - ); + function add_last_seen_column_date( $default, $column_name, $user_id ) { + if ( 'last_seen' !== $column_name ) { + return $default; + } + + $last_seen_timestamp = get_user_meta( $user_id, self::LAST_SEEN_META_KEY, true ); + + if ( ! $last_seen_timestamp ) { + return $default; + } + + $formatted_date = sprintf( + __( '%1$s at %2$s' ), + date_i18n( get_option('date_format'), $last_seen_timestamp ), + date_i18n( get_option('time_format'), $last_seen_timestamp ) + ); + + if ( ! $this->is_considered_inactive( $user_id ) ) { + return sprintf( '%s', esc_html__( $formatted_date ) ); + } + + $unblock_link = ''; + if ( current_user_can( 'edit_user', $user_id ) ) { + $url = add_query_arg( array( + 'action' => 'reset_last_seen', + 'user_id' => $user_id, + 'reset_last_seen_nonce' => wp_create_nonce( 'reset_last_seen_action' ) + ) ); + + $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; + } + return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); + } - if ( ! is_considered_inactive( $user_id ) ) { - return sprintf( '%s', esc_html__( $formatted_date ) ); + function add_blocked_users_filter( $views ) { + $blog_id = is_network_admin() ? null : get_current_blog_id(); + + $users_query = new \WP_User_Query( + array( + 'blog_id' => $blog_id, + 'fields' => 'ID', + 'meta_key' => self::LAST_SEEN_META_KEY, + 'meta_value' => $this->get_inactivity_timestamp(), + 'meta_type' => 'NUMERIC', + 'meta_compare' => '<', + 'count_total' => true, + ), + ); + $count = (int) $users_query->get_total(); + + $view = __( 'Blocked Users' ); + if ( $count ) { + $class = isset( $_REQUEST[ 'last_seen_filter' ] ) ? 'current' : ''; + + $url = add_query_arg( array( + 'last_seen_filter' => 'blocked', + ) ); + + $view = '' . esc_html( $view ) . ''; + } + $views['blocked_users'] = $view . ' (' . $count . ')'; + + return $views; } - $unblock_link = ''; - if ( current_user_can( 'edit_user', $user_id ) ) { - $url = add_query_arg( array( - 'action' => 'reset_last_seen', - 'user_id' => $user_id, - 'reset_last_seen_nonce' => wp_create_nonce( 'reset_last_seen_action' ) - ) ); + function last_seen_unblock_action() { + $admin_notices_hook_name = is_network_admin() ? 'network_admin_notices' : 'admin_notices'; - $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; - } - return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); -}, 10, 3 ); + if ( isset( $_GET['reset_last_seen_success'] ) && $_GET['reset_last_seen_success'] === '1' ) { + add_action( $admin_notices_hook_name, function() { + $class = 'notice notice-success is-dismissible'; + $error = __( 'User unblocked.', 'inactive-account-lockdown' ); + printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); + } ); + } -add_action( 'admin_init', function () { - $admin_notices_hook_name = is_network_admin() ? 'network_admin_notices' : 'admin_notices'; + if ( ! isset( $_GET['action'] ) || $_GET['action'] !== 'reset_last_seen' ) { + return; + } - if ( isset( $_GET['reset_last_seen_success'] ) && $_GET['reset_last_seen_success'] === '1' ) { - add_action( $admin_notices_hook_name, function() { - $class = 'notice notice-success is-dismissible'; - $error = __( 'User unblocked.', 'inactive-account-lockdown' ); + $user_id = absint( $_GET['user_id'] ); - printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); - } ); - } + $error = null; + if ( ! wp_verify_nonce( $_GET['reset_last_seen_nonce'], 'reset_last_seen_action' ) ) { + $error = __( 'Unable to verify your request', 'inactive-account-lockdown' ); + } - if ( ! isset( $_GET['action'] ) || $_GET['action'] !== 'reset_last_seen' ) { - return; - } + if ( ! get_userdata( $user_id) ) { + $error = __( 'User not found.', 'inactive-account-lockdown' ); + } - $user_id = absint( $_GET['user_id'] ); + if ( ! current_user_can( 'edit_user', $user_id ) ) { + $error = __( 'You do not have permission to unblock this user.', 'inactive-account-lockdown' ); + } - $error = null; - if ( ! wp_verify_nonce( $_GET['reset_last_seen_nonce'], 'reset_last_seen_action' ) ) { - $error = __( 'Unable to verify your request', 'inactive-account-lockdown' ); - } + if ( ! $error && ! delete_user_meta( $user_id, self::LAST_SEEN_META_KEY ) ) { + $error = __( 'Unable to unblock user.', 'inactive-account-lockdown' ); + } - if ( ! get_userdata( $user_id) ) { - $error = __( 'User not found.', 'inactive-account-lockdown' ); - } + if ( $error ) { + add_action( $admin_notices_hook_name, function() use ( $error ) { + $class = 'notice notice-error is-dismissible'; - if ( ! current_user_can( 'edit_user', $user_id ) ) { - $error = __( 'You do not have permission to unblock this user.', 'inactive-account-lockdown' ); - } + printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); + } ); + return; + } - if ( ! $error && ! delete_user_meta( $user_id, LAST_SEEN_META_KEY ) ) { - $error = __( 'Unable to unblock user.', 'inactive-account-lockdown' ); - } + $url = remove_query_arg( array( + 'action', + 'user_id', + 'reset_last_seen_nonce', + ) ); - if ( $error ) { - add_action( $admin_notices_hook_name, function() use ( $error ) { - $class = 'notice notice-error is-dismissible'; + $url = add_query_arg( array( + 'reset_last_seen_success' => 1, + ), $url ); - printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); - } ); - return; + wp_redirect( $url ); + exit(); } - $url = remove_query_arg( array( - 'action', - 'user_id', - 'reset_last_seen_nonce', - ) ); - - $url = add_query_arg( array( - 'reset_last_seen_success' => 1, - ), $url ); - - wp_redirect( $url ); - exit(); -} ); - -add_action( 'admin_init', function () { - if ( ! get_option( LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { - // Right after the first admin_init, set the release date timestamp - // to be used as a fallback for users that never logged in before. - add_option( LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time() ); + public function register_release_date() { + if ( ! get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { + // Right after the first admin_init, set the release date timestamp + // to be used as a fallback for users that never logged in before. + add_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time() ); + } } -} ); -function is_considered_inactive( $user_id ) { - if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) || ! constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) !== 'BLOCK' ) { - return false; - } + public function is_considered_inactive( $user_id ) { + if ( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) !== 'BLOCK' ) { + return false; + } - $last_seen_timestamp = get_user_meta( $user_id, LAST_SEEN_META_KEY, true ); - if ( $last_seen_timestamp ) { - return $last_seen_timestamp < get_inactivity_timestamp(); - } + $last_seen_timestamp = get_user_meta( $user_id, self::LAST_SEEN_META_KEY, true ); + if ( $last_seen_timestamp ) { + return $last_seen_timestamp < $this->get_inactivity_timestamp(); + } - $release_date_timestamp = get_option( LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); - if ( $release_date_timestamp ) { - return $release_date_timestamp < get_inactivity_timestamp(); + $release_date_timestamp = get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + if ( $release_date_timestamp ) { + return $release_date_timestamp < $this->get_inactivity_timestamp(); + } + + // Release date is not defined yed, so we can't consider the user inactive. + return false; } - // Release date is not defined yed, so we can't consider the user inactive. - return false; -} + public function get_inactivity_timestamp() { + if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { + return 0; + } -function get_inactivity_timestamp() { - if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { - return 0; + return strtotime( sprintf('-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } - - return strtotime( sprintf('-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } From 267e26f4367aadbd0c33f3d33b65a4ecec6c394e Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Mon, 21 Aug 2023 17:37:30 -0300 Subject: [PATCH 12/27] Add unit tests --- security/user-last-seen.php | 51 +++--- tests/security/test-user-last-seen.php | 219 +++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 24 deletions(-) create mode 100644 tests/security/test-user-last-seen.php diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 4b8151ccc8..0a240d507f 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -8,15 +8,17 @@ class User_Last_Seen { const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; public function init() { - if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) || constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'NO_ACTION' ) { + if ( ! defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) || constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'NO_ACTION' ) { return; } // Use a global cache group to avoid having to the data for each site wp_cache_add_global_groups( array( self::LAST_SEEN_CACHE_GROUP ) ); + add_action( 'admin_init', array( $this, 'register_release_date' ) ); + add_filter( 'determine_current_user', array( $this, 'determine_current_user' ), 30, 1 ); - add_filter( 'authenticate', array( $this, 'determine_current_user' ), 20, 1 ); + add_filter( 'authenticate', array( $this, 'authenticate' ), 20, 1 ); if ( in_array( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ), array( 'REPORT', 'BLOCK' ) ) ) { add_filter( 'wpmu_users_columns', array( $this, 'add_last_seen_column_head' ) ); @@ -25,17 +27,16 @@ public function init() { add_filter( 'manage_users_sortable_columns', array( $this, 'add_last_seen_sortable_column' ) ); add_filter( 'manage_users-network_sortable_columns', array( $this, 'add_last_seen_sortable_column' ) ); - add_filter( 'users_list_table_query_args', array( $this, 'last_seen_query_args') ); + add_filter( 'users_list_table_query_args', array( $this, 'last_seen_order_by_query_args') ); + } + if ( $this->is_block_action_enabled() ) { add_filter( 'views_users', array( $this, 'add_blocked_users_filter' ) ); add_filter( 'views_users-network', array( $this, 'add_blocked_users_filter' ) ); - } + add_filter( 'users_list_table_query_args', array( $this, 'last_seen_blocked_users_filter_query_args') ); - if ( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'BLOCK' ) { add_action( 'admin_init', array( $this, 'last_seen_unblock_action' ) ); } - - add_action( 'admin_init', array( $this, 'register_release_date' ) ); } public function determine_current_user( $user_id ) { @@ -48,7 +49,7 @@ public function determine_current_user( $user_id ) { return $user_id; } - if ( $this->is_considered_inactive( $user_id ) ) { + if ( $this->is_block_action_enabled() && $this->is_considered_inactive( $user_id ) ) { // Force current user to 0 to avoid recursive calls to this filter wp_set_current_user( 0 ); @@ -67,30 +68,34 @@ public function authenticate( $user ) { return $user; } - if ( $user->ID && $this->is_considered_inactive( $user->ID ) ) { + if ( $user->ID && $this->is_block_action_enabled() && $this->is_considered_inactive( $user->ID ) ) { return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) );; } return $user; } - function add_last_seen_column_head( $columns ) { + public function add_last_seen_column_head( $columns ) { $columns[ 'last_seen' ] = __( 'Last seen' ); return $columns; } - function add_last_seen_sortable_column( $columns ) { + public function add_last_seen_sortable_column( $columns ) { $columns['last_seen'] = 'last_seen'; return $columns; } - function last_seen_query_args( $vars ) { + public function last_seen_order_by_query_args( $vars ) { if ( isset( $vars['orderby'] ) && $vars['orderby'] === 'last_seen' ) { $vars[ 'meta_key' ] = self::LAST_SEEN_META_KEY; $vars[ 'orderby' ] = 'meta_value_num'; } + return $vars; + } + + public function last_seen_blocked_users_filter_query_args($vars ) { if ( isset( $_REQUEST[ 'last_seen_filter' ] ) && $_REQUEST[ 'last_seen_filter' ] === 'blocked' ) { $vars[ 'meta_key' ] = self::LAST_SEEN_META_KEY; $vars[ 'meta_value' ] = $this->get_inactivity_timestamp(); @@ -101,7 +106,7 @@ function last_seen_query_args( $vars ) { return $vars; } - function add_last_seen_column_date( $default, $column_name, $user_id ) { + public function add_last_seen_column_date( $default, $column_name, $user_id ) { if ( 'last_seen' !== $column_name ) { return $default; } @@ -118,7 +123,7 @@ function add_last_seen_column_date( $default, $column_name, $user_id ) { date_i18n( get_option('time_format'), $last_seen_timestamp ) ); - if ( ! $this->is_considered_inactive( $user_id ) ) { + if ( ! $this->is_block_action_enabled() || ! $this->is_considered_inactive( $user_id ) ) { return sprintf( '%s', esc_html__( $formatted_date ) ); } @@ -135,7 +140,7 @@ function add_last_seen_column_date( $default, $column_name, $user_id ) { return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); } - function add_blocked_users_filter( $views ) { + public function add_blocked_users_filter( $views ) { $blog_id = is_network_admin() ? null : get_current_blog_id(); $users_query = new \WP_User_Query( @@ -166,7 +171,7 @@ function add_blocked_users_filter( $views ) { return $views; } - function last_seen_unblock_action() { + 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'] ) && $_GET['reset_last_seen_success'] === '1' ) { @@ -233,10 +238,6 @@ public function register_release_date() { } public function is_considered_inactive( $user_id ) { - if ( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) !== 'BLOCK' ) { - return false; - } - $last_seen_timestamp = get_user_meta( $user_id, self::LAST_SEEN_META_KEY, true ); if ( $last_seen_timestamp ) { return $last_seen_timestamp < $this->get_inactivity_timestamp(); @@ -252,10 +253,12 @@ public function is_considered_inactive( $user_id ) { } public function get_inactivity_timestamp() { - if ( ! defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) { - return 0; - } - return strtotime( sprintf('-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } + + private function is_block_action_enabled() { + return defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) && + defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) && + constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'BLOCK'; + } } diff --git a/tests/security/test-user-last-seen.php b/tests/security/test-user-last-seen.php new file mode 100644 index 0000000000..8185b99d5f --- /dev/null +++ b/tests/security/test-user-last-seen.php @@ -0,0 +1,219 @@ +init(); + + $this->assertFalse( has_filter( 'determine_current_user' ) ); + $this->assertFalse( has_filter( 'authenticate' ) ); + } + + public function test__should_not_load_actions_and_filters_when_env_vars_is_set_to_no_action() { + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'NO_ACTION' ); + + remove_all_filters( 'determine_current_user' ); + remove_all_filters( 'authenticate' ); + + $last_seen = new User_Last_Seen(); + $last_seen->init(); + + $this->assertFalse( has_filter( 'determine_current_user' ) ); + $this->assertFalse( has_filter( 'authenticate' ) ); + } + + public function test__is_considered_inactive__should_consider_user_meta() + { + Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); + + $user_inactive_id = $this->factory()->user->create([ + 'meta_input' => [ + 'wpvip_last_seen' => strtotime('-31 days'), + ], + ]); + + $user_active_id = $this->factory()->user->create([ + 'meta_input' => [ + 'wpvip_last_seen' => strtotime('-29 days'), + ], + ]); + + $last_seen = new User_Last_Seen(); + $last_seen->init(); + + $this->assertTrue( $last_seen->is_considered_inactive( $user_inactive_id ) ); + $this->assertFalse( $last_seen->is_considered_inactive( $user_active_id ) ); + } + + 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); + + delete_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + + $user_without_meta = $this->factory()->user->create(); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); + } + + public function test__is_considered_inactive__should_use_release_date_option_when_user_meta_is_not_defined() + { + Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + + add_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime('-16 days') ); + + $user_without_meta = $this->factory()->user->create(); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $this->assertTrue( $last_seen->is_considered_inactive( $user_without_meta ) ); + + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime('-10 days') ); + + $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); + } + + public function test__determine_current_user_should_record_once_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( 'determine_current_user' ); + + $previous_last_seen = sprintf('%d', strtotime('-10 days') ); + + $user_id = $this->factory()->user->create([ + 'meta_input' => [ + 'wpvip_last_seen' => $previous_last_seen, + ], + ]); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + apply_filters( 'determine_current_user', $user_id, $user_id ); + + $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + + $new_user_id = apply_filters( 'determine_current_user', $user_id, $user_id ); + + $cached_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + + $this->assertTrue( $current_last_seen > $previous_last_seen ); + $this->assertSame( $current_last_seen, $cached_last_seen ); + $this->assertSame( $new_user_id, $user_id ); + } + + public function test__determine_current_user_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); + + remove_all_filters( 'determine_current_user' ); + + $user_id = $this->factory()->user->create([ + 'meta_input' => [ + 'wpvip_last_seen' => strtotime('-100 days'), + ], + ]); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $user = apply_filters( 'determine_current_user', $user_id, $user_id ); + + $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); + } + + 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); + + remove_all_filters( 'authenticate' ); + + $user_id = $this->factory()->user->create([ + 'meta_input' => [ + 'wpvip_last_seen' => strtotime('-5 days'), + ], + ]); + + $user = get_user_by( 'id', $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $new_user = apply_filters( 'authenticate', $user, $user ); + + $this->assertSame( $user->ID, $new_user->ID ); + } + + 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); + + remove_all_filters( 'authenticate' ); + + $user_id = $this->factory()->user->create([ + 'meta_input' => [ + 'wpvip_last_seen' => strtotime('-100 days'), + ], + ]); + $user = get_user_by( 'id', $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $user = apply_filters( 'authenticate', $user, $user ); + + $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); + } + + public function test__register_release_date_should_register_release_date_only_once() + { + Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'RECORD_LAST_SEEN' ); + + remove_all_actions( 'admin_init' ); + delete_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->register_release_date(); + + $release_date = get_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + + $last_seen->register_release_date(); + + $new_release_date = get_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + + $this->assertIsNumeric( $release_date ); + $this->assertSame( $release_date, $new_release_date ); + } +} From 26bdd274aa0e7e17315a094a31eb6db8ee45ed63 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Mon, 21 Aug 2023 17:45:55 -0300 Subject: [PATCH 13/27] Linting fixes --- security/user-last-seen.php | 91 ++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 41 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 0a240d507f..aa99950735 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -2,9 +2,9 @@ namespace Automattic\VIP\Security; class User_Last_Seen { - const LAST_SEEN_META_KEY = 'wpvip_last_seen'; - const LAST_SEEN_CACHE_GROUP = 'wpvip_last_seen'; - const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations + const LAST_SEEN_META_KEY = 'wpvip_last_seen'; + const LAST_SEEN_CACHE_GROUP = 'wpvip_last_seen'; + const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; public function init() { @@ -27,13 +27,13 @@ public function init() { add_filter( 'manage_users_sortable_columns', array( $this, 'add_last_seen_sortable_column' ) ); add_filter( 'manage_users-network_sortable_columns', array( $this, 'add_last_seen_sortable_column' ) ); - add_filter( 'users_list_table_query_args', array( $this, 'last_seen_order_by_query_args') ); + add_filter( 'users_list_table_query_args', array( $this, 'last_seen_order_by_query_args' ) ); } if ( $this->is_block_action_enabled() ) { add_filter( 'views_users', array( $this, 'add_blocked_users_filter' ) ); add_filter( 'views_users-network', array( $this, 'add_blocked_users_filter' ) ); - add_filter( 'users_list_table_query_args', array( $this, 'last_seen_blocked_users_filter_query_args') ); + add_filter( 'users_list_table_query_args', array( $this, 'last_seen_blocked_users_filter_query_args' ) ); add_action( 'admin_init', array( $this, 'last_seen_unblock_action' ) ); } @@ -56,6 +56,7 @@ public function determine_current_user( $user_id ) { return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) ); } + // 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() ); } @@ -69,14 +70,17 @@ public function authenticate( $user ) { } if ( $user->ID && $this->is_block_action_enabled() && $this->is_considered_inactive( $user->ID ) ) { - return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) );; + return new \WP_Error( + 'inactive_account', + __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) + ); } return $user; } public function add_last_seen_column_head( $columns ) { - $columns[ 'last_seen' ] = __( 'Last seen' ); + $columns['last_seen'] = __( 'Last seen' ); return $columns; } @@ -87,20 +91,21 @@ public function add_last_seen_sortable_column( $columns ) { } public function last_seen_order_by_query_args( $vars ) { - if ( isset( $vars['orderby'] ) && $vars['orderby'] === 'last_seen' ) { - $vars[ 'meta_key' ] = self::LAST_SEEN_META_KEY; - $vars[ 'orderby' ] = 'meta_value_num'; + if ( isset( $vars['orderby'] ) && 'last_seen' === $vars['orderby'] ) { + $vars['meta_key'] = self::LAST_SEEN_META_KEY; + $vars['orderby'] = 'meta_value_num'; } return $vars; } - public function last_seen_blocked_users_filter_query_args($vars ) { - if ( isset( $_REQUEST[ 'last_seen_filter' ] ) && $_REQUEST[ 'last_seen_filter' ] === 'blocked' ) { - $vars[ 'meta_key' ] = self::LAST_SEEN_META_KEY; - $vars[ 'meta_value' ] = $this->get_inactivity_timestamp(); - $vars[ 'meta_type' ] = 'NUMERIC'; - $vars[ 'meta_compare' ] = '<'; + public function last_seen_blocked_users_filter_query_args( $vars ) { + if ( isset( $_GET['last_seen_filter'] ) && 'blocked' === $_GET['last_seen_filter'] && isset( $_GET['last_seen_filter_nonce'] ) && wp_verify_nonce( sanitize_text_field( $_GET['last_seen_filter_nonce'] ), 'last_seen_filter' ) ) { + $vars['meta_key'] = self::LAST_SEEN_META_KEY; + // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value + $vars['meta_value'] = $this->get_inactivity_timestamp(); + $vars['meta_type'] = 'NUMERIC'; + $vars['meta_compare'] = '<'; } return $vars; @@ -117,27 +122,28 @@ public function add_last_seen_column_date( $default, $column_name, $user_id ) { return $default; } - $formatted_date = sprintf( + $date = sprintf( + /* translators: 1: Comment date, 2: Comment time. */ __( '%1$s at %2$s' ), - date_i18n( get_option('date_format'), $last_seen_timestamp ), - date_i18n( get_option('time_format'), $last_seen_timestamp ) + date_i18n( get_option( 'date_format' ), $last_seen_timestamp ), + date_i18n( get_option( 'time_format' ), $last_seen_timestamp ) ); if ( ! $this->is_block_action_enabled() || ! $this->is_considered_inactive( $user_id ) ) { - return sprintf( '%s', esc_html__( $formatted_date ) ); + return sprintf( '%s', esc_html( $date ) ); } $unblock_link = ''; if ( current_user_can( 'edit_user', $user_id ) ) { $url = add_query_arg( array( - 'action' => 'reset_last_seen', - 'user_id' => $user_id, - 'reset_last_seen_nonce' => wp_create_nonce( 'reset_last_seen_action' ) + 'action' => 'reset_last_seen', + 'user_id' => $user_id, + 'reset_last_seen_nonce' => wp_create_nonce( 'reset_last_seen_action' ), ) ); - $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . "
"; + $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . '
'; } - return sprintf( '%s' . $unblock_link, esc_html__( $formatted_date ) ); + return sprintf( '%s' . $unblock_link, esc_html( $date ) ); } public function add_blocked_users_filter( $views ) { @@ -145,25 +151,28 @@ public function add_blocked_users_filter( $views ) { $users_query = new \WP_User_Query( array( - 'blog_id' => $blog_id, - 'fields' => 'ID', - 'meta_key' => self::LAST_SEEN_META_KEY, - 'meta_value' => $this->get_inactivity_timestamp(), - 'meta_type' => 'NUMERIC', + 'blog_id' => $blog_id, + 'fields' => 'ID', + 'meta_key' => self::LAST_SEEN_META_KEY, + // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value + 'meta_value' => $this->get_inactivity_timestamp(), + 'meta_type' => 'NUMERIC', 'meta_compare' => '<', - 'count_total' => true, + 'count_total' => true, ), ); - $count = (int) $users_query->get_total(); + $count = (int) $users_query->get_total(); $view = __( 'Blocked Users' ); if ( $count ) { - $class = isset( $_REQUEST[ 'last_seen_filter' ] ) ? 'current' : ''; - $url = add_query_arg( array( - 'last_seen_filter' => 'blocked', + 'last_seen_filter' => 'blocked', + 'last_seen_filter_nonce' => wp_create_nonce( 'last_seen_filter' ), ) ); + // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $class = isset( $_GET['last_seen_filter'] ) ? 'current' : ''; + $view = '' . esc_html( $view ) . ''; } $views['blocked_users'] = $view . ' (' . $count . ')'; @@ -174,7 +183,7 @@ public function add_blocked_users_filter( $views ) { 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'] ) && $_GET['reset_last_seen_success'] === '1' ) { + if ( isset( $_GET['reset_last_seen_success'] ) && '1' === $_GET['reset_last_seen_success'] ) { add_action( $admin_notices_hook_name, function() { $class = 'notice notice-success is-dismissible'; $error = __( 'User unblocked.', 'inactive-account-lockdown' ); @@ -183,18 +192,18 @@ public function last_seen_unblock_action() { } ); } - if ( ! isset( $_GET['action'] ) || $_GET['action'] !== 'reset_last_seen' ) { + if ( ! isset( $_GET['user_id'] ) || ! isset( $_GET['action'] ) || 'reset_last_seen' !== $_GET['action'] ) { return; } $user_id = absint( $_GET['user_id'] ); $error = null; - if ( ! wp_verify_nonce( $_GET['reset_last_seen_nonce'], 'reset_last_seen_action' ) ) { + if ( ! isset( $_GET['reset_last_seen_nonce'] ) || ! wp_verify_nonce( sanitize_text_field( $_GET['reset_last_seen_nonce'] ), 'reset_last_seen_action' ) ) { $error = __( 'Unable to verify your request', 'inactive-account-lockdown' ); } - if ( ! get_userdata( $user_id) ) { + if ( ! get_userdata( $user_id ) ) { $error = __( 'User not found.', 'inactive-account-lockdown' ); } @@ -225,7 +234,7 @@ public function last_seen_unblock_action() { 'reset_last_seen_success' => 1, ), $url ); - wp_redirect( $url ); + wp_safe_redirect( $url ); exit(); } @@ -253,7 +262,7 @@ public function is_considered_inactive( $user_id ) { } public function get_inactivity_timestamp() { - return strtotime( sprintf('-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; + return strtotime( sprintf( '-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } private function is_block_action_enabled() { From d1bdbb43173ff89ef3f637ba97a0d771b1d1d5a9 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Tue, 22 Aug 2023 08:52:18 -0300 Subject: [PATCH 14/27] Fix tests to run on WP 5.8 --- tests/security/test-user-last-seen.php | 31 +++++++++----------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/tests/security/test-user-last-seen.php b/tests/security/test-user-last-seen.php index 8185b99d5f..f97eb1748b 100644 --- a/tests/security/test-user-last-seen.php +++ b/tests/security/test-user-last-seen.php @@ -55,12 +55,14 @@ public function test__is_considered_inactive__should_consider_user_meta() 'wpvip_last_seen' => strtotime('-31 days'), ], ]); + add_user_meta( $user_inactive_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); $user_active_id = $this->factory()->user->create([ 'meta_input' => [ 'wpvip_last_seen' => strtotime('-29 days'), ], ]); + add_user_meta( $user_active_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-29 days') ); $last_seen = new User_Last_Seen(); $last_seen->init(); @@ -110,11 +112,8 @@ public function test__determine_current_user_should_record_once_last_seen_meta() $previous_last_seen = sprintf('%d', strtotime('-10 days') ); - $user_id = $this->factory()->user->create([ - 'meta_input' => [ - 'wpvip_last_seen' => $previous_last_seen, - ], - ]); + $user_id = $this->factory()->user->create(); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $previous_last_seen ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); @@ -139,11 +138,8 @@ public function test__determine_current_user_should_return_an_error_when_user_is remove_all_filters( 'determine_current_user' ); - $user_id = $this->factory()->user->create([ - 'meta_input' => [ - 'wpvip_last_seen' => strtotime('-100 days'), - ], - ]); + $user_id = $this->factory()->user->create(); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); @@ -160,11 +156,8 @@ public function test__authenticate_should_not_return_error_when_user_is_active() remove_all_filters( 'authenticate' ); - $user_id = $this->factory()->user->create([ - 'meta_input' => [ - 'wpvip_last_seen' => strtotime('-5 days'), - ], - ]); + $user_id = $this->factory()->user->create(); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-5 days') ); $user = get_user_by( 'id', $user_id ); @@ -183,11 +176,9 @@ public function test__authenticate_should_return_an_error_when_user_is_inactive( remove_all_filters( 'authenticate' ); - $user_id = $this->factory()->user->create([ - 'meta_input' => [ - 'wpvip_last_seen' => strtotime('-100 days'), - ], - ]); + $user_id = $this->factory()->user->create(); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + $user = get_user_by( 'id', $user_id ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); From 5287197d16a433b87d8870fd59cb3d1f69e98321 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Tue, 22 Aug 2023 15:35:34 -0300 Subject: [PATCH 15/27] Update test-user-last-seen.php --- tests/security/test-user-last-seen.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/security/test-user-last-seen.php b/tests/security/test-user-last-seen.php index f97eb1748b..5b653f7c50 100644 --- a/tests/security/test-user-last-seen.php +++ b/tests/security/test-user-last-seen.php @@ -50,18 +50,10 @@ public function test__is_considered_inactive__should_consider_user_meta() { Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); - $user_inactive_id = $this->factory()->user->create([ - 'meta_input' => [ - 'wpvip_last_seen' => strtotime('-31 days'), - ], - ]); + $user_inactive_id = $this->factory()->user->create(); add_user_meta( $user_inactive_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); - $user_active_id = $this->factory()->user->create([ - 'meta_input' => [ - 'wpvip_last_seen' => strtotime('-29 days'), - ], - ]); + $user_active_id = $this->factory()->user->create(); add_user_meta( $user_active_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-29 days') ); $last_seen = new User_Last_Seen(); From bb49b0cb066c6e26aff2f395733629ecd457c1fb Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 24 Aug 2023 12:25:04 -0300 Subject: [PATCH 16/27] Code review suggestions --- security/user-last-seen.php | 117 +++++++++++++++++++------ tests/security/test-user-last-seen.php | 106 ++++++++++++++++++++-- 2 files changed, 190 insertions(+), 33 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index aa99950735..a4693a783a 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -18,7 +18,6 @@ public function init() { add_action( 'admin_init', array( $this, 'register_release_date' ) ); add_filter( 'determine_current_user', array( $this, 'determine_current_user' ), 30, 1 ); - add_filter( 'authenticate', array( $this, 'authenticate' ), 20, 1 ); if ( in_array( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ), array( 'REPORT', 'BLOCK' ) ) ) { add_filter( 'wpmu_users_columns', array( $this, 'add_last_seen_column_head' ) ); @@ -31,6 +30,8 @@ public function init() { } if ( $this->is_block_action_enabled() ) { + add_filter( 'authenticate', array( $this, 'authenticate' ), 20, 1 ); + add_filter( 'views_users', array( $this, 'add_blocked_users_filter' ) ); add_filter( 'views_users-network', array( $this, 'add_blocked_users_filter' ) ); add_filter( 'users_list_table_query_args', array( $this, 'last_seen_blocked_users_filter_query_args' ) ); @@ -44,6 +45,10 @@ public function determine_current_user( $user_id ) { return $user_id; } + if ( ! $this->should_check_user_last_seen( $user_id ) ) { + return $user_id; + } + if ( wp_cache_get( $user_id, self::LAST_SEEN_CACHE_GROUP ) ) { // Last seen meta was checked recently return $user_id; @@ -53,7 +58,7 @@ public function determine_current_user( $user_id ) { // Force current user to 0 to avoid recursive calls to this filter wp_set_current_user( 0 ); - return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) ); + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); } // phpcs:ignore WordPressVIPMinimum.Performance.LowExpiryCacheTime.CacheTimeUndetermined @@ -69,10 +74,14 @@ public function authenticate( $user ) { return $user; } + if ( ! $this->should_check_user_last_seen( $user->ID ) ) { + return $user; + } + if ( $user->ID && $this->is_block_action_enabled() && $this->is_considered_inactive( $user->ID ) ) { return new \WP_Error( 'inactive_account', - __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'inactive-account-lockdown' ) + __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); } @@ -80,7 +89,7 @@ public function authenticate( $user ) { } public function add_last_seen_column_head( $columns ) { - $columns['last_seen'] = __( 'Last seen' ); + $columns['last_seen'] = __( 'Last seen', 'wpvip' ); return $columns; } @@ -141,7 +150,7 @@ public function add_last_seen_column_date( $default, $column_name, $user_id ) { 'reset_last_seen_nonce' => wp_create_nonce( 'reset_last_seen_action' ), ) ); - $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock' ) . '
'; + $unblock_link = "
User blocked due to inactivity. " . __( 'Unblock', 'wpvip' ) . '
'; } return sprintf( '%s' . $unblock_link, esc_html( $date ) ); } @@ -158,24 +167,28 @@ public function add_blocked_users_filter( $views ) { 'meta_value' => $this->get_inactivity_timestamp(), 'meta_type' => 'NUMERIC', 'meta_compare' => '<', - 'count_total' => true, + 'count_total' => false, + 'number' => 1, // To minimize the query time, we only need to know if there are any blocked users to show the link ), ); - $count = (int) $users_query->get_total(); - $view = __( 'Blocked Users' ); - if ( $count ) { - $url = add_query_arg( array( - 'last_seen_filter' => 'blocked', - 'last_seen_filter_nonce' => wp_create_nonce( 'last_seen_filter' ), - ) ); + $views['blocked_users'] = __( 'Blocked Users', 'wpvip' ); - // phpcs:ignore WordPress.Security.NonceVerification.Recommended - $class = isset( $_GET['last_seen_filter'] ) ? 'current' : ''; - - $view = '' . esc_html( $view ) . ''; + if ( ! $users_query->get_results() ) { + return $views; } - $views['blocked_users'] = $view . ' (' . $count . ')'; + + $url = add_query_arg( array( + 'last_seen_filter' => 'blocked', + 'last_seen_filter_nonce' => wp_create_nonce( 'last_seen_filter' ), + ) ); + + // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $class = isset( $_GET['last_seen_filter'] ) ? 'current' : ''; + + $view = '' . esc_html( $views['blocked_users'] ) . ''; + + $views['blocked_users'] = $view; return $views; } @@ -186,7 +199,7 @@ public function last_seen_unblock_action() { if ( isset( $_GET['reset_last_seen_success'] ) && '1' === $_GET['reset_last_seen_success'] ) { add_action( $admin_notices_hook_name, function() { $class = 'notice notice-success is-dismissible'; - $error = __( 'User unblocked.', 'inactive-account-lockdown' ); + $error = __( 'User unblocked.', 'wpvip' ); printf( '

%2$s

', esc_attr( $class ), esc_html( $error ) ); } ); @@ -200,19 +213,19 @@ public function last_seen_unblock_action() { $error = null; if ( ! isset( $_GET['reset_last_seen_nonce'] ) || ! wp_verify_nonce( sanitize_text_field( $_GET['reset_last_seen_nonce'] ), 'reset_last_seen_action' ) ) { - $error = __( 'Unable to verify your request', 'inactive-account-lockdown' ); + $error = __( 'Unable to verify your request', 'wpvip' ); } if ( ! get_userdata( $user_id ) ) { - $error = __( 'User not found.', 'inactive-account-lockdown' ); + $error = __( 'User not found.', 'wpvip' ); } if ( ! current_user_can( 'edit_user', $user_id ) ) { - $error = __( 'You do not have permission to unblock this user.', 'inactive-account-lockdown' ); + $error = __( 'You do not have permission to unblock this user.', 'wpvip' ); } if ( ! $error && ! delete_user_meta( $user_id, self::LAST_SEEN_META_KEY ) ) { - $error = __( 'Unable to unblock user.', 'inactive-account-lockdown' ); + $error = __( 'Unable to unblock user.', 'wpvip' ); } if ( $error ) { @@ -242,7 +255,7 @@ public function register_release_date() { if ( ! get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { // Right after the first admin_init, set the release date timestamp // to be used as a fallback for users that never logged in before. - add_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time() ); + add_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time(), '', 'no' ); } } @@ -262,7 +275,9 @@ public function is_considered_inactive( $user_id ) { } public function get_inactivity_timestamp() { - return strtotime( sprintf( '-%d days', constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; + $days = constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ? absint( constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) : 90; + + return strtotime( sprintf( '-%d days', $days ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } private function is_block_action_enabled() { @@ -270,4 +285,56 @@ private function is_block_action_enabled() { defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) && constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'BLOCK'; } + + private function should_check_user_last_seen( $user_id ) { + /** + * Filters the users that should be skipped when checking/recording the last seen. + * + * @param array $skip_users The list of users to skip. + */ + $skip_users = apply_filters( 'vip_security_last_seen_skip_users', array() ); + if ( in_array( $user_id, $skip_users ) ) { + return false; + } + + $user = get_userdata( $user_id ); + if ( ! $user ) { + throw new \Exception( 'User not found' ); + } + + $elevated_capabilities = array( + 'edit_themes', + 'switch_themes', + 'activate_plugins', + 'edit_users', + 'edit_files', + 'promote_users', + 'moderate_comments', + 'publish_posts', + 'edit_posts', + 'delete_posts', + 'publish_pages', + 'edit_pages', + 'manage_options', + 'manage_network_users', + 'manage_network_themes', + 'manage_network_plugins', + 'manage_network_options', + ); + + /** + * Filters the last seen elevated capabilities that are used to determine if the last seen should be checked. + * + * @param array $elevated_capabilities The elevated capabilities. + */ + $elevated_capabilities = apply_filters( 'vip_security_last_seen_elevated_capabilities', $elevated_capabilities ); + + foreach ( $user->allcaps as $capability => $value ) { + if ( $value === true && in_array( $capability, $elevated_capabilities ) ) { + return true; + } + } + + return false; + } } diff --git a/tests/security/test-user-last-seen.php b/tests/security/test-user-last-seen.php index 5b653f7c50..57a50ee064 100644 --- a/tests/security/test-user-last-seen.php +++ b/tests/security/test-user-last-seen.php @@ -50,10 +50,10 @@ public function test__is_considered_inactive__should_consider_user_meta() { Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); - $user_inactive_id = $this->factory()->user->create(); + $user_inactive_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_inactive_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); - $user_active_id = $this->factory()->user->create(); + $user_active_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_active_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-29 days') ); $last_seen = new User_Last_Seen(); @@ -69,7 +69,7 @@ public function test__is_considered_inactive__should_return_false_if_user_meta_a delete_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); - $user_without_meta = $this->factory()->user->create(); + $user_without_meta = $this->factory()->user->create( array( 'role' => 'administrator' ) ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); @@ -83,7 +83,7 @@ public function test__is_considered_inactive__should_use_release_date_option_whe add_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime('-16 days') ); - $user_without_meta = $this->factory()->user->create(); + $user_without_meta = $this->factory()->user->create( array( 'role' => 'administrator' ) ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); @@ -95,6 +95,26 @@ public function test__is_considered_inactive__should_use_release_date_option_whe $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); } + public function test__determine_current_user_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( 'determine_current_user' ); + + $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $new_user_id = apply_filters( 'determine_current_user', $user_id, $user_id ); + + $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + + $this->assertSame( $new_user_id, $user_id ); + $this->assertIsNumeric( $current_last_seen ); + } + public function test__determine_current_user_should_record_once_last_seen_meta() { Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); @@ -104,7 +124,7 @@ public function test__determine_current_user_should_record_once_last_seen_meta() $previous_last_seen = sprintf('%d', strtotime('-10 days') ); - $user_id = $this->factory()->user->create(); + $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $previous_last_seen ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); @@ -130,7 +150,7 @@ public function test__determine_current_user_should_return_an_error_when_user_is remove_all_filters( 'determine_current_user' ); - $user_id = $this->factory()->user->create(); + $user_id = $this->factory()->user->create( array( 'role' => 'editor' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); @@ -148,7 +168,7 @@ public function test__authenticate_should_not_return_error_when_user_is_active() remove_all_filters( 'authenticate' ); - $user_id = $this->factory()->user->create(); + $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-5 days') ); $user = get_user_by( 'id', $user_id ); @@ -168,7 +188,7 @@ public function test__authenticate_should_return_an_error_when_user_is_inactive( remove_all_filters( 'authenticate' ); - $user_id = $this->factory()->user->create(); + $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); $user = get_user_by( 'id', $user_id ); @@ -199,4 +219,74 @@ public function test__register_release_date_should_register_release_date_only_on $this->assertIsNumeric( $release_date ); $this->assertSame( $release_date, $new_release_date ); } + + public function test__authenticate_should_not_consider_users_without_elevated_capabilities() + { + Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + + remove_all_filters( 'authenticate' ); + + $user_id = $this->factory()->user->create( array( 'role' => 'subscriber' ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + + $user = get_user_by( 'id', $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $this->assertSame( $user, apply_filters( 'authenticate', $user, $user ) ); + } + + public function test__should_check_user_last_seen_should_call_elevated_capabilities_filters() + { + Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + + remove_all_filters( 'authenticate' ); + remove_all_filters( 'vip_security_last_seen_elevated_capabilities' ); + + $user_id = $this->factory()->user->create( array( 'role' => 'subscriber' ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + + $user = get_user_by( 'id', $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + add_filter( 'vip_security_last_seen_elevated_capabilities', function ( $capabilities ) { + $capabilities[] = 'read'; + + return $capabilities; + } ); + + $user = apply_filters( 'authenticate', $user, $user ); + + $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); + } + + public function test__should_check_user_last_seen_should_call_skip_users_filters() + { + Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + + remove_all_filters( 'authenticate' ); + remove_all_filters( 'vip_security_last_seen_elevated_capabilities' ); + + $user_id = $this->factory()->user->create( array( 'role' => 'subscriber' ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + + $user = get_user_by( 'id', $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + add_filter( 'vip_security_last_seen_skip_users', function ( $users ) use ( $user_id ) { + $users[] = $user_id; + + return $users; + } ); + + $this->assertSame( $user, apply_filters( 'authenticate', $user, $user ) ); + } } From c74c4bbcdf297f4363a1153030750785e09d781f Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 24 Aug 2023 12:37:30 -0300 Subject: [PATCH 17/27] Check user caps inside is_considered_inactive --- security/user-last-seen.php | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index a4693a783a..3f760b855f 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -45,10 +45,6 @@ public function determine_current_user( $user_id ) { return $user_id; } - if ( ! $this->should_check_user_last_seen( $user_id ) ) { - return $user_id; - } - if ( wp_cache_get( $user_id, self::LAST_SEEN_CACHE_GROUP ) ) { // Last seen meta was checked recently return $user_id; @@ -74,10 +70,6 @@ public function authenticate( $user ) { return $user; } - if ( ! $this->should_check_user_last_seen( $user->ID ) ) { - return $user; - } - if ( $user->ID && $this->is_block_action_enabled() && $this->is_considered_inactive( $user->ID ) ) { return new \WP_Error( 'inactive_account', @@ -260,6 +252,10 @@ public function register_release_date() { } public function is_considered_inactive( $user_id ) { + if ( ! $this->should_check_user_last_seen( $user_id ) ) { + return false; + } + $last_seen_timestamp = get_user_meta( $user_id, self::LAST_SEEN_META_KEY, true ); if ( $last_seen_timestamp ) { return $last_seen_timestamp < $this->get_inactivity_timestamp(); @@ -330,7 +326,7 @@ private function should_check_user_last_seen( $user_id ) { $elevated_capabilities = apply_filters( 'vip_security_last_seen_elevated_capabilities', $elevated_capabilities ); foreach ( $user->allcaps as $capability => $value ) { - if ( $value === true && in_array( $capability, $elevated_capabilities ) ) { + if ( true === $value && in_array( $capability, $elevated_capabilities ) ) { return true; } } From d5adcb43daa956b7fdcd93859b0bc89e03f6eb71 Mon Sep 17 00:00:00 2001 From: Caleb Burks Date: Tue, 5 Sep 2023 21:49:11 -0500 Subject: [PATCH 18/27] Couple tweaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removed excess $this->is_block_action_enabled() call - Don’t lookup the option for ajax requests (which can be logged-out FE users) - VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS doesn’t have to be defined in order for the block action to be enabled. It defaults to 90 days. - Re-organized the capabilities (most common first, more or less). Uses user_can() so that filters have a say in the matter. --- security/user-last-seen.php | 68 +++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 3f760b855f..645ecc1781 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -12,7 +12,7 @@ public function init() { return; } - // Use a global cache group to avoid having to the data for each site + // 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( 'admin_init', array( $this, 'register_release_date' ) ); @@ -70,11 +70,8 @@ public function authenticate( $user ) { return $user; } - if ( $user->ID && $this->is_block_action_enabled() && $this->is_considered_inactive( $user->ID ) ) { - return new \WP_Error( - 'inactive_account', - __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) - ); + if ( $user->ID && $this->is_considered_inactive( $user->ID ) ) { + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); } return $user; @@ -197,7 +194,7 @@ public function last_seen_unblock_action() { } ); } - if ( ! isset( $_GET['user_id'] ) || ! isset( $_GET['action'] ) || 'reset_last_seen' !== $_GET['action'] ) { + if ( ! isset( $_GET['user_id'], $_GET['action'] ) || 'reset_last_seen' !== $_GET['action'] ) { return; } @@ -244,7 +241,7 @@ public function last_seen_unblock_action() { } public function register_release_date() { - if ( ! get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { + if ( ! wp_doing_ajax() && ! get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { // Right after the first admin_init, set the release date timestamp // to be used as a fallback for users that never logged in before. add_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time(), '', 'no' ); @@ -266,27 +263,25 @@ public function is_considered_inactive( $user_id ) { return $release_date_timestamp < $this->get_inactivity_timestamp(); } - // Release date is not defined yed, so we can't consider the user inactive. + // Release date is not defined yet, so we can't consider the user inactive. return false; } - public function get_inactivity_timestamp() { + private function get_inactivity_timestamp() { $days = constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ? absint( constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) : 90; return strtotime( sprintf( '-%d days', $days ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; } private function is_block_action_enabled() { - return defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) && - defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) && - constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'BLOCK'; + return defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) && constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'BLOCK'; } private function should_check_user_last_seen( $user_id ) { /** * Filters the users that should be skipped when checking/recording the last seen. * - * @param array $skip_users The list of users to skip. + * @param array $skip_users The list of user IDs to skip. */ $skip_users = apply_filters( 'vip_security_last_seen_skip_users', array() ); if ( in_array( $user_id, $skip_users ) ) { @@ -298,35 +293,34 @@ private function should_check_user_last_seen( $user_id ) { throw new \Exception( 'User not found' ); } - $elevated_capabilities = array( - 'edit_themes', - 'switch_themes', - 'activate_plugins', - 'edit_users', - 'edit_files', - 'promote_users', - 'moderate_comments', - 'publish_posts', - 'edit_posts', - 'delete_posts', - 'publish_pages', - 'edit_pages', - 'manage_options', - 'manage_network_users', - 'manage_network_themes', - 'manage_network_plugins', - 'manage_network_options', - ); - /** * Filters the last seen elevated capabilities that are used to determine if the last seen should be checked. * * @param array $elevated_capabilities The elevated capabilities. */ - $elevated_capabilities = apply_filters( 'vip_security_last_seen_elevated_capabilities', $elevated_capabilities ); + $elevated_capabilities = apply_filters( 'vip_security_last_seen_elevated_capabilities', [ + 'edit_posts', + 'delete_posts', + 'publish_posts', + 'edit_pages', + 'delete_pages', + 'publish_pages', + 'edit_others_posts', + 'edit_others_pages', + 'manage_options', + 'edit_users', + 'promote_users', + 'activate_plugins', + 'manage_network', + ] ); + + // Prevent infinite loops inside user_can() due to other security logic. + if ( is_automattician( $user_id ) ) { + return true; + } - foreach ( $user->allcaps as $capability => $value ) { - if ( true === $value && in_array( $capability, $elevated_capabilities ) ) { + foreach ( $elevated_capabilities as $elevated_capability ) { + if ( user_can( $user, $elevated_capability ) ) { return true; } } From f7ef136db9ce0523295fcf048f5447f106dbf998 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Wed, 6 Sep 2023 18:46:02 -0300 Subject: [PATCH 19/27] Introduce ignore inactivity check until --- security/user-last-seen.php | 43 +++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index 645ecc1781..ab03c0d6ea 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -2,16 +2,21 @@ namespace Automattic\VIP\Security; class User_Last_Seen { - const LAST_SEEN_META_KEY = 'wpvip_last_seen'; - const LAST_SEEN_CACHE_GROUP = 'wpvip_last_seen'; - const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations - const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; + const LAST_SEEN_META_KEY = 'wpvip_last_seen'; + const LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY = 'wpvip_last_seen_ignore_inactivity_check_until'; + const LAST_SEEN_CACHE_GROUP = 'wpvip_last_seen'; + const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations + const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; + + private $release_date = null; public function init() { if ( ! defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) || constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'NO_ACTION' ) { return; } + $this->release_date = get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); + // Use a global cache group since users are shared among network sites. wp_cache_add_global_groups( array( self::LAST_SEEN_CACHE_GROUP ) ); @@ -116,17 +121,16 @@ public function add_last_seen_column_date( $default, $column_name, $user_id ) { $last_seen_timestamp = get_user_meta( $user_id, self::LAST_SEEN_META_KEY, true ); - if ( ! $last_seen_timestamp ) { - return $default; + $date = __( 'Indeterminate', 'wpvip' ); + if ( $last_seen_timestamp ) { + $date = sprintf( + /* translators: 1: Comment date, 2: Comment time. */ + __( '%1$s at %2$s' ), + date_i18n( get_option( 'date_format' ), $last_seen_timestamp ), + date_i18n( get_option( 'time_format' ), $last_seen_timestamp ) + ); } - $date = sprintf( - /* translators: 1: Comment date, 2: Comment time. */ - __( '%1$s at %2$s' ), - date_i18n( get_option( 'date_format' ), $last_seen_timestamp ), - date_i18n( get_option( 'time_format' ), $last_seen_timestamp ) - ); - if ( ! $this->is_block_action_enabled() || ! $this->is_considered_inactive( $user_id ) ) { return sprintf( '%s', esc_html( $date ) ); } @@ -213,7 +217,8 @@ public function last_seen_unblock_action() { $error = __( 'You do not have permission to unblock this user.', 'wpvip' ); } - if ( ! $error && ! delete_user_meta( $user_id, self::LAST_SEEN_META_KEY ) ) { + $ignore_inactivity_check_until = strtotime( '+2 days' ); + if ( ! $error && ! update_user_meta( $user_id, self::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY, $ignore_inactivity_check_until ) ) { $error = __( 'Unable to unblock user.', 'wpvip' ); } @@ -253,6 +258,11 @@ public function is_considered_inactive( $user_id ) { return false; } + $ignore_inactivity_check_until = get_user_meta( $user_id, self::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY, true ); + if ( $ignore_inactivity_check_until && $ignore_inactivity_check_until > time() ) { + return false; + } + $last_seen_timestamp = get_user_meta( $user_id, self::LAST_SEEN_META_KEY, true ); if ( $last_seen_timestamp ) { return $last_seen_timestamp < $this->get_inactivity_timestamp(); @@ -293,6 +303,10 @@ private function should_check_user_last_seen( $user_id ) { throw new \Exception( 'User not found' ); } + if ( $user->user_registered && strtotime( $user->user_registered ) > $this->get_inactivity_timestamp() ) { + return false; + } + /** * Filters the last seen elevated capabilities that are used to determine if the last seen should be checked. * @@ -325,6 +339,7 @@ private function should_check_user_last_seen( $user_id ) { } } + return false; } } From d90217187af605c8e51df27c56d32f047e390b07 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 7 Sep 2023 11:05:29 -0300 Subject: [PATCH 20/27] Add extra time when user is promoted to prevent user lockdown --- security/user-last-seen.php | 38 ++++++++++++++++++++++---- vip-support/class-vip-support-user.php | 2 ++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/security/user-last-seen.php b/security/user-last-seen.php index ab03c0d6ea..56a1a9b55d 100644 --- a/security/user-last-seen.php +++ b/security/user-last-seen.php @@ -8,8 +8,6 @@ class User_Last_Seen { const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; - private $release_date = null; - public function init() { if ( ! defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) || constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'NO_ACTION' ) { return; @@ -21,6 +19,12 @@ public function init() { wp_cache_add_global_groups( array( self::LAST_SEEN_CACHE_GROUP ) ); 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 ) { + $ignore_inactivity_check_until = strtotime( '+2 hours' ); + + $this->ignore_inactivity_check_for_user( $user_id, $ignore_inactivity_check_until ); + } ); add_filter( 'determine_current_user', array( $this, 'determine_current_user' ), 30, 1 ); @@ -218,7 +222,7 @@ public function last_seen_unblock_action() { } $ignore_inactivity_check_until = strtotime( '+2 days' ); - if ( ! $error && ! update_user_meta( $user_id, self::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY, $ignore_inactivity_check_until ) ) { + if ( ! $error && ! $this->ignore_inactivity_check_for_user( $user_id, $ignore_inactivity_check_until ) ) { $error = __( 'Unable to unblock user.', 'wpvip' ); } @@ -245,6 +249,27 @@ public function last_seen_unblock_action() { exit(); } + public function ignore_inactivity_check_for_user( $user_id, $until_timestamp = null ) { + if ( ! $until_timestamp ) { + $until_timestamp = strtotime( '+2 days' ); + } + + return update_user_meta( $user_id, self::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY, $until_timestamp ); + } + + public function user_promoted( $user_id ) { + $user = get_userdata( $user_id ); + if ( ! $user ) { + throw new \Exception( 'User not found' ); + } + + if ( ! $this->user_with_elevated_capabilities( $user ) ) { + return; + } + + $this->ignore_inactivity_check_for_user( $user_id ); + } + public function register_release_date() { if ( ! wp_doing_ajax() && ! get_option( self::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ) ) { // Right after the first admin_init, set the release date timestamp @@ -307,6 +332,10 @@ private function should_check_user_last_seen( $user_id ) { return false; } + return $this->user_with_elevated_capabilities( $user ); + } + + private function user_with_elevated_capabilities( $user ) { /** * Filters the last seen elevated capabilities that are used to determine if the last seen should be checked. * @@ -329,7 +358,7 @@ private function should_check_user_last_seen( $user_id ) { ] ); // Prevent infinite loops inside user_can() due to other security logic. - if ( is_automattician( $user_id ) ) { + if ( is_automattician( $user->ID ) ) { return true; } @@ -339,7 +368,6 @@ private function should_check_user_last_seen( $user_id ) { } } - return false; } } diff --git a/vip-support/class-vip-support-user.php b/vip-support/class-vip-support-user.php index b21db5d3b7..e6b35a4351 100644 --- a/vip-support/class-vip-support-user.php +++ b/vip-support/class-vip-support-user.php @@ -1038,6 +1038,8 @@ public static function add( $user_data ) { grant_super_admin( $user->ID ); } + do_action( 'vip_support_user_added', $user_id ); + return $user_id; } From ed87a3ead0a851b71e8c804f07eda4a744beafbc Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 14 Sep 2023 18:52:05 -0400 Subject: [PATCH 21/27] Add tests; Fix linting --- security.php | 2 +- ...last-seen.php => class-user-last-seen.php} | 0 ...seen.php => test-class-user-last-seen.php} | 57 ++++++++++++++++--- 3 files changed, 51 insertions(+), 8 deletions(-) rename security/{user-last-seen.php => class-user-last-seen.php} (100%) rename tests/security/{test-user-last-seen.php => test-class-user-last-seen.php} (81%) diff --git a/security.php b/security.php index 89f77371c9..21307291ef 100644 --- a/security.php +++ b/security.php @@ -17,7 +17,7 @@ require_once __DIR__ . '/security/password.php'; if ( defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) ) { - require_once __DIR__ . '/security/user-last-seen.php'; + require_once __DIR__ . '/security/class-user-last-seen.php'; $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); diff --git a/security/user-last-seen.php b/security/class-user-last-seen.php similarity index 100% rename from security/user-last-seen.php rename to security/class-user-last-seen.php diff --git a/tests/security/test-user-last-seen.php b/tests/security/test-class-user-last-seen.php similarity index 81% rename from tests/security/test-user-last-seen.php rename to tests/security/test-class-user-last-seen.php index 57a50ee064..b94283fc85 100644 --- a/tests/security/test-user-last-seen.php +++ b/tests/security/test-class-user-last-seen.php @@ -6,7 +6,7 @@ use Automattic\VIP\Security\User_Last_Seen; use WP_UnitTestCase; -require_once __DIR__ . '/../../security/user-last-seen.php'; +require_once __DIR__ . '/../../security/class-user-last-seen.php'; class User_Last_Seen_Test extends WP_UnitTestCase { public function setUp(): void { @@ -46,14 +46,57 @@ public function test__should_not_load_actions_and_filters_when_env_vars_is_set_t $this->assertFalse( has_filter( 'authenticate' ) ); } + public function test__is_considered_inactive__should_consider_user_registered() { + 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' ) ); + + // Recent registered user + $user1 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => date('Y-m-d' ) ) ); + + // Inactive user (last seen 20 days ago) + $user2 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); + add_user_meta( $user2, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); + + // Active user (last seen 2 days ago) + $user3 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); + add_user_meta( $user3, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-2 days') ); + + // Old user without meta + $user4 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => date('Y-m-d', strtotime('-40 days' ) ) ) ); + + $last_seen = new User_Last_Seen(); + $last_seen->init(); + + $this->assertFalse( $last_seen->is_considered_inactive( $user1 ) ); + $this->assertTrue( $last_seen->is_considered_inactive( $user2 ) ); + $this->assertFalse( $last_seen->is_considered_inactive( $user3 ) ); + $this->assertTrue( $last_seen->is_considered_inactive( $user4 ) ); + } + + public function test__is_considered_inactive__add_extra_time_when_user_is_promoted() + { + Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); + + $user_id = $this->factory()->user->create( array( 'role' => 'subscriber', 'user_registered' => '2020-01-01' ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); + + $user = get_user_by( 'ID', $user_id ); + $user->set_role( 'administrator' ); + + $last_seen = new User_Last_Seen(); + $last_seen->init(); + + $this->assertTrue( $last_seen->is_considered_inactive( $user_id ) ); + } + public function test__is_considered_inactive__should_consider_user_meta() { Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); - $user_inactive_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + $user_inactive_id = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); add_user_meta( $user_inactive_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); - $user_active_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + $user_active_id = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); add_user_meta( $user_active_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-29 days') ); $last_seen = new User_Last_Seen(); @@ -83,7 +126,7 @@ public function test__is_considered_inactive__should_use_release_date_option_whe add_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime('-16 days') ); - $user_without_meta = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + $user_without_meta = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); @@ -150,7 +193,7 @@ public function test__determine_current_user_should_return_an_error_when_user_is remove_all_filters( 'determine_current_user' ); - $user_id = $this->factory()->user->create( array( 'role' => 'editor' ) ); + $user_id = $this->factory()->user->create( array( 'role' => 'editor', 'user_registered' => '2020-01-01' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); @@ -188,7 +231,7 @@ public function test__authenticate_should_return_an_error_when_user_is_inactive( remove_all_filters( 'authenticate' ); - $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('-100 days') ); $user = get_user_by( 'id', $user_id ); @@ -246,7 +289,7 @@ public function test__should_check_user_last_seen_should_call_elevated_capabilit remove_all_filters( 'authenticate' ); remove_all_filters( 'vip_security_last_seen_elevated_capabilities' ); - $user_id = $this->factory()->user->create( array( 'role' => 'subscriber' ) ); + $user_id = $this->factory()->user->create( array( 'role' => 'subscriber', '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 ); From 9cb737beeb64f575e32cf9747b2d0496ae4ad293 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 14 Sep 2023 19:01:25 -0400 Subject: [PATCH 22/27] Fix linting --- tests/security/test-class-user-last-seen.php | 168 +++++++++++-------- 1 file changed, 94 insertions(+), 74 deletions(-) diff --git a/tests/security/test-class-user-last-seen.php b/tests/security/test-class-user-last-seen.php index b94283fc85..34b1e4ced8 100644 --- a/tests/security/test-class-user-last-seen.php +++ b/tests/security/test-class-user-last-seen.php @@ -47,22 +47,34 @@ public function test__should_not_load_actions_and_filters_when_env_vars_is_set_t } public function test__is_considered_inactive__should_consider_user_registered() { - 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' ) ); + 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' ) ); // Recent registered user - $user1 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => date('Y-m-d' ) ) ); + $user1 = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => gmdate( 'Y-m-d' ), + ) ); // Inactive user (last seen 20 days ago) - $user2 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user2, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); + $user2 = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user2, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-31 days' ) ); // Active user (last seen 2 days ago) - $user3 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user3, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-2 days') ); + $user3 = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user3, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-2 days' ) ); // Old user without meta - $user4 = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => date('Y-m-d', strtotime('-40 days' ) ) ) ); + $user4 = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => gmdate( 'Y-m-d', strtotime( '-40 days' ) ), + ) ); $last_seen = new User_Last_Seen(); $last_seen->init(); @@ -73,12 +85,14 @@ public function test__is_considered_inactive__should_consider_user_registered() $this->assertTrue( $last_seen->is_considered_inactive( $user4 ) ); } - public function test__is_considered_inactive__add_extra_time_when_user_is_promoted() - { - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); + public function test__is_considered_inactive__add_extra_time_when_user_is_promoted() { + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30 ); - $user_id = $this->factory()->user->create( array( 'role' => 'subscriber', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); + $user_id = $this->factory()->user->create( array( + 'role' => 'subscriber', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-31 days' ) ); $user = get_user_by( 'ID', $user_id ); $user->set_role( 'administrator' ); @@ -89,15 +103,20 @@ public function test__is_considered_inactive__add_extra_time_when_user_is_promot $this->assertTrue( $last_seen->is_considered_inactive( $user_id ) ); } - public function test__is_considered_inactive__should_consider_user_meta() - { - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30); + public function test__is_considered_inactive__should_consider_user_meta() { + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 30 ); - $user_inactive_id = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user_inactive_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-31 days') ); + $user_inactive_id = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user_inactive_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-31 days' ) ); - $user_active_id = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user_active_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-29 days') ); + $user_active_id = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user_active_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-29 days' ) ); $last_seen = new User_Last_Seen(); $last_seen->init(); @@ -106,9 +125,8 @@ public function test__is_considered_inactive__should_consider_user_meta() $this->assertFalse( $last_seen->is_considered_inactive( $user_active_id ) ); } - 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); + 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 ); delete_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); @@ -120,28 +138,29 @@ public function test__is_considered_inactive__should_return_false_if_user_meta_a $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); } - public function test__is_considered_inactive__should_use_release_date_option_when_user_meta_is_not_defined() - { - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + public function test__is_considered_inactive__should_use_release_date_option_when_user_meta_is_not_defined() { + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); - add_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime('-16 days') ); + add_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-16 days' ) ); - $user_without_meta = $this->factory()->user->create( array( 'role' => 'administrator', 'user_registered' => '2020-01-01' ) ); + $user_without_meta = $this->factory()->user->create( array( + 'role' => 'administrator', + 'user_registered' => '2020-01-01', + ) ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); $this->assertTrue( $last_seen->is_considered_inactive( $user_without_meta ) ); - update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime('-10 days') ); + update_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-10 days' ) ); $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); } - public function test__determine_current_user_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); + public function test__determine_current_user_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( 'determine_current_user' ); @@ -158,14 +177,13 @@ public function test__determine_current_user_should_record_last_seen_meta() $this->assertIsNumeric( $current_last_seen ); } - public function test__determine_current_user_should_record_once_last_seen_meta() - { - Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + public function test__determine_current_user_should_record_once_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( 'determine_current_user' ); - $previous_last_seen = sprintf('%d', strtotime('-10 days') ); + $previous_last_seen = sprintf( '%d', strtotime( '-10 days' ) ); $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $previous_last_seen ); @@ -186,15 +204,17 @@ public function test__determine_current_user_should_record_once_last_seen_meta() $this->assertSame( $new_user_id, $user_id ); } - public function test__determine_current_user_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); + public function test__determine_current_user_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 ); remove_all_filters( 'determine_current_user' ); - $user_id = $this->factory()->user->create( array( 'role' => 'editor', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + $user_id = $this->factory()->user->create( array( + 'role' => 'editor', + 'user_registered' => '2020-01-01', + ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-100 days' ) ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); @@ -204,15 +224,14 @@ public function test__determine_current_user_should_return_an_error_when_user_is $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); } - 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); + 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 ); remove_all_filters( 'authenticate' ); $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-5 days') ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-5 days' ) ); $user = get_user_by( 'id', $user_id ); @@ -224,15 +243,17 @@ public function test__authenticate_should_not_return_error_when_user_is_active() $this->assertSame( $user->ID, $new_user->ID ); } - 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); + 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 ); remove_all_filters( 'authenticate' ); - $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('-100 days') ); + $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( '-100 days' ) ); $user = get_user_by( 'id', $user_id ); @@ -244,9 +265,8 @@ 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__register_release_date_should_register_release_date_only_once() - { - Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'RECORD_LAST_SEEN' ); + public function test__register_release_date_should_register_release_date_only_once() { + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'RECORD_LAST_SEEN' ); remove_all_actions( 'admin_init' ); delete_option( User_Last_Seen::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY ); @@ -263,15 +283,14 @@ public function test__register_release_date_should_register_release_date_only_on $this->assertSame( $release_date, $new_release_date ); } - public function test__authenticate_should_not_consider_users_without_elevated_capabilities() - { - Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + public function test__authenticate_should_not_consider_users_without_elevated_capabilities() { + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); remove_all_filters( 'authenticate' ); $user_id = $this->factory()->user->create( array( 'role' => 'subscriber' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-100 days' ) ); $user = get_user_by( 'id', $user_id ); @@ -281,16 +300,18 @@ public function test__authenticate_should_not_consider_users_without_elevated_ca $this->assertSame( $user, apply_filters( 'authenticate', $user, $user ) ); } - public function test__should_check_user_last_seen_should_call_elevated_capabilities_filters() - { - Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + public function test__should_check_user_last_seen_should_call_elevated_capabilities_filters() { + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); remove_all_filters( 'authenticate' ); remove_all_filters( 'vip_security_last_seen_elevated_capabilities' ); - $user_id = $this->factory()->user->create( array( 'role' => 'subscriber', 'user_registered' => '2020-01-01' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + $user_id = $this->factory()->user->create( array( + 'role' => 'subscriber', + '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 ); @@ -308,16 +329,15 @@ public function test__should_check_user_last_seen_should_call_elevated_capabilit $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); } - public function test__should_check_user_last_seen_should_call_skip_users_filters() - { - Constant_Mocker::define('VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); - Constant_Mocker::define('VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15); + public function test__should_check_user_last_seen_should_call_skip_users_filters() { + Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'BLOCK' ); + Constant_Mocker::define( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS', 15 ); remove_all_filters( 'authenticate' ); remove_all_filters( 'vip_security_last_seen_elevated_capabilities' ); $user_id = $this->factory()->user->create( array( 'role' => 'subscriber' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime('-100 days') ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-100 days' ) ); $user = get_user_by( 'id', $user_id ); From 3dc2715060d15dac6dffe9ec474c653b716823f9 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Mon, 25 Sep 2023 17:44:21 -0300 Subject: [PATCH 23/27] Avoid using determine_user filter --- security/class-user-last-seen.php | 49 +++++++----- tests/security/test-class-user-last-seen.php | 80 +++++++++++++------- 2 files changed, 84 insertions(+), 45 deletions(-) diff --git a/security/class-user-last-seen.php b/security/class-user-last-seen.php index 56a1a9b55d..69368b1cd3 100644 --- a/security/class-user-last-seen.php +++ b/security/class-user-last-seen.php @@ -1,6 +1,8 @@ ignore_inactivity_check_for_user( $user_id, $ignore_inactivity_check_until ); } ); - add_filter( 'determine_current_user', array( $this, 'determine_current_user' ), 30, 1 ); - if ( in_array( constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ), array( 'REPORT', 'BLOCK' ) ) ) { add_filter( 'wpmu_users_columns', array( $this, 'add_last_seen_column_head' ) ); add_filter( 'manage_users_columns', array( $this, 'add_last_seen_column_head' ) ); @@ -40,6 +40,7 @@ public function init() { if ( $this->is_block_action_enabled() ) { add_filter( 'authenticate', array( $this, 'authenticate' ), 20, 1 ); + add_filter( 'rest_authentication_errors', array( $this, 'rest_authentication' ), 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' ) ); @@ -49,29 +50,16 @@ public function init() { } } - public function determine_current_user( $user_id ) { - if ( ! $user_id ) { - return $user_id; - } - + public function record_activity( $user_id ) { if ( wp_cache_get( $user_id, self::LAST_SEEN_CACHE_GROUP ) ) { // Last seen meta was checked recently - return $user_id; - } - - if ( $this->is_block_action_enabled() && $this->is_considered_inactive( $user_id ) ) { - // Force current user to 0 to avoid recursive calls to this filter - wp_set_current_user( 0 ); - - return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); + return; } // 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() ); } - - return $user_id; } public function authenticate( $user ) { @@ -80,12 +68,39 @@ public function authenticate( $user ) { } if ( $user->ID && $this->is_considered_inactive( $user->ID ) ) { + if ( Context::is_xmlrpc_api() ) { + add_filter('xmlrpc_login_error', function () { + return new \IXR_Error( 403, __( 'Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); + }); + } + return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); } + $this->record_activity( $user->ID ); + return $user; } + public function rest_authentication( $status ) { + if ( is_wp_error( $status ) || ! $status ) { + return $status; + } + + $user_id = get_current_user_id(); + if ( ! $user_id ) { + return $status; + } + + if ( $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 ) ); + } + + $this->record_activity( $user_id ); + + return $status; + } + public function add_last_seen_column_head( $columns ) { $columns['last_seen'] = __( 'Last seen', 'wpvip' ); return $columns; diff --git a/tests/security/test-class-user-last-seen.php b/tests/security/test-class-user-last-seen.php index 34b1e4ced8..79120a9d2c 100644 --- a/tests/security/test-class-user-last-seen.php +++ b/tests/security/test-class-user-last-seen.php @@ -23,26 +23,26 @@ public function tearDown(): void { public function test__should_not_load_actions_and_filters_when_env_vars_are_not_defined() { Constant_Mocker::undefine( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ); - remove_all_filters( 'determine_current_user' ); + remove_all_filters( 'rest_authentication_errors' ); remove_all_filters( 'authenticate' ); $last_seen = new User_Last_Seen(); $last_seen->init(); - $this->assertFalse( has_filter( 'determine_current_user' ) ); + $this->assertFalse( has_filter( 'rest_authentication_errors' ) ); $this->assertFalse( has_filter( 'authenticate' ) ); } public function test__should_not_load_actions_and_filters_when_env_vars_is_set_to_no_action() { Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'NO_ACTION' ); - remove_all_filters( 'determine_current_user' ); + remove_all_filters( 'rest_authentication_errors' ); remove_all_filters( 'authenticate' ); $last_seen = new User_Last_Seen(); $last_seen->init(); - $this->assertFalse( has_filter( 'determine_current_user' ) ); + $this->assertFalse( has_filter( 'rest_authentication_errors' ) ); $this->assertFalse( has_filter( 'authenticate' ) ); } @@ -158,96 +158,120 @@ public function test__is_considered_inactive__should_use_release_date_option_whe $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); } - public function test__determine_current_user_should_record_last_seen_meta() { + public function test__authenticate_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( 'determine_current_user' ); + remove_all_filters( 'authenticate' ); $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + $user = get_user_by( 'id', $user_id ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); - $new_user_id = apply_filters( 'determine_current_user', $user_id, $user_id ); + $new_user = apply_filters( 'authenticate', $user ); $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); - $this->assertSame( $new_user_id, $user_id ); + $this->assertSame( $new_user->ID, $user->ID ); $this->assertIsNumeric( $current_last_seen ); } - public function test__determine_current_user_should_record_once_last_seen_meta() { + public function test__authenticate_should_record_once_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( 'determine_current_user' ); + remove_all_filters( 'authenticate' ); $previous_last_seen = sprintf( '%d', strtotime( '-10 days' ) ); $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $previous_last_seen ); + $user = get_user_by( 'id', $user_id ); $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); - apply_filters( 'determine_current_user', $user_id, $user_id ); + apply_filters( 'authenticate', $user ); $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); - $new_user_id = apply_filters( 'determine_current_user', $user_id, $user_id ); + $new_user = apply_filters( 'authenticate', $user ); $cached_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); $this->assertTrue( $current_last_seen > $previous_last_seen ); $this->assertSame( $current_last_seen, $cached_last_seen ); - $this->assertSame( $new_user_id, $user_id ); + $this->assertSame( $new_user->ID, $user_id ); } - public function test__determine_current_user_should_return_an_error_when_user_is_inactive() { + 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 ); - remove_all_filters( 'determine_current_user' ); + remove_all_filters( 'authenticate' ); + + $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); + add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-5 days' ) ); + + $user = get_user_by( 'id', $user_id ); + + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); + $last_seen->init(); + + $new_user = apply_filters( 'authenticate', $user, $user ); + + $this->assertSame( $user->ID, $new_user->ID ); + } + + 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 ); + + remove_all_filters( 'authenticate' ); $user_id = $this->factory()->user->create( array( - 'role' => 'editor', + 'role' => 'administrator', '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 ); + $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); - $user = apply_filters( 'determine_current_user', $user_id, $user_id ); + $user = apply_filters( 'authenticate', $user, $user ); $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); } - public function test__authenticate_should_not_return_error_when_user_is_active() { + 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( 'authenticate' ); + remove_all_filters( 'rest_authentication_errors' ); $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, strtotime( '-5 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(); - $new_user = apply_filters( 'authenticate', $user, $user ); + apply_filters( 'rest_authentication_errors', true ); - $this->assertSame( $user->ID, $new_user->ID ); + $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); + + $this->assertIsNumeric( $current_last_seen ); } - public function test__authenticate_should_return_an_error_when_user_is_inactive() { + 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 ); - remove_all_filters( 'authenticate' ); + remove_all_filters( 'rest_authentication_errors' ); $user_id = $this->factory()->user->create( array( 'role' => 'administrator', @@ -255,14 +279,14 @@ public function test__authenticate_should_return_an_error_when_user_is_inactive( ) ); 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(); - $user = apply_filters( 'authenticate', $user, $user ); + $result = apply_filters( 'rest_authentication_errors', true ); - $this->assertWPError( $user, 'Expected WP_Error object to be returned' ); + $this->assertWPError( $result, 'Expected WP_Error object to be returned' ); } public function test__register_release_date_should_register_release_date_only_once() { From 550b4c4bde6b03af4eb340d51cddad5de3ba7887 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Wed, 27 Sep 2023 15:02:47 -0300 Subject: [PATCH 24/27] Register activity on set_current_user and rest_authentication_errors --- security/class-user-last-seen.php | 34 ++++++--- tests/security/test-class-user-last-seen.php | 72 +++++++------------- 2 files changed, 46 insertions(+), 60 deletions(-) diff --git a/security/class-user-last-seen.php b/security/class-user-last-seen.php index 69368b1cd3..87908ff302 100644 --- a/security/class-user-last-seen.php +++ b/security/class-user-last-seen.php @@ -20,6 +20,9 @@ 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_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 ) { @@ -40,7 +43,6 @@ public function init() { if ( $this->is_block_action_enabled() ) { add_filter( 'authenticate', array( $this, 'authenticate' ), 20, 1 ); - add_filter( 'rest_authentication_errors', array( $this, 'rest_authentication' ), 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' ) ); @@ -50,15 +52,27 @@ public function init() { } } - public function record_activity( $user_id ) { - if ( wp_cache_get( $user_id, self::LAST_SEEN_CACHE_GROUP ) ) { + public function record_activity( $user = null ) { + if ( $user === null ) { + $user = wp_get_current_user(); + } + + if ( ! $user || ! $user->ID ) { + return; + } + + if ( ! $this->user_with_elevated_capabilities( $user ) ) { + return; + } + + if ( wp_cache_get( $user->ID, self::LAST_SEEN_CACHE_GROUP ) ) { // Last seen meta was checked recently return; } // 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() ); } } @@ -77,8 +91,6 @@ public function authenticate( $user ) { return new \WP_Error( 'inactive_account', __( 'Error: Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ) ); } - $this->record_activity( $user->ID ); - return $user; } @@ -87,16 +99,16 @@ public function rest_authentication( $status ) { return $status; } - $user_id = get_current_user_id(); - if ( ! $user_id ) { + $user = wp_get_current_user(); + if ( ! $user->ID ) { return $status; } - if ( $this->is_considered_inactive( $user_id ) ) { + 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 ) ); } - $this->record_activity( $user_id ); + $this->record_activity( $user ); return $status; } diff --git a/tests/security/test-class-user-last-seen.php b/tests/security/test-class-user-last-seen.php index 79120a9d2c..38dafec768 100644 --- a/tests/security/test-class-user-last-seen.php +++ b/tests/security/test-class-user-last-seen.php @@ -158,54 +158,6 @@ public function test__is_considered_inactive__should_use_release_date_option_whe $this->assertFalse( $last_seen->is_considered_inactive( $user_without_meta ) ); } - public function test__authenticate_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( 'authenticate' ); - - $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); - $user = get_user_by( 'id', $user_id ); - - $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); - $last_seen->init(); - - $new_user = apply_filters( 'authenticate', $user ); - - $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); - - $this->assertSame( $new_user->ID, $user->ID ); - $this->assertIsNumeric( $current_last_seen ); - } - - public function test__authenticate_should_record_once_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( 'authenticate' ); - - $previous_last_seen = sprintf( '%d', strtotime( '-10 days' ) ); - - $user_id = $this->factory()->user->create( array( 'role' => 'administrator' ) ); - add_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, $previous_last_seen ); - $user = get_user_by( 'id', $user_id ); - - $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); - $last_seen->init(); - - apply_filters( 'authenticate', $user ); - - $current_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); - - $new_user = apply_filters( 'authenticate', $user ); - - $cached_last_seen = get_user_meta( $user_id, User_Last_Seen::LAST_SEEN_META_KEY, true ); - - $this->assertTrue( $current_last_seen > $previous_last_seen ); - $this->assertSame( $current_last_seen, $cached_last_seen ); - $this->assertSame( $new_user->ID, $user_id ); - } - 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 ); @@ -260,11 +212,12 @@ public function test__rest_authentication_should_record_last_seen_meta() { $last_seen = new \Automattic\VIP\Security\User_Last_Seen(); $last_seen->init(); - apply_filters( 'rest_authentication_errors', true ); + $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() { @@ -289,6 +242,27 @@ public function test__rest_authentication_should_return_an_error_when_user_is_in $this->assertWPError( $result, 'Expected WP_Error object to be returned' ); } + 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_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__register_release_date_should_register_release_date_only_once() { Constant_Mocker::define( 'VIP_SECURITY_INACTIVE_USERS_ACTION', 'RECORD_LAST_SEEN' ); From 24b1f8277e8848d43ab0812798bf70ed444d96cb Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Wed, 18 Oct 2023 19:32:29 -0300 Subject: [PATCH 25/27] 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 ); + } } From 182dc0d33f2e68dd24dc2c66eb51209c59126170 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 19 Oct 2023 09:01:40 -0300 Subject: [PATCH 26/27] Use a class property rather than a global --- security/class-user-last-seen.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/security/class-user-last-seen.php b/security/class-user-last-seen.php index de4457a747..003f477362 100644 --- a/security/class-user-last-seen.php +++ b/security/class-user-last-seen.php @@ -10,6 +10,13 @@ class User_Last_Seen { const LAST_SEEN_UPDATE_USER_META_CACHE_TTL = MINUTE_IN_SECONDS * 5; // Store last seen once every five minute to avoid too many write DB operations const LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY = 'wpvip_last_seen_release_date_timestamp'; + /** + * May store inactive account authentication error for application passwords to be used later in rest_authentication_errors + * + * @var \WP_Error|null + */ + private $application_password_authentication_error; + public function init() { if ( ! defined( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) || constant( 'VIP_SECURITY_INACTIVE_USERS_ACTION' ) === 'NO_ACTION' ) { return; @@ -100,10 +107,8 @@ public function authenticate( $user ) { } public function rest_authentication_errors( $status ) { - global $wp_last_seen_application_password_error; - - if ( is_wp_error( $wp_last_seen_application_password_error ) ) { - return $wp_last_seen_application_password_error; + if ( is_wp_error( $this->application_password_authentication_error ) ) { + return $this->application_password_authentication_error; } return $status; @@ -115,14 +120,12 @@ public function rest_authentication_errors( $status ) { * @return bool */ public function application_password_authentication( $available, $user ) { - global $wp_last_seen_application_password_error; - if ( ! $available || ( $user && ! $user->exists() ) ) { return false; } 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 ) ); + $this->application_password_authentication_error = new \WP_Error( 'inactive_account', __( 'Your account has been flagged as inactive. Please contact your site administrator.', 'wpvip' ), array( 'status' => 403 ) ); return false; } From 37eb09066e6bf80f7e8c650c9f5f94908ba77b42 Mon Sep 17 00:00:00 2001 From: Luis Henrique Mulinari Date: Thu, 25 Jan 2024 16:44:14 -0300 Subject: [PATCH 27/27] Check if constant is defined before using it --- security/class-user-last-seen.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/class-user-last-seen.php b/security/class-user-last-seen.php index 003f477362..81338df8d1 100644 --- a/security/class-user-last-seen.php +++ b/security/class-user-last-seen.php @@ -350,7 +350,7 @@ public function is_considered_inactive( $user_id ) { } private function get_inactivity_timestamp() { - $days = constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ? absint( constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) : 90; + $days = defined( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ? absint( constant( 'VIP_SECURITY_CONSIDER_USERS_INACTIVE_AFTER_DAYS' ) ) : 90; return strtotime( sprintf( '-%d days', $days ) ) + self::LAST_SEEN_UPDATE_USER_META_CACHE_TTL; }