Replace $wgAbuseFilterRestrictions with more specific variables

So that sysadmins can further customize the extension. It was also wrong
to use the same variable for many different things.

Note that there's no associated patch in wmf-config because we use the
defaults. However, before merging this patch, please recheck that
AbuseFilterRestrictions and AbuseFilterDisallowGlobalLocalBlocks aren't
used there (https://codesearch.wmflabs.org/operations/?q=AbuseFilterDisallowGlobalLocalBlocks%7CAbuseFilterRestrictions&i=nope&files=&repos=)

Bug: T175221
Change-Id: I7581b3ee6d9d11a6cf1599b8ff874e8c3d54adf4

extension.json

@@ -347,7 +347,7 @@
 			},
 			"description": "See description for AbuseFilterEmergencyDisableThreshold"
 		},
-		"AbuseFilterRestrictions": {
+		"AbuseFilterActionRestrictions": {
 			"value": {
 				"throttle": false,
 				"warn": false,
@@ -376,9 +376,18 @@
 			"value": false,
 			"description": "Set this variable to true for the wiki where global AbuseFilters are stored in"
 		},
-		"AbuseFilterDisallowGlobalLocalBlocks": {
-			"value": false,
-			"description": "Disallow centralised filters from taking actions that locally block, remove from groups, or revoke permissions"
+		"AbuseFilterLocallyDisabledGlobalActions": {
+			"value": {
+				"throttle": false,
+				"warn": false,
+				"disallow": false,
+				"blockautopromote": false,
+				"block": false,
+				"rangeblock": false,
+				"degroup": false,
+				"tag": false
+			},
+			"description":  "An array of actions which, if set on a global filter, will not be used for local hits of such filter (not in the central wiki). The array is [ action => will be disabled? ]."
 		},
 		"AbuseFilterBlockDuration": {
 			"value": "indefinite",

hooks.txt

@@ -70,3 +70,7 @@ $vars: AbuseFilterVariableHolder
 $title: Title object target of the action
 $user: User object performer of the action
 &$skipReasons: Array of reasons why the action should be skipped
+
+'AbuseFilterGetDangerousActions': Allows specifying custom consequences which can harm the user and prevent
+the edit from being saved.
+&$actions: Array (string[]) of dangerous actions

includes/AbuseFilter.php

@@ -56,6 +56,17 @@ class AbuseFilter {
 		'af_group'
 	];
 
+	/**
+	 * @var array Actions which may harm the user. Only retrieve via self::getDangerousActions
+	 * @internal
+	 */
+	public const DANGEROUS_ACTIONS = [
+		'block',
+		'blockautopromote',
+		'degroup',
+		'rangeblock'
+	];
+
 	public const HISTORY_MAPPINGS = [
 		'af_pattern' => 'afh_pattern',
 		'af_user' => 'afh_user',
@@ -81,6 +92,25 @@ class AbuseFilter {
 		return $generator->addUserVars( $user, $entry )->getVariableHolder();
 	}
 
+	/**
+	 * Get an array of action which harm the user.
+	 *
+	 * @return string[]
+	 * @internal Temporary hack
+	 */
+	public static function getDangerousActions() : array {
+		static $actions = null;
+
+		if ( !$actions ) {
+			$extActions = [];
+			AbuseFilterHookRunner::getRunner()->onAbuseFilterGetDangerousActions( $extActions );
+			$actions = array_unique(
+				array_merge( $extActions, self::DANGEROUS_ACTIONS )
+			);
+		}
+		return $actions;
+	}
+
 	/**
 	 * @param int $filterID The ID of the filter
 	 * @param bool|int $global Whether the filter is global
@@ -260,10 +290,9 @@ class AbuseFilter {
 		);
 
 		// Categorise consequences by filter.
-		global $wgAbuseFilterRestrictions;
 		foreach ( $res as $row ) {
 			if ( $row->af_throttled
-				&& !empty( $wgAbuseFilterRestrictions[$row->afa_consequence] )
+				&& in_array( $row->afa_consequence, self::getDangerousActions() )
 			) {
 				// Don't do the action, just log
 				$logger = LoggerFactory::getInstance( 'AbuseFilter' );
@@ -937,7 +966,7 @@ class AbuseFilter {
 		}
 
 		// Check for restricted actions
-		$restrictions = $config->get( 'AbuseFilterRestrictions' );
+		$restrictions = $config->get( 'AbuseFilterActionRestrictions' );
 		if ( count( array_intersect_key(
 				array_filter( $restrictions ),
 				array_merge( $actions, $originalActions )

includes/AbuseFilterHooks.php

@@ -20,7 +20,8 @@ class AbuseFilterHooks {
 	public static function onRegistration() {
 		global $wgAuthManagerAutoConfig, $wgActionFilteredLogs, $wgAbuseFilterProfile,
 			$wgAbuseFilterProfiling, $wgAbuseFilterPrivateLog, $wgAbuseFilterForceSummary,
-			$wgGroupPermissions;
+			$wgGroupPermissions, $wgAbuseFilterRestrictions, $wgAbuseFilterDisallowGlobalLocalBlocks,
+			$wgAbuseFilterActionRestrictions, $wgAbuseFilterLocallyDisabledGlobalActions;
 
 		// @todo Remove this in a future release (added in 1.33)
 		if ( isset( $wgAbuseFilterProfile ) || isset( $wgAbuseFilterProfiling ) ) {
@@ -66,6 +67,35 @@ class AbuseFilterHooks {
 			);
 		}
 
+		// @todo Remove this in a future release (added in 1.36)
+		if ( isset( $wgAbuseFilterDisallowGlobalLocalBlocks ) ) {
+			wfWarn( '$wgAbuseFilterDisallowGlobalLocalBlocks has been removed and replaced by ' .
+				'$wgAbuseFilterLocallyDisabledGlobalActions. You can now specify which actions to disable. ' .
+				'If you had set the former to true, you should set to true all of the actions in ' .
+				'$wgAbuseFilterRestrictions (if you were manually setting the variable) or ' .
+				'AbuseFilter::DANGEROUS_ACTIONS. ' .
+				'If you had set it to false (or left the default), just remove it from your wiki settings.'
+			);
+			if ( $wgAbuseFilterDisallowGlobalLocalBlocks === true ) {
+				$wgAbuseFilterLocallyDisabledGlobalActions = [
+					'throttle' => false,
+					'warn' => false,
+					'disallow' => false,
+					'blockautopromote' => true,
+					'block' => true,
+					'rangeblock' => true,
+					'degroup' => true,
+					'tag' => false
+				];
+			}
+		}
+
+		// @todo Remove this in a future release (added in 1.36)
+		if ( isset( $wgAbuseFilterRestrictions ) ) {
+			wfWarn( '$wgAbuseFilterRestrictions has been renamed to $wgAbuseFilterActionRestrictions.' );
+			$wgAbuseFilterActionRestrictions = $wgAbuseFilterRestrictions;
+		}
+
 		$wgAuthManagerAutoConfig['preauth'][AbuseFilterPreAuthenticationProvider::class] = [
 			'class' => AbuseFilterPreAuthenticationProvider::class,
 			// Run after normal preauth providers to keep the log cleaner

includes/AbuseFilterRunner.php

@@ -490,7 +490,7 @@ class AbuseFilterRunner {
 	 *         the errors and warnings to be shown to the user to explain the actions.
 	 */
 	protected function executeFilterActions( array $filters ) : Status {
-		global $wgMainCacheType, $wgAbuseFilterDisallowGlobalLocalBlocks, $wgAbuseFilterRestrictions,
+		global $wgMainCacheType, $wgAbuseFilterLocallyDisabledGlobalActions,
 			   $wgAbuseFilterBlockDuration, $wgAbuseFilterAnonBlockDuration;
 
 		$actionsByFilter = AbuseFilter::getConsequencesForFilters( $filters );
@@ -530,8 +530,11 @@ class AbuseFilterRunner {
 				}
 			}
 
-			if ( $wgAbuseFilterDisallowGlobalLocalBlocks && $isGlobalFilter ) {
-				$actions = array_diff_key( $actions, array_filter( $wgAbuseFilterRestrictions ) );
+			if ( $isGlobalFilter ) {
+				$actions = array_diff_key(
+					$actions,
+					array_filter( $wgAbuseFilterLocallyDisabledGlobalActions )
+				);
 			}
 
 			if ( !empty( $actions['warn'] ) ) {
@@ -563,9 +566,9 @@ class AbuseFilterRunner {
 				unset( $actions['warn'] );
 			}
 
-			// Prevent double warnings
-			if ( count( array_intersect_key( $actions, array_filter( $wgAbuseFilterRestrictions ) ) ) > 0 &&
-				!empty( $actions['disallow'] )
+			// Don't show the disallow message if a blocking action is executed
+			if ( count( array_intersect( array_keys( $actions ), AbuseFilter::getDangerousActions() ) ) > 0
+				&& !empty( $actions['disallow'] )
 			) {
 				unset( $actions['disallow'] );
 			}

includes/Hooks/AbuseFilterGetDangerousActionsHook.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter\Hooks;
+
+interface AbuseFilterGetDangerousActionsHook {
+	/**
+	 * Hook runner for the `AbuseFilterGetDangerousActions` hook
+	 *
+	 * Allows specifying custom consequences which can harm the user and prevent
+	 * the edit from being saved.
+	 *
+	 * @param string[] &$actions
+	 */
+	public function onAbuseFilterGetDangerousActions( array &$actions ) : void;
+}

includes/Hooks/AbuseFilterHookRunner.php

@@ -25,7 +25,8 @@ class AbuseFilterHookRunner implements
 	AbuseFilterGenerateTitleVarsHook,
 	AbuseFilterGenerateUserVarsHook,
 	AbuseFilterInterceptVariableHook,
-	AbuseFilterShouldFilterActionHook
+	AbuseFilterShouldFilterActionHook,
+	AbuseFilterGetDangerousActionsHook
 {
 
 	/** @var HookContainer */
@@ -287,4 +288,14 @@ class AbuseFilterHookRunner implements
 		);
 	}
 
+	/**
+	 * @inheritDoc
+	 */
+	public function onAbuseFilterGetDangerousActions( array &$actions ) : void {
+		$this->hookContainer->run(
+			'AbuseFilterGetDangerousActions',
+			[ &$actions ],
+			[ 'abortable' => false ]
+		);
+	}
 }

includes/Views/AbuseFilterViewEdit.php

@@ -335,17 +335,18 @@ class AbuseFilterViewEdit extends AbuseFilterView {
 
 		if ( isset( $row->af_throttled ) && $row->af_throttled ) {
 			$filterActions = explode( ',', $row->af_actions );
-			$throttledActions = array_intersect_key(
-				array_flip( $filterActions ),
-				array_filter( $this->getConfig()->get( 'AbuseFilterRestrictions' ) )
+			$throttledActions = array_intersect(
+				$filterActions,
+				AbuseFilter::getDangerousActions()
 			);
 
 			if ( $throttledActions ) {
 				$throttledActions = array_map(
 					function ( $filterAction ) {
+						// TODO: This is AbuseFilter::getActionDisplay, but not escaped
 						return $this->msg( 'abusefilter-action-' . $filterAction )->text();
 					},
-					array_keys( $throttledActions )
+					$throttledActions
 				);
 
 				$flags .= Html::warningBox(

tests/phpunit/AbuseFilterConsequencesTest.php

@@ -649,7 +649,7 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
 	 * @return array [ expected consequences, actual consequences ]
 	 */
 	private function checkConsequences( $result, $actionParams, $consequences ) {
-		global $wgAbuseFilterRestrictions;
+		global $wgAbuseFilterActionRestrictions;
 
 		$expectedErrors = [];
 		$testErrorMessage = false;
@@ -738,10 +738,10 @@ class AbuseFilterConsequencesTest extends MediaWikiTestCase {
 			}
 		}
 
-		if ( array_intersect_key( $expectedErrors, array_filter( $wgAbuseFilterRestrictions ) ) ) {
+		if ( array_intersect_key( $expectedErrors, array_filter( $wgAbuseFilterActionRestrictions ) ) ) {
 			$filteredExpected = array_intersect_key(
 				$expectedErrors,
-				array_filter( $wgAbuseFilterRestrictions )
+				array_filter( $wgAbuseFilterActionRestrictions )
 			);
 			$expected = [];
 			foreach ( $filteredExpected as $values ) {

tests/phpunit/AbuseFilterSaveTest.php

@@ -68,7 +68,7 @@ class AbuseFilterSaveTest extends MediaWikiTestCase {
 				'default',
 				'flow'
 			],
-			'AbuseFilterRestrictions' => [
+			'AbuseFilterActionRestrictions' => [
 				'degroup' => true
 			],
 			'AbuseFilterIsCentral' => true,