From c51ba24e340496226e768ab99c87076f11f20ee2 Mon Sep 17 00:00:00 2001 From: Dreamy Jazz Date: Fri, 13 Feb 2026 12:24:33 +0000 Subject: [PATCH] SECURITY: Hide hidden users in Special:SuggestedInvestigations Why: * Special:SuggestedInvestigations shows users in cases to users who are trusted on the wikis ** However, not all users with access to this page may have the 'hideuser' right * We should hide usernames which are blocked with a 'hide user' block from users without the associated right What: * Update SuggestedInvestigationsCasesPager::formatUsersCell to replace usernames the user cannot see with the 'rev-deleted-user' i18n message, similar to how other interfaces handle hidden users in CheckUser ** When hiding the username also remove the user toollinks as the URLs of these toolinks would expose the hidden username * Update SuggestedInvestigationsCasesPager::formatActionsCell to not include usernames the user cannot see in the link to Special:Investigate * Add PHPUnit tests to verify that these changes work as expected Bug: T411366 Change-Id: I3ad0ece14cd0b65468485e388716c06951bc6ab0 --- .../SuggestedInvestigationsCasesPager.php | 135 ++++++++++-------- .../SuggestedInvestigationsPagerFactory.php | 1 + .../SuggestedInvestigationsCasesPagerTest.php | 65 +++++++++ 3 files changed, 143 insertions(+), 58 deletions(-) diff --git a/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPager.php b/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPager.php index d0896822..705300db 100644 --- a/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPager.php +++ b/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPager.php @@ -39,6 +39,7 @@ use MediaWiki\SpecialPage\SpecialPage; use MediaWiki\SpecialPage\SpecialPageFactory; use MediaWiki\Title\Title; use MediaWiki\User\UserEditTracker; +use MediaWiki\User\UserFactory; use MediaWiki\User\UserIdentity; use MediaWiki\User\UserIdentityLookup; use MediaWiki\User\UserIdentityValue; @@ -130,6 +131,7 @@ class SuggestedInvestigationsCasesPager extends CodexTablePager { private readonly CommentFormatter $commentFormatter, private readonly ?CentralAuthEditCounter $centralAuthEditCounter, private readonly LinkBatchFactory $linkBatchFactory, + private readonly UserFactory $userFactory, LinkRenderer $linkRenderer, IContextSource $context, array $signals @@ -279,66 +281,84 @@ class SuggestedInvestigationsCasesPager extends CodexTablePager { $contributionsSpecialPage = $this->useGlobalContribs ? 'GlobalContributions' : 'Contributions'; foreach ( $users as $i => $user ) { - $userLink = $this->getLinkRenderer()->makeUserLink( $user, $this->getContext() ); + $userVisible = $this->getAuthority()->isAllowed( 'hideuser' ) || + !$this->userFactory->newFromUserIdentity( $user )->isHidden(); + if ( $userVisible ) { + $userLink = $this->getLinkRenderer()->makeUserLink( $user, $this->getContext() ); + + // Generate a link to Special:CheckUser with a prefilled 'reason' input field that links back to the + // case that this user is in. + $checkUserPrefilledReason = $this->msg( 'checkuser-suggestedinvestigations-user-check-reason-prefill' ) + ->params( $detailViewLink ) + ->numParams( $this->mCurrentRow->sic_id ) + ->params( $user->getName() ) + ->inContentLanguage() + ->text(); - // Generate a link to Special:CheckUser with a prefilled 'reason' input field that links back to the - // case that this user is in. - $checkUserPrefilledReason = $this->msg( 'checkuser-suggestedinvestigations-user-check-reason-prefill' ) - ->params( $detailViewLink ) - ->numParams( $this->mCurrentRow->sic_id ) - ->params( $user->getName() ) - ->inContentLanguage() - ->text(); + // Generate the link class for the "contribs" tool link + $userContribsLinkClass = 'mw-usertoollinks-contribs'; - // Generate the link class for the "contribs" tool link - $userContribsLinkClass = 'mw-usertoollinks-contribs'; + if ( $this->useGlobalContribs ) { + $editCount = $this->centralAuthEditCounter->getCount( + CentralAuthUser::getInstance( $user ) + ); + } else { + $editCount = $this->userEditTracker->getUserEditCount( $user ); + } + if ( $editCount === 0 ) { + // Use same CSS classes as Linker::userToolLinkArray to get a red link when no contribs + $userContribsLinkClass .= ' mw-usertoollinks-contribs-no-edits'; + } - if ( $this->useGlobalContribs ) { - $editCount = $this->centralAuthEditCounter->getCount( - CentralAuthUser::getInstance( $user ) + // Add link to either Special:Contributions or Special:GlobalContributions + $userToolLinks = []; + $userToolLinks[] = $this->getLinkRenderer()->makeKnownLink( + SpecialPage::getTitleFor( $contributionsSpecialPage, $user->getName() ), + $this->msg( 'contribslink' ) + ->params( $user->getName() ) + ->text(), + [ 'class' => $userContribsLinkClass ] ); - } else { - $editCount = $this->userEditTracker->getUserEditCount( $user ); - } - if ( $editCount === 0 ) { - // Use same CSS classes as Linker::userToolLinkArray to get a red link when no contribs - $userContribsLinkClass .= ' mw-usertoollinks-contribs-no-edits'; - } - // Add link to either Special:Contributions or Special:GlobalContributions - $userToolLinks = []; - $userToolLinks[] = $this->getLinkRenderer()->makeKnownLink( - SpecialPage::getTitleFor( $contributionsSpecialPage, $user->getName() ), - $this->msg( 'contribslink' ) - ->params( $user->getName() ) - ->text(), - [ 'class' => $userContribsLinkClass ] - ); + // Add link to Special:CheckUserLog if the user has been checked before and the + // viewing authority has the 'checkuser-log' right + if ( + in_array( $user->getId(), $this->usersWhoHaveBeenChecked ) && + $this->getAuthority()->isAllowed( 'checkuser-log' ) + ) { + $userToolLinks[] = $this->getLinkRenderer()->makeKnownLink( + SpecialPage::getTitleFor( 'CheckUserLog', $user->getName() ), + $this->msg( 'checkuser-suggestedinvestigations-user-past-checks-link-text' ) + ->params( $user->getName() ) + ->text() + ); + } - // Add link to Special:CheckUserLog if the user has been checked before and the - // viewing authority has the 'checkuser-log' right - if ( - in_array( $user->getId(), $this->usersWhoHaveBeenChecked ) && - $this->getAuthority()->isAllowed( 'checkuser-log' ) - ) { - $userToolLinks[] = $this->getLinkRenderer()->makeKnownLink( - SpecialPage::getTitleFor( 'CheckUserLog', $user->getName() ), - $this->msg( 'checkuser-suggestedinvestigations-user-past-checks-link-text' ) - ->params( $user->getName() ) - ->text() + // Add link to Special:CheckUser if the user has the 'checkuser' right + if ( $this->getAuthority()->isAllowed( 'checkuser' ) ) { + $userToolLinks[] = $this->getLinkRenderer()->makeKnownLink( + SpecialPage::getTitleFor( 'CheckUser', $user->getName() ), + $this->msg( 'checkuser-suggestedinvestigations-user-check-link-text' ) + ->params( $user->getName() ) + ->text(), + [], + [ 'reason' => $checkUserPrefilledReason ] + ); + } + } else { + $userLink = Html::element( + 'span', + [ 'class' => 'history-deleted' ], + $this->msg( 'rev-deleted-user' )->text() ); + $userToolLinks = []; } - // Add link to Special:CheckUser if the user has the 'checkuser' right - if ( $this->getAuthority()->isAllowed( 'checkuser' ) ) { - $userToolLinks[] = $this->getLinkRenderer()->makeKnownLink( - SpecialPage::getTitleFor( 'CheckUser', $user->getName() ), - $this->msg( 'checkuser-suggestedinvestigations-user-check-link-text' ) - ->params( $user->getName() ) - ->text(), - [], - [ 'reason' => $checkUserPrefilledReason ] - ); + $userToolLinksHtml = ''; + if ( $userToolLinks !== [] ) { + $userToolLinksHtml = $this->msg( 'parentheses' ) + ->rawParams( $this->getLanguage()->pipeList( $userToolLinks ) ) + ->escaped(); } $formattedUsers .= Html::rawElement( @@ -348,12 +368,7 @@ class SuggestedInvestigationsCasesPager extends CodexTablePager { : '', ], $this->msg( 'checkuser-suggestedinvestigations-user' ) - ->rawParams( - $userLink, - $this->msg( 'parentheses' ) - ->rawParams( $this->getLanguage()->pipeList( $userToolLinks ) ) - ->escaped() - ) + ->rawParams( $userLink, $userToolLinksHtml ) ->parse() ); } @@ -440,7 +455,11 @@ class SuggestedInvestigationsCasesPager extends CodexTablePager { ] ); /** @var UserIdentity[] $users */ - $users = $this->mCurrentRow->users; + $users = array_filter( + $this->mCurrentRow->users, + fn ( UserIdentity $user ) => $this->getAuthority()->isAllowed( 'hideuser' ) || + !$this->userFactory->newFromUserIdentity( $user )->isHidden() + ); $investigateEnabled = false; $investigateUrl = null; diff --git a/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsPagerFactory.php b/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsPagerFactory.php index ae4f702b..525068f9 100644 --- a/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsPagerFactory.php +++ b/src/SuggestedInvestigations/Pagers/SuggestedInvestigationsPagerFactory.php @@ -96,6 +96,7 @@ class SuggestedInvestigationsPagerFactory { $this->commentFormatter, $this->centralAuthEditCounter, $this->linkBatchFactory, + $this->userFactory, $this->linkRenderer, $context, $signals diff --git a/tests/phpunit/integration/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPagerTest.php b/tests/phpunit/integration/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPagerTest.php index ff13faa0..d491e74a 100644 --- a/tests/phpunit/integration/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPagerTest.php +++ b/tests/phpunit/integration/SuggestedInvestigations/Pagers/SuggestedInvestigationsCasesPagerTest.php @@ -374,6 +374,7 @@ class SuggestedInvestigationsCasesPagerTest extends MediaWikiIntegrationTestCase $this->addCaseWithTwoUsers(); $context = RequestContext::getMain(); $context->setTitle( Title::newFromText( 'Special:SuggestedInvestigations' ) ); + $context->setAuthority( $this->mockRegisteredUltimateAuthority() ); // Mock the global edit counts for our test users so that the first test user has no edits // and all other users have one edit @@ -558,6 +559,70 @@ class SuggestedInvestigationsCasesPagerTest extends MediaWikiIntegrationTestCase ); } + public function testOutputWhenUsersHidden() { + $this->overrideConfigValues( [ + 'CheckUserSuggestedInvestigationsUseGlobalContributionsLink' => false, + MainConfigNames::LanguageCode => 'qqx', + ] ); + ConvertibleTimestamp::setFakeTime( '20250403020100' ); + + // Get a case with one of the users blocked with a 'hideuser' block + $this->addCaseWithTwoUsers(); + $this->getServiceContainer()->getBlockUserFactory() + ->newBlockUser( + self::$testUser1, + $this->mockRegisteredUltimateAuthority(), + 'indefinite', + 'Test reason', + [ 'isHideUser' => true ] + ) + ->placeBlock(); + + // Load the special page with a user who cannot see hidden users + $context = RequestContext::getMain(); + $context->setTitle( Title::newFromText( 'Special:SuggestedInvestigations' ) ); + $context->setLanguage( 'qqx' ); + $context->setAuthority( $this->mockRegisteredAuthorityWithoutPermissions( [ 'hideuser' ] ) ); + + $pager = $this->getPager( $context ); + + $html = $pager->getFullOutput()->getContentHolder()->getAsHtmlString(); + + $this->assertStringContainsString( + '(rev-deleted-user)', + $html, + 'First test username should be replaced with the rev-deleted-user message' + ); + + $this->assertStringNotContainsString( + '?title=Special:CheckUser/' . str_replace( ' ', '_', self::$testUser1->getName() ) . + '&reason=%28checkuser-suggestedinvestigations-user-check-reason-prefill', + $html, + 'Should not contain link to Special:CheckUser for the first user' + ); + $this->assertStringContainsString( + '?title=Special:CheckUser/' . str_replace( ' ', '_', self::$testUser2->getName() ) . + '&reason=%28checkuser-suggestedinvestigations-user-check-reason-prefill', + $html, + 'Should contain link to Special:CheckUser for the second user' + ); + + $name2 = urlencode( self::$testUser2->getName() ); + $this->assertStringContainsString( + '?title=Special:Investigate&targets=' . $name2 . + '&reason=%28checkuser-suggestedinvestigations-user-investigate-reason-prefill', + $html, + 'Should contain link to Special:Investigate in the case row with only the second user' + ); + + $this->assertStringNotContainsString( + self::$testUser1->getName(), + $html, + 'As the first test user is not visible by the viewing authority, ' . + 'their name should be not visible anywhere on the page' + ); + } + public function testInvestigateDisabledWhenTooManyUsers() { $caseId = $this->addCaseWithManyUsers(); -- 2.34.1