wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-09-23 18:28:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Filter out actions to execute before actually executing them

Browse source 
This way we don't have special cases in executeFilterActions, and instead, we execute
all actions in the same place. In turn, this is going to ease the
transition to a new consequences system: next step is refactoring this
code into a service with proper DI etc.

Bug: T204447
Change-Id: I8134ecc41fbecdbed99faf406e9e3ca91b6123b9
This commit is contained in:
Daimona Eaytoy 2018-09-26 13:38:51 +02:00
parent e7813fbafb
commit ef9e828fbe
3 changed files with 386 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

171
includes/AbuseFilterRunner.php
View file

@ -449,22 +449,61 @@ class AbuseFilterRunner {
* the errors and warnings to be shown to the user to explain the actions.
*/
protected function executeFilterActions( array $filters ) : Status {
global $wgAbuseFilterLocallyDisabledGlobalActions,
$wgAbuseFilterBlockDuration, $wgAbuseFilterAnonBlockDuration;
$actionsByFilter = AbuseFilter::getConsequencesForFilters( $filters );
$actionsToTake = $this->getFilteredConsequences( $actionsByFilter );
$actionsTaken = array_fill_keys( $filters, [] );
$messages = [];
// Accumulator to track max block to issue
$maxExpiry = -1;
foreach ( $actionsByFilter as $filter => $actions ) {
foreach ( $actionsToTake as $filter => $actions ) {
[ $filterID, $isGlobalFilter ] = AbuseFilter::splitGlobalName( $filter );
$filterObj = $this->filterLookup->getFilter( $filterID, $isGlobalFilter );
$filterPublicComments = $filterObj->getName();
foreach ( $actions as $action => $info ) {
$newMsg = $this->takeConsequenceAction(
$action,
$info,
$filterPublicComments,
$filter
);
$isGlobalFilter = $filterObj->isGlobal();
if ( $newMsg !== null ) {
$messages[] = $newMsg;
}
// Don't add it if throttle limit has been reached, or if the warning has already been shown
if ( ( $action !== 'throttle' || !$info['throttled'] ) &&
( $action !== 'warn' || $info['shouldWarn'] ) ) {
$actionsTaken[$filter][] = $action;
}
}
}
return $this->buildStatus( $actionsTaken, $messages );
}
/**
* Idempotent and pure method that, given a raw list of consequences, determines which ones
* should be actually executed. Normalizations done here:
* - Only keep the longest block from all filters
* - For global filters, remove locally disabled actions
* - For every filter with "throttle" enabled, remove other actions if the throttle counter hasn't been reached
* - For every filter with "warn" enabled, remove other actions if the warning hasn't been shown
* - For every filter, remove "disallow" if a blocking action will be executed
* - Rewrite parameters of "block", "warn" and "throttle"
*
* @param array[] $actionsByFilter
* @return array[]
* @internal Temporary method
*/
public function getFilteredConsequences( array $actionsByFilter ) : array {
global $wgAbuseFilterLocallyDisabledGlobalActions,
$wgAbuseFilterBlockDuration, $wgAbuseFilterAnonBlockDuration;
// Keep track of the longest block
$maxBlock = [ 'id' => null, 'expiry' => -1, 'blocktalk' => null ];
foreach ( $actionsByFilter as $filter => &$actions ) {
$isGlobalFilter = AbuseFilter::splitGlobalName( $filter )[1];
if ( $isGlobalFilter ) {
$actions = array_diff_key( $actions, array_filter( $wgAbuseFilterLocallyDisabledGlobalActions ) );
@ -485,34 +524,32 @@ class AbuseFilterRunner {
foreach ( $parameters as $throttleType ) {
$hitThrottle = $this->isThrottled( $throttleId, $throttleType, $rateCount, $isGlobalFilter )
|| $hitThrottle;
$this->setThrottled( $throttleId, $throttleType, $ratePeriod, $isGlobalFilter );
}
unset( $actions['throttle'] );
$newParams = [
'throttled' => $hitThrottle,
'id' => $throttleId,
'types' => $parameters,
'period' => $ratePeriod,
'global' => $isGlobalFilter
];
$actions['throttle'] = $newParams;
if ( !$hitThrottle ) {
$actionsTaken[$filter][] = 'throttle';
$actions = [ 'throttle' => $actions['throttle'] ];
continue;
}
}
if ( isset( $actions['warn'] ) ) {
$parameters = $actions['warn'];
if ( $this->shouldBeWarned( $filter ) ) {
$this->setWarn( $filter, true );
$msg = $parameters[0] ?? 'abusefilter-warning';
$messages[] = [ $msg, $filterPublicComments, $filter ];
$actionsTaken[$filter][] = 'warn';
// Don't do anything else.
$shouldWarn = $this->shouldBeWarned( $filter );
$msg = $parameters[0] ?? 'abusefilter-warning';
$actions['warn'] = [ 'msg' => $msg, 'shouldWarn' => $shouldWarn ];
if ( $shouldWarn ) {
$actions = [ 'warn' => $actions['warn'] ];
continue;
} else {
$this->setWarn( $filter, false );
}
unset( $actions['warn'] );
}
// Don't show the disallow message if a blocking action is executed
@ -522,7 +559,6 @@ class AbuseFilterRunner {
unset( $actions['disallow'] );
}
// Find out the max expiry to issue the longest triggered block.
if ( isset( $actions['block'] ) ) {
$parameters = $actions['block'];
@ -544,53 +580,30 @@ class AbuseFilterRunner {
}
}
$currentExpiry = SpecialBlock::parseExpiryInput( $expiry );
if ( $maxExpiry === -1 || $currentExpiry > SpecialBlock::parseExpiryInput( $maxExpiry ) ) {
if (
$maxBlock['expiry'] === -1 ||
SpecialBlock::parseExpiryInput( $expiry ) > SpecialBlock::parseExpiryInput( $maxBlock['expiry'] )
) {
// Save the parameters to issue the block with
$maxExpiry = $expiry;
$blockValues = [
$filterPublicComments,
$filter,
is_array( $parameters ) && in_array( 'blocktalk', $parameters )
$maxBlock = [
'id' => $filter,
'expiry' => $expiry,
'blocktalk' => is_array( $parameters ) && in_array( 'blocktalk', $parameters )
];
}
// We'll re-add it later
unset( $actions['block'] );
}
}
unset( $actions );
// Do the rest of the actions
foreach ( $actions as $action => $info ) {
$newMsg = $this->takeConsequenceAction(
$action,
$info,
$filterPublicComments,
$filter
);
if ( $newMsg !== null ) {
$messages[] = $newMsg;
}
$actionsTaken[$filter][] = $action;
}
if ( $maxBlock['id'] !== null ) {
$id = $maxBlock['id'];
unset( $maxBlock['id'] );
$actionsByFilter[ $id ]['block'] = $maxBlock;
}
// Since every filter has been analysed, we now know what the
// longest block duration is, so we can issue the block if
// maxExpiry has been changed.
if ( $maxExpiry !== -1 ) {
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
$this->doBlock( $blockValues[0], $blockValues[1], $maxExpiry, $blockValues[2] );
$message = [
'abusefilter-blocked-display',
$blockValues[0],
$blockValues[1]
];
// Manually add the message. If we're here, there is one.
$messages[] = $message;
// @phan-suppress-next-line PhanTypeMismatchDimAssignment
$actionsTaken[$blockValues[1]][] = 'block';
}
return $this->buildStatus( $actionsTaken, $messages );
return $actionsByFilter;
}
/**
@ -762,6 +775,29 @@ class AbuseFilterRunner {
$message = null;
switch ( $action ) {
case 'throttle':
foreach ( $parameters['types'] as $type ) {
$this->setThrottled(
$parameters['id'],
$type,
$parameters['period'],
$parameters['global']
);
}
break;
case 'warn':
$this->setWarn( $ruleNumber, $parameters[ 'shouldWarn' ] );
if ( !$parameters['shouldWarn'] ) {
break;
}
if ( isset( $parameters['msg'] ) && strlen( $parameters['msg'] ) ) {
$msg = $parameters['msg'];
} else {
$msg = 'abusefilter-warning';
}
$message = [ $msg, $ruleDescription, $ruleNumber ];
break;
case 'disallow':
$msg = $parameters[0] ?? 'abusefilter-disallowed';
$message = [ $msg, $ruleDescription, $ruleNumber ];
@ -853,7 +889,12 @@ class AbuseFilterRunner {
break;
case 'block':
// Do nothing, handled at the end of executeFilterActions. Here for completeness.
$this->doBlock( $ruleDescription, $ruleNumber, $parameters['expiry'], $parameters['blocktalk'] );
$message = [
'abusefilter-blocked-display',
$ruleDescription,
$ruleNumber
];
break;
case 'tag':

111
tests/phpunit/AbuseFilterConsequencesTest.php
View file

@ -302,7 +302,19 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
],
'blockautopromote' => []
]
]
],
23 => [
'af_pattern' => '1 === 1',
'af_public_comments' => 'Catch-all for warning + disallow',
'actions' => [
'warn' => [
'abusefilter-my-warning'
],
'disallow' => [
'abusefilter-my-disallow'
]
]
],
];
// phpcs:enable Generic.Files.LineLength
@ -1217,7 +1229,104 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
]
],
[ 'disallow' => [ 11 ] ]
]
];
}
/**
* Like self::testFilterConsequences but for warn, which deserves a special treatment.
* Data provider passes parameters for a single action, which we repeat twice
*
* @param int[] $createIds IDs of the filters to create
* @param array[] $actionParams Details of the action we need to execute to trigger filters
* @param int[] $warnIDs IDs of the filters which will warn us
* @param array $consequences The consequences we're expecting
* @dataProvider provideWarnFilters
*/
public function testWarn( $createIds, $actionParams, $warnIDs, $consequences ) {
$this->createFilters( $createIds );
$params = [ $actionParams, $actionParams ];
list( $warnedStatus, $finalStatus ) = $this->doActions( $params );
list( $expectedWarn, $actualWarn ) = $this->checkConsequences(
$warnedStatus,
$actionParams,
[ 'warn' => $warnIDs ]
);
$this->assertSame(
$expectedWarn,
$actualWarn,
'The error messages for the first action do not match.'
);
list( $expectedFinal, $actualFinal ) = $this->checkConsequences(
$finalStatus,
$actionParams,
$consequences
);
$this->assertSame(
$expectedFinal,
$actualFinal,
'The error messages for the second action do not match.'
);
}
/**
* Data provider for testWarn. For every test case, we pass
* - an array with the IDs of the filters to be created (listed in self::$filters),
* - an array with action parameters, like in self::provideFilters. This will be executed twice.
* - an array of IDs of the filter which should give a warning
* - an array of expected consequences for the last action (i.e. after throttling) of the form
* [ 'consequence name' => [ IDs of the filter to take its parameters from ] ]
* Such IDs may be more than one if we have a warning that is shown twice.
*
* @return array
*/
public function provideWarnFilters() {
return [
'Basic test for warning and then tag' => [
[ 1 ],
[
'action' => 'edit',
'target' => 'Foo',
'oldText' => 'Neutral text',
'newText' => 'First foo version',
'summary' => ''
],
[ 1 ],
[ 'tag' => [ 1 ] ],
],
'Basic test for warning on "move"' => [
[ 23 ],
[
'action' => 'move',
'target' => 'Test warn',
'newTitle' => 'Another warn test'
],
[ 23 ],
[ 'disallow' => [ 23 ] ]
],
'Basic test for warning on "delete"' => [
[ 23 ],
[
'action' => 'delete',
'target' => 'Warned'
],
[ 23 ],
[ 'disallow' => [ 23 ] ]
],
'Basic test for warning on "createaccount"' => [
[ 23 ],
[
'action' => 'createaccount',
'target' => 'User:AnotherWarnedUser',
'username' => 'AnotherWarnedUser'
],
[ 23 ],
[ 'disallow' => [ 23 ] ]
]
];
}

170
tests/phpunit/AbuseFilterDBTest.php
View file

@ -314,4 +314,174 @@ class AbuseFilterDBTest extends MediaWikiTestCase {
yield 'All suppressed, not privileged' => [ $allSupp, false, false ];
yield 'All suppressed, privileged' => [ $allSupp, true, true ];
}
/**
* @param array $rawConsequences A raw, unfiltered list of consequences
* @param array $expected The expected, filtered list
* @param Title $title
* @covers AbuseFilterRunner::getFilteredConsequences
* @dataProvider provideConsequences
*/
public function testGetFilteredConsequences( $rawConsequences, $expected, Title $title ) {
$this->setMwGlobals( [
'wgAbuseFilterLocallyDisabledGlobalActions' => [
'flag' => false,
'throttle' => false,
'warn' => false,
'disallow' => false,
'blockautopromote' => true,
'block' => true,
'rangeblock' => true,
'degroup' => true,
'tag' => false
],
'wgMainCacheType' => 'hash'
] );
$user = $this->getTestUser()->getUser();
$vars = AbuseFilterVariableHolder::newFromArray( [ 'action' => 'edit' ] );
$runner = new AbuseFilterRunner( $user, $title, $vars, 'default' );
$actual = $runner->getFilteredConsequences( $rawConsequences );
$this->assertEquals( $expected, $actual );
}
/**
* Data provider for testGetFilteredConsequences
* @todo Split these
* @return array
*/
public function provideConsequences() {
$pageName = __METHOD__;
$title = $this->createMock( Title::class );
$title->method( 'getPrefixedText' )->willReturn( $pageName );
return [
'warn and throttle exclude other actions' => [
[
2 => [
'warn' => [
'abusefilter-warning'
],
'tag' => [
'some tag'
]
],
13 => [
'throttle' => [
'13',
'14,15',
'user'
],
'disallow' => []
],
168 => [
'degroup' => []
]
],
[
2 => [
'warn' => [
'msg' => 'abusefilter-warning',
'shouldWarn' => true
]
],
13 => [
'throttle' => [
'throttled' => false,
'id' => '13',
'types' => [ 'user' ],
'period' => 15,
'global' => false
]
],
168 => [
'degroup' => []
]
],
$title
],
'warn excludes other actions, block excludes disallow' => [
[
3 => [
'tag' => [
'some tag'
]
],
'global-2' => [
'warn' => [
'abusefilter-beautiful-warning'
],
'degroup' => []
],
4 => [
'disallow' => [],
'block' => [
'blocktalk',
'15 minutes',
'indefinite'
]
]
],
[
3 => [
'tag' => [
'some tag'
]
],
'global-2' => [
'warn' => [
'msg' => 'abusefilter-beautiful-warning',
'shouldWarn' => true
]
],
4 => [
'block' => [
'expiry' => 'indefinite',
'blocktalk' => true
]
]
],
$title
],
'some global actions are disabled locally, the longest block is chosen' => [
[
'global-1' => [
'blockautopromote' => [],
'block' => [
'blocktalk',
'indefinite',
'indefinite'
]
],
1 => [
'block' => [
'blocktalk',
'4 hours',
'4 hours'
]
],
2 => [
'degroup' => [],
'block' => [
'blocktalk',
'infinity',
'never'
]
]
],
[
'global-1' => [],
1 => [],
2 => [
'degroup' => [],
'block' => [
'expiry' => 'never',
'blocktalk' => true
]
]
],
$title
],
];
}
}