Page MenuHomePhabricator

Investigate: Update CheckUser for IP Masking
Closed, ResolvedPublic

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

StatusSubtypeAssignedTask
In ProgressNiharika
OpenNone
OpenTchanders
ResolvedCyndymediawiksim
DuplicateNone
ResolvedTchanders
ResolvedTchanders
Resolved AGueyte
ResolvedTchanders
ResolvedTchanders
Resolved AGueyte
Resolved TThoabala
Resolved AGueyte
ResolvedNone
Resolved TThoabala
Resolved TThoabala
Resolvedabi_
Resolved TThoabala
Resolved TThoabala
Invalid TThoabala
ResolvedNone
ResolvedTchanders

Event Timeline

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				}

Moving to blocked/stalled while follow-ups are created by @Cyndymediawiksim and @TThoabala

@Tchanders , @TThoabala , Tasks created under this task : T336477. Please let me know if I missed anything! Thanks!