Investigate: Update CheckUser for IP Masking
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Tchanders
	Mar 10 2023, 9:12 PM

Description

Background

We need to investigate how CheckUser may need updating for IP Masking.

Note that we have already decided that Special:CheckUser and Special:Investigate UI will not change, i.e. temporary accounts won't be treated in a custom way. We're also working on the CheckUser extension a lot as part of IP Masking, so clearly this work shouldn't need investigating.

But we should do our due diligence, and check that nothing else in the extension is affected in a way that needs to be fixed.

From a product perspective, read the documentation and try out the product. Consider whether any changes may need making for temporary account users.

From a technical perspective, check where the search terms from T326759 appear, and consider whether these places appear to need updating.

These instructions are left intentionally vague to avoid biasing the investigation.

Outcome of this investigation

Write up a summary of findings
Raise any questions
File tasks if there is anything obvious to file

Related Objects
Search...

Status	Assigned	Task
		Restricted Task
Resolved	kostajh	T294511 2021 Security Team wikireplicas audit
Declined	None	T284948 Raw IPs of logged-out users disclosed in wiki-replicas
Resolved	Niharika	T324492 Temporary accounts - MVP
Open	None	T326816 [Epic] Update features for temporary accounts
Resolved	Tchanders	T326869 Update TSP-owned products that may be affected by IP Masking
Resolved	Cyndymediawiksim	T331752 Investigate: Update CheckUser for IP Masking
Duplicate	None	T332821 Should temporary users be treated as possible initiators of a check for the CheckUserLog
Resolved	Tchanders	T330816 [Epic] Temporary users should not be assigned to user groups
Resolved	Tchanders	T340457 In UserGroupManager::getUserImplicitGroups, add temporary users to a 'temp' group instead of the 'user' group
Resolved	• AGueyte	T340458 Throw an error from API Userrights explaining that temporary users do not have groups
Resolved	Tchanders	T340462 Special:UserRights should not list autopromote groups for temporary users
Resolved	Tchanders	T340464 Prevent temporary users from being added to autopromote-once groups
Resolved	• AGueyte	T340468 Throw an error from UserGroupManager::addUserToGroup if called on a temporary user
Resolved	• TThoabala	T340470 Investigate: UserGroupManager::addUserToGroup should not called on temporary users from WMF deployed extensions
Resolved	• AGueyte	T340578 Ensure UserGroupManager::addUserToGroup is not called on temporary users from AbuseFilter extension
Resolved	None	T340579 Ensure UserGroupManager::addUserToGroup is not called on temporary users from Flow extension
Resolved	• TThoabala	T340580 Ensure UserGroupManager::addUserToGroup is not called on temporary users from Echo extension
Resolved	• TThoabala	T340583 Ensure UserGroupManager::addUserToGroup is not called on temporary users from LdapAuthentication extension
Resolved	abi_	T340584 Ensure UserGroupManager::addUserToGroup is not called on temporary users from Translate extension
Resolved	• TThoabala	T340587 Ensure UserGroupManager::addUserToGroup is not called on temporary users from MassMessage extension
Resolved	• TThoabala	T340588 Ensure UserGroupManager::addUserToGroup is not called on temporary users from Wikibase extension
Invalid	• TThoabala	T340616 Ensure User::addGroup is not called on temporary users from WikiLambda extension
Resolved	None	T340618 Ensure User::addGroup is not called on temporary users from CentralAuth extension
Resolved	Tchanders	T341600 OutputPage::showPermissionsErrorPage should prompt temporary users to log in for certain actions

Event Timeline

Tchanders created this task.Mar 10 2023, 9:12 PM

JJMC89 added a project: CheckUser.Mar 10 2023, 9:16 PM

Tchanders mentioned this in T326869: Update TSP-owned products that may be affected by IP Masking.Mar 10 2023, 9:17 PM

Dreamy_Jazz added a subtask: T332821: Should temporary users be treated as possible initiators of a check for the CheckUserLog.Mar 22 2023, 8:21 PM

Tchanders edited projects, added Anti-Harassment-Team (AHaT Sprint 27: The Big Yellow Hat); removed Anti-Harassment-Team.Mar 23 2023, 5:28 PM

Cyndymediawiksim claimed this task.Mar 27 2023, 10:04 AM

Cyndymediawiksim moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to In Progress 💪 on the Anti-Harassment-Team (AHaT Sprint 27: The Big Yellow Hat) board.

• 3vtf closed subtask T332821: Should temporary users be treated as possible initiators of a check for the CheckUserLog as Declined.Mar 31 2023, 3:22 PM

• 3vtf moved this task from In Progress 💪 to Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) on the Anti-Harassment-Team (AHaT Sprint 27: The Big Yellow Hat) board.Mar 31 2023, 3:24 PM

Dreamy_Jazz reopened subtask T332821: Should temporary users be treated as possible initiators of a check for the CheckUserLog as Open.Mar 31 2023, 3:33 PM

JJMC89 moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to In Progress 💪 on the Anti-Harassment-Team (AHaT Sprint 27: The Big Yellow Hat) board.Mar 31 2023, 3:36 PM

ARamirez_WMF edited projects, added Anti-Harassment-Team (AHaT Sprint 28: The Mad Hatter Hat); removed Anti-Harassment-Team (AHaT Sprint 27: The Big Yellow Hat).Apr 11 2023, 5:07 PM

