Hook on privateEvent and logEvent insert hooks like CuChangesInsert

Hook on to CheckUserInsertPrivateEventRow and CheckUserInsertLogEventRow
to override the IP, XFF and User-Agent string when the user is the
abuse filter user for log events.

These two hooks are being added as log entries are being removed from
cu_changes and added into two new tables. Because the columns and their
names are different for these tables, reusing the same hook won't work
for callers that rely on setting values for a specific column name.

Edits and log entries performed by the abuse filter user need to be
marked as being by the software (and not using the IP, XFF and
User-Agent provided in the main request).

These hooks will not be run until the appropriate config is set to
write to the two new tables. Until that point using the one currently
defined hook will work for all actions.

Bug: T324907
Bug: T44345
Depends-On: I7c7754323ade9a8d96273c1742f30b1b5fbe5828
Follow-Up: Idd77545af94f9f9930d9ff38ab6423a72e680df9
Change-Id: Id78417e9d95220946f110afbe1430df5b3bb4f4f
This commit is contained in:
Dreamy Jazz 2023-01-08 00:32:47 +00:00
parent d3d0910bee
commit 8e4a1237f1
2 changed files with 90 additions and 16 deletions

View file

@ -3,11 +3,17 @@
namespace MediaWiki\Extension\AbuseFilter\Hooks\Handlers;
use MediaWiki\CheckUser\Hook\CheckUserInsertChangesRow;
use MediaWiki\CheckUser\Hook\CheckUserInsertLogEventRow;
use MediaWiki\CheckUser\Hook\CheckUserInsertPrivateEventRow;
use MediaWiki\Extension\AbuseFilter\FilterUser;
use MediaWiki\User\UserIdentity;
use RecentChange;
class CheckUserHandler implements CheckUserInsertChangesRow {
class CheckUserHandler implements
CheckUserInsertChangesRow,
CheckUserInsertPrivateEventRow,
CheckUserInsertLogEventRow
{
/** @var FilterUser */
private $filterUser;
@ -20,13 +26,13 @@ class CheckUserHandler implements CheckUserInsertChangesRow {
}
/**
* Any actions by the filter user should always be marked as by the software
* Any edits by the filter user should always be marked as by the software
* using IP 127.0.0.1, no XFF and no UA.
*
* @inheritDoc
*/
public function onCheckUserInsertChangesRow(
string &$ip, &$xff, array &$row, UserIdentity $user, ?RecentChange $rc = null
string &$ip, &$xff, array &$row, UserIdentity $user, ?RecentChange $rc
) {
if (
$user->isRegistered() &&
@ -37,4 +43,42 @@ class CheckUserHandler implements CheckUserInsertChangesRow {
$row['cuc_agent'] = '';
}
}
/**
* Any log actions by the filter user should always be marked as by the software
* using IP 127.0.0.1, no XFF and no UA.
*
* @inheritDoc
*/
public function onCheckUserInsertLogEventRow(
string &$ip, &$xff, array &$row, UserIdentity $user, int $id, ?RecentChange $rc
) {
if (
$user->isRegistered() &&
$this->filterUser->getUserIdentity()->getId() == $user->getId()
) {
$ip = '127.0.0.1';
$xff = false;
$row['cule_agent'] = '';
}
}
/**
* Any log actions by the filter user should always be marked as by the software
* using IP 127.0.0.1, no XFF and no UA.
*
* @inheritDoc
*/
public function onCheckUserInsertPrivateEventRow(
string &$ip, &$xff, array &$row, UserIdentity $user, ?RecentChange $rc
) {
if (
$user->isRegistered() &&
$this->filterUser->getUserIdentity()->getId() == $user->getId()
) {
$ip = '127.0.0.1';
$xff = false;
$row['cupe_agent'] = '';
}
}
}

View file

@ -20,16 +20,7 @@ class CheckUserHandlerTest extends MediaWikiUnitTestCase {
return new CheckUserHandler( $filterUser );
}
/**
* @covers ::onCheckUserInsertChangesRow
* @dataProvider provideOnCheckUserInsertChangesRow
*/
public function testOnCheckUserInsertChangesRow( $user, $shouldChange ) {
$checkUserHandler = $this->getCheckUserHandler();
$ip = '1.2.3.4';
$xff = '1.2.3.5';
$row = [];
$checkUserHandler->onCheckUserInsertChangesRow( $ip, $xff, $row, $user );
private function commonInsertHookAssertions( $shouldChange, $agentField, $ip, $xff, $row ) {
if ( $shouldChange ) {
$this->assertSame(
'127.0.0.1',
@ -42,7 +33,7 @@ class CheckUserHandlerTest extends MediaWikiUnitTestCase {
);
$this->assertSame(
'',
$row['cuc_agent'],
$row[$agentField],
'User agent should have been blanked because the abuse filter is making the action.'
);
} else {
@ -57,14 +48,53 @@ class CheckUserHandlerTest extends MediaWikiUnitTestCase {
'XFF should have not been modified by AbuseFilter handling the checkuser insert row hook.'
);
$this->assertArrayNotHasKey(
'cuc_agent',
$agentField,
$row,
'User agent should have not been modified by AbuseFilter handling the checkuser insert row hook.'
);
}
}
public function provideOnCheckUserInsertChangesRow() {
/**
* @covers ::onCheckUserInsertChangesRow
* @dataProvider provideDataForCheckUserInsertHooks
*/
public function testOnCheckUserInsertChangesRow( $user, $shouldChange ) {
$checkUserHandler = $this->getCheckUserHandler();
$ip = '1.2.3.4';
$xff = '1.2.3.5';
$row = [];
$checkUserHandler->onCheckUserInsertChangesRow( $ip, $xff, $row, $user, null );
$this->commonInsertHookAssertions( $shouldChange, 'cuc_agent', $ip, $xff, $row );
}
/**
* @covers ::onCheckUserInsertPrivateEventRow
* @dataProvider provideDataForCheckUserInsertHooks
*/
public function testOnCheckUserInsertPrivateEventRow( $user, $shouldChange ) {
$checkUserHandler = $this->getCheckUserHandler();
$ip = '1.2.3.4';
$xff = '1.2.3.5';
$row = [];
$checkUserHandler->onCheckUserInsertPrivateEventRow( $ip, $xff, $row, $user, null );
$this->commonInsertHookAssertions( $shouldChange, 'cupe_agent', $ip, $xff, $row );
}
/**
* @covers ::onCheckUserInsertLogEventRow
* @dataProvider provideDataForCheckUserInsertHooks
*/
public function testOnCheckUserInsertLogEventRow( $user, $shouldChange ) {
$checkUserHandler = $this->getCheckUserHandler();
$ip = '1.2.3.4';
$xff = '1.2.3.5';
$row = [];
$checkUserHandler->onCheckUserInsertLogEventRow( $ip, $xff, $row, $user, 1, null );
$this->commonInsertHookAssertions( $shouldChange, 'cule_agent', $ip, $xff, $row );
}
public function provideDataForCheckUserInsertHooks() {
return [
'Anonymous user' => [ UserIdentityValue::newAnonymous( '127.0.0.1' ), false ],
'Registered user' => [ new UserIdentityValue( 2, 'Test' ), false ],