Skip to content

Commit 06fa632

Browse files
brunobastobrunobasto
committed
Make sure admins can only unblock users that are members of the current site
Otherwise an admin would be able to unblock users who aren't even members of the current site
1 parent 6caa8fb commit 06fa632

2 files changed

Lines changed: 172 additions & 1 deletion

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -611,10 +611,17 @@ public static function last_seen_unblock_action() {
611611
$error = __( 'You do not have permission to unblock this user.', 'wpvip' );
612612
}
613613

614+
// Additional multisite security check: ensure user belongs to current site
615+
if ( ! $error && is_multisite() && ! is_user_member_of_blog( $user_id, get_current_blog_id() ) ) {
616+
$error = __( 'You can only unblock users who are members of this site.', 'wpvip' );
617+
}
618+
614619
$ignore_inactivity_check_until = strtotime( '+2 days' );
615620
if ( ! $error && ! self::ignore_inactivity_check_for_user( $user_id, $ignore_inactivity_check_until ) ) {
616621
$error = __( 'Unable to unblock user.', 'wpvip' );
617-
} else {
622+
}
623+
624+
if ( ! $error ) {
618625
// Track successful user unblock
619626
$user = get_userdata( $user_id );
620627
$user_role = $user && ! empty( $user->roles ) ? $user->roles[0] : '';

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,4 +749,168 @@ public function test_only_inactive_and_release_users_are_returned_blocked_filter
749749
wp_delete_user( $u3 );
750750
wp_delete_user( $u4 );
751751
}
752+
753+
/**
754+
* Test multisite security: admin can only unblock users who are members of current site
755+
*/
756+
public function test_multisite_unblock_when_user_not_member_of_site() {
757+
if ( ! is_multisite() ) {
758+
$this->markTestSkipped( 'This test requires multisite to be enabled' );
759+
}
760+
761+
// Create a user who will be an admin on the current site
762+
$network_user_id = $this->factory->user->create([
763+
'role' => 'administrator',
764+
'user_registered' => gmdate( 'Y-m-d H:i:s', strtotime( '-100 days' ) ),
765+
]);
766+
767+
// Make the user inactive
768+
update_user_meta( $network_user_id, Inactive_Users::LAST_SEEN_META_KEY, strtotime( '-91 days' ) );
769+
delete_user_meta( $network_user_id, Inactive_Users::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY );
770+
771+
// Create a second site
772+
$site_2_id = $this->factory->blog->create();
773+
774+
// Remove the user from the current site and add them to site 2
775+
remove_user_from_blog( $network_user_id, get_current_blog_id() );
776+
// phpcs:ignore WordPressVIPMinimum.Functions.RestrictedFunctions.switch_to_blog_switch_to_blog
777+
switch_to_blog( $site_2_id );
778+
add_user_to_blog( $site_2_id, $network_user_id, 'administrator' );
779+
restore_current_blog();
780+
781+
// Create an admin user on the current site
782+
$current_site_admin = $this->factory->user->create([
783+
'role' => 'administrator',
784+
]);
785+
786+
// Set the current user to the admin of the current site
787+
wp_set_current_user( $current_site_admin );
788+
789+
// Verify the network user is not a member of the current site
790+
$this->assertFalse( is_user_member_of_blog( $network_user_id, get_current_blog_id() ) );
791+
792+
// Simulate the unblock action request
793+
$_GET['action'] = 'reset_last_seen';
794+
$_GET['user_id'] = $network_user_id;
795+
$_GET['reset_last_seen_nonce'] = wp_create_nonce( 'reset_last_seen_action' );
796+
797+
// Call the unblock action method
798+
Inactive_Users::last_seen_unblock_action();
799+
800+
// Verify that the user is still considered inactive, since the user is not a member of the current site
801+
$this->assertTrue( Inactive_Users::is_considered_inactive( $network_user_id ) );
802+
803+
// Clean up
804+
wp_delete_user( $network_user_id );
805+
wp_delete_user( $current_site_admin );
806+
unset( $_GET['action'], $_GET['user_id'], $_GET['reset_last_seen_nonce'] );
807+
}
808+
809+
/**
810+
* Test multisite security: admin can unblock users who ARE members of current site
811+
*/
812+
public function test_multisite_unblock_when_user_is_member_of_site() {
813+
if ( ! is_multisite() ) {
814+
$this->markTestSkipped( 'This test requires multisite to be enabled' );
815+
}
816+
817+
// Create a user who IS a member of the current site
818+
$site_user_id = $this->factory->user->create([
819+
'role' => 'administrator',
820+
'user_registered' => gmdate( 'Y-m-d H:i:s', strtotime( '-100 days' ) ),
821+
]);
822+
823+
// Make the user inactive
824+
update_user_meta( $site_user_id, Inactive_Users::LAST_SEEN_META_KEY, strtotime( '-91 days' ) );
825+
delete_user_meta( $site_user_id, Inactive_Users::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY );
826+
827+
// Set the current user to be a super admin (who can edit any user)
828+
$super_admin_id = $this->factory->user->create([
829+
'role' => 'administrator',
830+
]);
831+
grant_super_admin( $super_admin_id );
832+
wp_set_current_user( $super_admin_id );
833+
834+
// Verify the user IS a member of the current site
835+
$this->assertTrue( is_user_member_of_blog( $site_user_id, get_current_blog_id() ) );
836+
837+
// Verify the user is initially inactive
838+
$this->assertTrue( Inactive_Users::is_considered_inactive( $site_user_id ) );
839+
840+
// Verify the current user has edit_user capability for this user
841+
$this->assertTrue( current_user_can( 'edit_user', $site_user_id ), 'Super admin should have edit_user capability for any user' );
842+
843+
// Simulate the unblock action request
844+
$_GET['action'] = 'reset_last_seen';
845+
$_GET['user_id'] = $site_user_id;
846+
$_GET['reset_last_seen_nonce'] = wp_create_nonce( 'reset_last_seen_action' );
847+
848+
// Mock wp_safe_redirect to prevent actual redirect during test
849+
add_filter( 'wp_redirect', function () {
850+
// Just return false to prevent actual redirect
851+
return false;
852+
});
853+
854+
// Call the unblock action method
855+
Inactive_Users::last_seen_unblock_action();
856+
857+
// Verify that the user was successfully unblocked (should no longer be considered inactive)
858+
$this->assertFalse( Inactive_Users::is_considered_inactive( $site_user_id ) );
859+
860+
// Clean up
861+
wp_delete_user( $site_user_id );
862+
wp_delete_user( $super_admin_id );
863+
unset( $_GET['action'], $_GET['user_id'], $_GET['reset_last_seen_nonce'] );
864+
}
865+
866+
/**
867+
* Test that single-site unblock action
868+
*/
869+
public function test_single_site_unblock_action() {
870+
if ( is_multisite() ) {
871+
$this->markTestSkipped( 'This test is for single-site installations only' );
872+
}
873+
874+
// Create a user
875+
$user_id = $this->factory->user->create([
876+
'role' => 'administrator',
877+
'user_registered' => gmdate( 'Y-m-d H:i:s', strtotime( '-100 days' ) ),
878+
]);
879+
880+
// Make the user inactive
881+
update_user_meta( $user_id, Inactive_Users::LAST_SEEN_META_KEY, strtotime( '-91 days' ) );
882+
delete_user_meta( $user_id, Inactive_Users::LAST_SEEN_IGNORE_INACTIVITY_CHECK_UNTIL_META_KEY );
883+
884+
// Create an admin user
885+
$admin_user_id = $this->factory->user->create([
886+
'role' => 'administrator',
887+
]);
888+
889+
// Set the current user to the admin
890+
wp_set_current_user( $admin_user_id );
891+
892+
// Verify the user is initially inactive
893+
$this->assertTrue( Inactive_Users::is_considered_inactive( $user_id ) );
894+
895+
// Simulate the unblock action request
896+
$_GET['action'] = 'reset_last_seen';
897+
$_GET['user_id'] = $user_id;
898+
$_GET['reset_last_seen_nonce'] = wp_create_nonce( 'reset_last_seen_action' );
899+
900+
// Mock wp_safe_redirect to prevent actual redirect during test
901+
add_filter( 'wp_redirect', function () {
902+
return false;
903+
});
904+
905+
// Call the unblock action method
906+
Inactive_Users::last_seen_unblock_action();
907+
908+
// Verify that the user was successfully unblocked (should no longer be considered inactive)
909+
$this->assertFalse( Inactive_Users::is_considered_inactive( $user_id ) );
910+
911+
// Clean up
912+
wp_delete_user( $user_id );
913+
wp_delete_user( $admin_user_id );
914+
unset( $_GET['action'], $_GET['user_id'], $_GET['reset_last_seen_nonce'] );
915+
}
752916
}

0 commit comments

Comments
 (0)