ARamirez_WMF moved this task from Ready 🎬 (ONLY IF YOU HAVE NO MORE CODE TO REVIEW) to In Progress 💪 on the Anti-Harassment-Team (AHaT Sprint 28: The Mad Hatter Hat) board.

This is a technical investigation of functions within the CheckUser extension that may need updates with the current implementation of IP Masking(Product/IP Masking - Wikimedia Office).

The use of the function: isRegistered() and $user-<getName() and **Linker::userLink **from the following lines of code:

src/CheckUser/Pagers/CheckUserGetEditsPager.php
177			);
178		} else {
179			if ( !IPUtils::isIPAddress( $user ) && !$user->isRegistered() ) {
180				$templateParams['userLinkClass'] = 'mw-checkuser-nonexistent-user';
181			}


src/CheckUser/Pagers/CheckUserGetUsersPager.php
182		// Load user object
183		$user = new UserIdentityValue( $this->userSets['ids'][$user_text], $user_text );
184		$userNonExistent = !IPUtils::isIPAddress( $user ) && !$user->isRegistered();
185		if ( $userNonExistent ) {
186			$templateParams['userLinkClass'] = 'mw-checkuser-nonexistent-user';


src/Investigate/Pagers/TimelineRowFormatter.php
379
380			$links = Html::rawElement(
381				'span', [], Linker::userLink( $userId, $user->getName() )
382			);
383
384			$links .= Linker::userToolLinksRedContribs(
385				$userId,
386				$user->getName(),
387				$user->getEditCount()
388			);

src/CheckUser/Pagers/CheckUserGetEditsPager.php
180				$templateParams['userLinkClass'] = 'mw-checkuser-nonexistent-user';
181			}
182			$templateParams['userLink'] = Linker::userLink( $user->getId(), $row->cuc_user_text, $row->cuc_user_text );
183			$templateParams['userToolLinks'] = Linker::userToolLinksRedContribs(
184				$user->getId(),

src/CheckUser/Pagers/CheckUserGetUsersPager.php
186			$templateParams['userLinkClass'] = 'mw-checkuser-nonexistent-user';
187		}
188		$templateParams['userLink'] = Linker::userLink( $user->getId(), $user, $user );
189		$templateParams['userToolLinks'] = Linker::userToolLinksRedContribs(
190			$user->getId(),

src/Investigate/Pagers/ComparePager.php
205					$formatted = $this->msg( 'checkuser-investigate-compare-table-cell-unregistered' );
206				} else {
207					$formatted = Linker::userLink( $row->cuc_user ?? 0, $value );
208				}
209				break;

src/Investigate/Pagers/TimelineRowFormatter.php
379
380			$links = Html::rawElement(
381				'span', [], Linker::userLink( $userId, $user->getName() )
382			);

src/Investigate/SpecialInvestigateBlock.php
374				UserNameUtils::RIGOR_NONE
375			);
376			return Linker::userLink( $user->getId(), $userName );
377		}, $this->blockedUsers );

treats temp users the same as anon users. If you want to treat temp users the same as anonymous users, you should check $user->isNamed() instead of $user->isRegistered(). Therefore, the code snippet should be modified to use $user->isNamed() instead of $user->isRegistered().

Alternatively if you want to treat temp users differently from anon users, you can add another check for $user->isTemp().

e.g.

// Load user object
$user = new UserIdentityValue( $this->userSets['ids'][$user_text], $user_text );
$userNonExistent = !IPUtils::isIPAddress( $user ) && !$user->isRegistered();
$userTemp = $user->isTemp(); // Check if user is temp
if ( $userNonExistent ) {
	$templateParams['userLinkClass'] = 'mw-checkuser-nonexistent-user';
}
if ( $userTemp ) {
	// Do something different for temp users
}

The use of $user->getName() from the following lines of code:

src/CheckUser/Pagers/AbstractCheckUserPager.php
396			$flags[] = $this->getBlockFlag( $block );
397		} elseif (
398			$ip == $user->getName() &&
399			ExtensionRegistry::getInstance()->isLoaded( 'GlobalBlocking' ) &&
400			GlobalBlocking::getUserBlock(


// Globally blocked IP
406			$flags[] = '<strong>(' . $this->msg( 'checkuser-gblocked' )->escaped() . ')</strong>';
407		} elseif ( $this->userWasBlocked( $user->getName() ) ) {
408			// Previously blocked
409			$blocklog = $this->getLinkRenderer()->makeKnownLink(

'type' => 'block',
415					// @todo Use TitleFormatter and PageReference to avoid the global state
416					'page' => Title::makeTitle( NS_USER, $user->getName() )->getPrefixedText()
417				]
418			);

is using $user->getName() to compare with the IP address, create a Title object from the user’s name,
$user->isNamed() or $user->isTemp() can be used instead.

mw.util.isIPAddress()is used to check if the user name is an IP address, which will not work for temporary users. If you want to treat temporary users the same as anonymous users, you should use !obj.isNamed() instead. Alternatively, if you want to treat temporary users differently from anonymous users we can add another check for obj.isTemp().

modules/ext.checkUser/checkuser/caMultiLock.js
30			if ( obj.name && obj.name === 'users[]' ) {
31				// Only registered accounts (not IPs) can be locked
32				if ( !mw.util.isIPAddress( obj.value ) ) {
33					names.push( obj.value );
34				}