Skip to content

Commit 778c21b

Browse files
andrea-sdlandrea-sdl
committed
Add SQL-based counting for inactive users with elevated permissions and enhance tests
1 parent 864e553 commit 778c21b

2 files changed

Lines changed: 215 additions & 30 deletions

File tree

‎modules/inactive-users/class-inactive-users.php‎

Lines changed: 144 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -318,46 +318,161 @@ public static function last_seen_order_by_query_args( $vars ) {
318318
return $vars;
319319
}
320320

321+
/**
322+
* Count inactive users with elevated permissions.
323+
*
324+
* This method uses pure SQL to count users who:
325+
* 1. Have elevated roles or capabilities (configured via settings)
326+
* 2. Are active (not spam, deleted, or have user_status != 0)
327+
* 3. Registered before the inactivity threshold
328+
* 4. Either have not been seen since the threshold, or never had a last_seen recorded
329+
*
330+
* The query structure leverages WordPress's WP_User_Query to generate the capability/role
331+
* filtering SQL, then uses that as a subquery to filter users in the main count query.
332+
*
333+
* Results are cached with a 5-minute TTL to improve performance.
334+
*
335+
* Example SQL (single site with administrator role):
336+
*
337+
* SELECT COUNT(DISTINCT u.ID)
338+
* FROM wp_users u
339+
* LEFT JOIN wp_usermeta m_last_seen
340+
* ON u.ID = m_last_seen.user_id
341+
* AND m_last_seen.meta_key = 'wpvip_last_seen'
342+
* WHERE u.user_status=0 AND u.spam=0 and u.deleted=0
343+
* AND u.user_registered < '2024-08-06 12:00:00'
344+
* AND (
345+
* (m_last_seen.meta_value IS NOT NULL AND CAST(m_last_seen.meta_value AS UNSIGNED) < 1722945600)
346+
* OR m_last_seen.meta_value IS NULL
347+
* )
348+
* AND u.ID IN (
349+
* SELECT DISTINCT wp_users.ID
350+
* FROM wp_users
351+
* INNER JOIN wp_usermeta ON wp_users.ID = wp_usermeta.user_id
352+
* WHERE 1=1
353+
* AND (wp_usermeta.meta_key = 'wp_capabilities'
354+
* AND wp_usermeta.meta_value LIKE '%\"administrator\"%')
355+
* )
356+
*
357+
* Example SQL (network-wide with administrator role):
358+
*
359+
* SELECT COUNT(DISTINCT u.ID)
360+
* FROM wp_users u
361+
* LEFT JOIN wp_usermeta m_last_seen
362+
* ON u.ID = m_last_seen.user_id
363+
* AND m_last_seen.meta_key = 'wpvip_last_seen'
364+
* WHERE u.user_status=0 AND u.spam=0 and u.deleted=0
365+
* AND u.user_registered < '2024-08-06 12:00:00'
366+
* AND (
367+
* (m_last_seen.meta_value IS NOT NULL AND CAST(m_last_seen.meta_value AS UNSIGNED) < 1722945600)
368+
* OR m_last_seen.meta_value IS NULL
369+
* )
370+
* AND u.ID IN (
371+
* SELECT DISTINCT user_id
372+
* FROM wp_usermeta
373+
* WHERE meta_key LIKE 'wp%_capabilities'
374+
* AND (
375+
* meta_value LIKE '%\"administrator\";b:1%'
376+
* )
377+
* )
378+
*
379+
* @param int|null $blog_id Optional. Blog ID to query. Use 0 for network-wide queries. Default is current blog.
380+
* @return int Count of inactive users.
381+
*/
321382
public static function get_inactive_users_count( $blog_id = null ) {
383+
global $wpdb;
384+
322385
$blog_id = $blog_id ?? get_current_blog_id();
323386

324-
// Use global cache for network-wide queries (blog_id = 0)
325-
if ( 0 === $blog_id ) {
326-
$cache_key = self::get_inactive_users_count_cache_key( $blog_id );
327-
$cached_count = wp_cache_get( $cache_key, self::LAST_SEEN_CACHE_GROUP );
328387

329-
if ( false !== $cached_count ) {
330-
return $cached_count;
331-
}
388+
$cache_key = self::get_inactive_users_count_cache_key( $blog_id );
389+
$cached_count = wp_cache_get( $cache_key, self::LAST_SEEN_CACHE_GROUP );
390+
391+
if ( false !== $cached_count ) {
392+
return $cached_count;
332393
}
333394

334-
/**
335-
* We're doing two separate queries to avoid the query getting too complex and slow since excluding the last_seen meta would add an extra join
336-
* multiplying the complexity of the query beneath it
337-
*/
338-
// Use our utility method that properly handles network-wide capability filtering
339-
$inactive_users_with_last_seen_count = \Automattic\VIP\Security\Utils\Users_Query_Utils::query_users_with_capability_filtering(
340-
self::get_inactive_users_query_args( 'with_last_seen' ),
341-
$blog_id,
342-
true // count only
343-
);
344-
$inactive_users_without_last_seen_count = 0;
345-
if ( self::is_release_date_older_than_cutoff() ) {
346-
$inactive_users_without_last_seen_count = \Automattic\VIP\Security\Utils\Users_Query_Utils::query_users_with_capability_filtering(
347-
self::get_inactive_users_query_args( 'without_last_seen' ),
348-
$blog_id,
349-
true // count only
350-
);
395+
$inact_ts = self::get_inactivity_timestamp();
396+
$release_ts = static::get_last_seen_release_date_timestamp();
397+
398+
// Build the inactivity date for user_registered comparison
399+
$inactivity_date = gmdate( 'Y-m-d H:i:s', $inact_ts );
400+
401+
// Use WordPress's WP_User_Query to build the capability/role filtering SQL
402+
$capability_query_args = [
403+
'fields' => 'ID',
404+
'count_total' => false, // Prevent SQL_CALC_FOUND_ROWS which doesn't work in subqueries
405+
];
406+
407+
// Add capability or role filtering
408+
if ( ! empty( self::$elevated_capabilities ) ) {
409+
$capability_query_args['capability__in'] = self::$elevated_capabilities;
410+
} else {
411+
$capability_query_args['role__in'] = Capability_Utils::normalize_roles_input( self::$elevated_roles );
351412
}
352413

353-
$count = $inactive_users_with_last_seen_count + $inactive_users_without_last_seen_count;
414+
// Get the prepared query data using our utility method
415+
$prepared = Users_Query_Utils::get_prepared_user_query_data( $capability_query_args, $blog_id );
416+
$user_query = $prepared['query'];
417+
$is_network = $prepared['is_network'];
418+
$capability_where = $prepared['capability_where'];
354419

355-
// Cache the result for global queries (blog_id = 0)
356-
if ( 0 === $blog_id ) {
357-
$cache_key = self::get_inactive_users_count_cache_key( $blog_id );
358-
wp_cache_set( $cache_key, $count, self::LAST_SEEN_CACHE_GROUP, self::INACTIVE_USERS_COUNT_CACHE_TTL ); // phpcs:ignore WordPressVIPMinimum.Performance.LowExpiryCacheTime.CacheTimeUndetermined
420+
// Build the subquery for users with elevated permissions
421+
// Remove SQL_CALC_FOUND_ROWS if present (not compatible with subqueries in MySQL 8+)
422+
$query_fields = str_replace( 'SQL_CALC_FOUND_ROWS ', '', $user_query->query_fields );
423+
$users_with_caps_subquery = "SELECT DISTINCT {$query_fields} {$user_query->query_from} {$user_query->query_where}";
424+
425+
if ( $is_network && ! empty( $capability_where ) ) {
426+
$users_with_caps_subquery .= " AND ({$capability_where})";
427+
}
428+
429+
// Build the last_seen conditions
430+
$last_seen_conditions = [];
431+
432+
// Users with last_seen < inactivity threshold
433+
$last_seen_conditions[] = $wpdb->prepare( '(m_last_seen.meta_value IS NOT NULL AND CAST(m_last_seen.meta_value AS UNSIGNED) < %d)', $inact_ts );
434+
435+
// Users without last_seen meta (only if release date is older than cutoff)
436+
if ( $release_ts < $inact_ts ) {
437+
$last_seen_conditions[] = 'm_last_seen.meta_value IS NULL';
359438
}
360439

440+
$last_seen_where = '(' . implode( ' OR ', $last_seen_conditions ) . ')';
441+
$last_seen_meta_key = self::LAST_SEEN_META_KEY;
442+
443+
// include only valid users.
444+
$status_conditions = 'u.user_status = 0';
445+
if ( is_multisite() ) {
446+
$status_conditions .= ' AND u.spam = 0 AND u.deleted = 0';
447+
}
448+
449+
// Build the main SQL query
450+
// phpcs:disable WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPressVIPMinimum.Variables.RestrictedVariables.user_meta__wpdb__users
451+
$sql = $wpdb->prepare(
452+
"SELECT COUNT(DISTINCT u.ID)
453+
FROM {$wpdb->users} u
454+
LEFT JOIN {$wpdb->usermeta} m_last_seen
455+
ON u.ID = m_last_seen.user_id
456+
AND m_last_seen.meta_key = %s
457+
WHERE {$status_conditions}
458+
AND u.user_registered < %s
459+
AND {$last_seen_where}
460+
AND u.ID IN ({$users_with_caps_subquery})",
461+
$last_seen_meta_key,
462+
$inactivity_date
463+
);
464+
// phpcs:enable WordPress.DB.PreparedSQL.InterpolatedNotPrepared,WordPressVIPMinimum.Variables.RestrictedVariables.user_meta__wpdb__users
465+
466+
// Execute the query
467+
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared,WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching
468+
$count = (int) $wpdb->get_var( $sql );
469+
470+
// Cache the result for global queries (blog_id = 0)
471+
472+
$cache_key = self::get_inactive_users_count_cache_key( $blog_id );
473+
wp_cache_set( $cache_key, $count, self::LAST_SEEN_CACHE_GROUP, self::INACTIVE_USERS_COUNT_CACHE_TTL ); // phpcs:ignore WordPressVIPMinimum.Performance.LowExpiryCacheTime.CacheTimeUndetermined
474+
475+
361476
return $count;
362477
}
363478

‎tests/phpunit/test-inactive-users.php‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ class InactiveUsersTest extends WP_UnitTestCase {
66
private $user_id;
77
private $elevated_roles = [ 'administrator' ];
88
private $considered_inactive_after_days = 90;
9+
private $additional_user_ids = [];
10+
private $original_release_timestamp;
911

1012
public function setUp(): void {
1113
parent::setUp();
@@ -27,17 +29,56 @@ public function setUp(): void {
2729
],
2830
]);
2931
}
32+
$this->original_release_timestamp = get_option( Inactive_Users::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, false );
3033
// Let's assume the module has been activated today
31-
add_option( Inactive_Users::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time() );
34+
update_option( Inactive_Users::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, time() );
35+
update_user_meta( $this->user_id, Inactive_Users::LAST_SEEN_META_KEY, time() );
3236

3337
Inactive_Users::init();
3438
}
3539

3640
public function tearDown(): void {
41+
foreach ( $this->additional_user_ids as $user_id ) {
42+
wp_delete_user( $user_id );
43+
}
44+
$this->additional_user_ids = [];
45+
3746
wp_delete_user( $this->user_id );
47+
48+
if ( false === $this->original_release_timestamp ) {
49+
delete_option( Inactive_Users::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY );
50+
} else {
51+
update_option( Inactive_Users::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, $this->original_release_timestamp );
52+
}
53+
3854
parent::tearDown();
3955
}
4056

57+
/**
58+
* Test that get_inactive_users_count counts users with stale last_seen values.
59+
*/
60+
public function test_get_inactive_users_count_counts_users_with_stale_last_seen(): void {
61+
$baseline_count = $this->get_fresh_inactive_users_count();
62+
63+
$user_id = $this->create_test_user();
64+
update_user_meta( $user_id, Inactive_Users::LAST_SEEN_META_KEY, strtotime( '-91 days' ) );
65+
66+
$this->assertSame( $baseline_count + 1, $this->get_fresh_inactive_users_count() );
67+
}
68+
69+
/**
70+
* Test that users without last_seen are counted once the release date predates the cutoff.
71+
*/
72+
public function test_get_inactive_users_count_counts_users_without_last_seen_after_release_cutoff(): void {
73+
update_option( Inactive_Users::LAST_SEEN_RELEASE_DATE_TIMESTAMP_OPTION_KEY, strtotime( '-400 days' ) );
74+
$baseline_count = $this->get_fresh_inactive_users_count();
75+
76+
$user_id = $this->create_test_user();
77+
delete_user_meta( $user_id, Inactive_Users::LAST_SEEN_META_KEY );
78+
79+
$this->assertSame( $baseline_count + 1, $this->get_fresh_inactive_users_count() );
80+
}
81+
4182
/**
4283
* Test that add_last_seen_column_head adds a 'Last seen' column to the columns array
4384
*/
@@ -1107,4 +1148,33 @@ public function test_application_password_authentication_inactive_user() {
11071148

11081149
wp_delete_user( $user_id );
11091150
}
1151+
1152+
/**
1153+
* Create a test user and ensure it is cleaned up after the test.
1154+
*
1155+
* @param array $overrides Optional. User factory overrides.
1156+
* @return int The created user ID.
1157+
*/
1158+
private function create_test_user( array $overrides = [] ): int {
1159+
$defaults = [
1160+
'role' => 'administrator',
1161+
'user_registered' => gmdate( 'Y-m-d H:i:s', strtotime( '-120 days' ) ),
1162+
];
1163+
1164+
$user_id = $this->factory->user->create( array_merge( $defaults, $overrides ) );
1165+
$this->additional_user_ids[] = $user_id;
1166+
1167+
return $user_id;
1168+
}
1169+
1170+
/**
1171+
* Convenience wrapper to fetch a cache-bypassed inactive users count.
1172+
*
1173+
* @param int|null $blog_id Optional blog ID.
1174+
* @return int
1175+
*/
1176+
private function get_fresh_inactive_users_count( $blog_id = null ): int {
1177+
Inactive_Users::flush_cache();
1178+
return Inactive_Users::get_inactive_users_count( $blog_id );
1179+
}
11101180
}

0 commit comments

Comments
 (0)