Log changes to protected variables access

Similar to how CheckUser logs access to IP information about temporary
accounts, AbuseFilter needs to log whenever protected variables are
accessed.

- Implement ProtectedVarsAccessLogger which handles access logging
- Log whenever a user changes their ability to access protected
  variables via Special:Preferences

Bug: T371798
Change-Id: Ic7024d9c5f369eb33c4198a59638de9a1d58b04b

extension.json

@@ -106,14 +106,17 @@
 	"LogTypes": [
 		"abusefilter",
 		"abusefilterblockeddomainhit",
-		"abusefilterprivatedetails"
+		"abusefilterprivatedetails",
+		"abusefilter-protected-vars"
 	],
 	"LogNames": {
 		"abusefilter": "abusefilter-log-name",
-		"abusefilterprivatedetails": "abusefilterprivatedetails-log-name"
+		"abusefilterprivatedetails": "abusefilterprivatedetails-log-name",
+		"abusefilter-protected-vars": "abusefilter-protected-vars-log-name"
 	},
 	"LogHeaders": {
-		"abusefilter": "abusefilter-log-header"
+		"abusefilter": "abusefilter-log-header",
+		"abusefilter-protected-vars": "abusefilter-protected-vars-log-header"
 	},
 	"LogActionsHandlers": {
 		"abusefilter/hit": {
@@ -129,7 +132,8 @@
 		"suppress/hide-afl": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterSuppressLogFormatter",
 		"suppress/unhide-afl": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterSuppressLogFormatter",
 		"rights/blockautopromote": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterRightsLogFormatter",
-		"rights/restoreautopromote": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterRightsLogFormatter"
+		"rights/restoreautopromote": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterRightsLogFormatter",
+		"abusefilter-protected-vars/*": "LogFormatter"
 	},
 	"ActionFilteredLogs": {
 		"abusefilter": {
@@ -153,12 +157,18 @@
 			"restoreautopromote": [
 				"restoreautopromote"
 			]
+		},
+		"abusefilter-protected-vars": {
+			"change-access": [
+				"change-access"
+			]
 		}
 	},
 	"LogRestrictions": {
 		"abusefilter": "abusefilter-view",
 		"abusefilterprivatedetails": "abusefilter-privatedetails-log",
-		"abusefilterblockeddomainhit": "abusefilter-view"
+		"abusefilterblockeddomainhit": "abusefilter-view",
+		"abusefilter-protected-vars": "abusefilter-protected-vars-log"
 	},
 	"AuthManagerAutoConfig": {
 		"preauth": {
@@ -368,7 +378,8 @@
 		"preferences": {
 			"class": "MediaWiki\\Extension\\AbuseFilter\\Hooks\\Handlers\\PreferencesHandler",
 			"services": [
-				"PermissionManager"
+				"PermissionManager",
+				"AbuseFilterAbuseLoggerFactory"
 			]
 		},
 		"RecentChangeSave": {
@@ -428,7 +439,8 @@
 		"BeforeCreateEchoEvent": "Echo",
 		"ParserOutputStashForEdit": "FilteredActions",
 		"JsonValidateSave": "EditPermission",
-		"GetPreferences": "preferences"
+		"GetPreferences": "preferences",
+		"SaveUserOptions": "preferences"
 	},
 	"ServiceWiringFiles": [
 		"includes/ServiceWiring.php"

i18n/en.json

@@ -33,6 +33,7 @@
 	"right-abusefilter-hide-log": "Hide entries in the abuse log",
 	"right-abusefilter-hidden-log": "View hidden abuse log entries",
 	"right-abusefilter-modify-global": "Create or modify global abuse filters",
+	"right-abusefilter-protected-vars-log": "View logs related to accessing protected variable values",
 	"action-abusefilter-modify": "modify abuse filters",
 	"action-abusefilter-view": "view abuse filters",
 	"action-abusefilter-log": "view the abuse log",
@@ -48,6 +49,7 @@
 	"action-abusefilter-hide-log": "hide entries in the abuse log",
 	"action-abusefilter-hidden-log": "view hidden abuse log entries",
 	"action-abusefilter-modify-global": "create or modify global abuse filters",
+	"action-abusefilter-protected-vars-log": "view logs that reveal protected variables",
 	"abusefilter-log-summary": "This log shows a list of all actions caught by the filters.",
 	"abusefilter-log-search": "Search the abuse log",
 	"abusefilter-log-search-user": "User:",
@@ -631,5 +633,11 @@
 	"right-abusefilter-access-protected-vars": "View and create filters that use protected variables",
 	"action-abusefilter-access-protected-vars": "view and create filters that use protected variables",
 	"prefs-abusefilter": "AbuseFilter",
-	"abusefilter-preference-protected-vars-view-agreement": "Enable revealing IP addresses for temporary accounts in AbuseFilter"
+	"abusefilter-preference-protected-vars-view-agreement": "Enable revealing IP addresses for temporary accounts in AbuseFilter",
+	"abusefilter-protected-vars-log-name": "Abuse filter protected variables log",
+	"abusefilter-protected-vars-log-header": "This is a log of:\n# Viewing protected variables in log details\n# Changing user access levels for viewing protected variables",
+	"logentry-abusefilter-protected-vars-change-access-enable": "$1 enabled {{GENDER:$2|his|her|their}} own access to view protected variables",
+	"logentry-abusefilter-protected-vars-change-access-disable": "$1 disabled {{GENDER:$2|his|her|their}} own access to view protected variables",
+	"log-action-filter-abusefilter-protected-vars": "Type of action:",
+	"log-action-filter-abusefilter-protected-vars-change-access": "Change access"
 }

i18n/qqq.json

@@ -78,6 +78,7 @@
 	"right-abusefilter-hide-log": "{{doc-right|abusefilter-hide-log}}",
 	"right-abusefilter-hidden-log": "{{doc-right|abusefilter-hidden-log}}",
 	"right-abusefilter-modify-global": "{{doc-right|abusefilter-modify-global}}",
+	"right-abusefilter-protected-vars-log": "{{doc-right|abusefilter-protected-vars-log}}",
 	"action-abusefilter-modify": "{{doc-action|abusefilter-modify}}",
 	"action-abusefilter-view": "{{doc-action|abusefilter-view}}",
 	"action-abusefilter-log": "{{doc-action|abusefilter-log}}",
@@ -93,6 +94,7 @@
 	"action-abusefilter-hide-log": "{{doc-action|abusefilter-hide-log}}",
 	"action-abusefilter-hidden-log": "{{doc-action|abusefilter-hidden-log}}",
 	"action-abusefilter-modify-global": "{{doc-action|abusefilter-modify-global}}",
+	"action-abusefilter-protected-vars-log": "{{doc-action|abusefilter-protected-vars-log}",
 	"abusefilter-log-summary": "This message is displayed at the top of the log overview page for extension AbuseFilter.",
 	"abusefilter-log-search": "Caption of a fieldset for filter definition on [[Special:AbuseLog]]",
 	"abusefilter-log-search-user": "Field label in abuse filter log page.\n{{Identical|User}}",
@@ -676,5 +678,11 @@
 	"right-abusefilter-access-protected-vars": "{{doc-right|abusefilter-access-protected-vars}}",
 	"action-abusefilter-access-protected-vars": "{{doc-action|abusefilter-access-protected-vars}}",
 	"prefs-abusefilter": "Header of the AbuseFilter options on [[Special:Preferences]]",
-	"abusefilter-preference-protected-vars-view-agreement": "Agreement shown on [[Special:Preferences]] that the user can acknowledge via checkbox."
+	"abusefilter-preference-protected-vars-view-agreement": "Agreement shown on [[Special:Preferences]] that the user can acknowledge via checkbox.",
+	"abusefilter-protected-vars-log-name": "{{doc-logpage}}\n\nThe page name of [[Special:Log/abusefilter-protected-vars]]. Appears in the drop down menu of the [[Special:Log]] page and subpages.",
+	"abusefilter-protected-vars-log-header": "Appears on top of [[Special:Log/abusefilter-protected-vars]].",
+	"logentry-abusefilter-protected-vars-change-access-enable": "{{Logentry|[[Special:Log/abusefilter-protected-vars]]}}\n\nUsed if a user's state of access was enabled.",
+	"logentry-abusefilter-protected-vars-change-access-disable": "{{Logentry|[[Special:Log/abusefilter-protected-vars]]}}\n\nUsed if a user's state of access was disabled.",
+	"log-action-filter-abusefilter-protected-vars": "{{doc-log-action-filter-type|abusefilter-protected-vars}}\n{{related|Log-action-filter}}",
+	"log-action-filter-abusefilter-protected-vars-change-access": "{{doc-log-action-filter-action|abusefilter-protected-vars|change-access}}"
 }

includes/AbuseLoggerFactory.php

@@ -8,6 +8,7 @@ use MediaWiki\Extension\AbuseFilter\Variables\VariablesBlobStore;
 use MediaWiki\Extension\AbuseFilter\Variables\VariablesManager;
 use MediaWiki\Title\Title;
 use MediaWiki\User\User;
+use Psr\Log\LoggerInterface;
 use Wikimedia\Rdbms\LBFactory;
 
 class AbuseLoggerFactory {
@@ -31,6 +32,8 @@ class AbuseLoggerFactory {
 	private $wikiID;
 	/** @var string */
 	private $requestIP;
+	/** @var LoggerInterface */
+	private $logger;
 
 	/**
 	 * @param CentralDBManager $centralDBManager
@@ -42,6 +45,7 @@ class AbuseLoggerFactory {
 	 * @param ServiceOptions $options
 	 * @param string $wikiID
 	 * @param string $requestIP
+	 * @param LoggerInterface $logger
 	 */
 	public function __construct(
 		CentralDBManager $centralDBManager,
@@ -52,7 +56,8 @@ class AbuseLoggerFactory {
 		LBFactory $lbFactory,
 		ServiceOptions $options,
 		string $wikiID,
-		string $requestIP
+		string $requestIP,
+		LoggerInterface $logger
 	) {
 		$this->centralDBManager = $centralDBManager;
 		$this->filterLookup = $filterLookup;
@@ -63,6 +68,17 @@ class AbuseLoggerFactory {
 		$this->options = $options;
 		$this->wikiID = $wikiID;
 		$this->requestIP = $requestIP;
+		$this->logger = $logger;
+	}
+
+	/**
+	 * @return ProtectedVarsAccessLogger
+	 */
+	public function getProtectedVarsAccessLogger() {
+		return new ProtectedVarsAccessLogger(
+			$this->logger,
+			$this->lbFactory
+		);
 	}
 
 	/**

includes/Hooks/Handlers/PreferencesHandler.php

@@ -2,16 +2,22 @@
 
 namespace MediaWiki\Extension\AbuseFilter\Hooks\Handlers;
 
+use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
 use MediaWiki\Permissions\PermissionManager;
 use MediaWiki\Preferences\Hook\GetPreferencesHook;
+use MediaWiki\User\UserIdentity;
 
 class PreferencesHandler implements GetPreferencesHook {
 	private PermissionManager $permissionManager;
 
+	private AbuseLoggerFactory $abuseLoggerFactory;
+
 	public function __construct(
-		PermissionManager $permissionManager
+		PermissionManager $permissionManager,
+		AbuseLoggerFactory $abuseLoggerFactory
 	) {
 		$this->permissionManager = $permissionManager;
+		$this->abuseLoggerFactory = $abuseLoggerFactory;
 	}
 
 	/** @inheritDoc */
@@ -27,4 +33,30 @@ class PreferencesHandler implements GetPreferencesHook {
 			'noglobal' => true,
 		];
 	}
+
+	/**
+	 * @param UserIdentity $user
+	 * @param array &$modifiedOptions
+	 * @param array $originalOptions
+	 */
+	public function onSaveUserOptions( UserIdentity $user, array &$modifiedOptions, array $originalOptions ) {
+		$wasEnabled = !empty( $originalOptions['abusefilter-protected-vars-view-agreement'] );
+		$wasDisabled = !$wasEnabled;
+
+		$willEnable = !empty( $modifiedOptions['abusefilter-protected-vars-view-agreement'] );
+		$willDisable = isset( $modifiedOptions['abusefilter-protected-vars-view-agreement'] ) &&
+			!$modifiedOptions['abusefilter-protected-vars-view-agreement'];
+
+		if (
+			( $wasEnabled && $willDisable ) ||
+			( $wasDisabled && $willEnable )
+		) {
+			$logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger();
+			if ( $willEnable ) {
+				$logger->logAccessEnabled( $user );
+			} else {
+				$logger->logAccessDisabled( $user );
+			}
+		}
+	}
 }

includes/ProtectedVarsAccessLogger.php

@@ -0,0 +1,111 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter;
+
+use ManualLogEntry;
+use MediaWiki\Title\Title;
+use MediaWiki\User\UserIdentity;
+use Psr\Log\LoggerInterface;
+use Wikimedia\Rdbms\DBError;
+use Wikimedia\Rdbms\IConnectionProvider;
+
+/**
+ * Defines the API for the component responsible for logging the following interactions:
+ *
+ * - A user enables protected variable viewing
+ * - A user disables protected variable viewing
+ */
+class ProtectedVarsAccessLogger {
+	/**
+	 * Represents a user enabling their own access to view protected variables
+	 *
+	 * @var string
+	 */
+	public const ACTION_CHANGE_ACCESS_ENABLED = 'change-access-enable';
+
+	/**
+	 * Represents a user disabling their own access to view protected variables
+	 *
+	 * @var string
+	 */
+	public const ACTION_CHANGE_ACCESS_DISABLED = 'change-access-disable';
+
+	/**
+	 * @var string
+	 */
+	private const LOG_TYPE = 'abusefilter-protected-vars';
+
+	private LoggerInterface $logger;
+	private IConnectionProvider $lbFactory;
+
+	/**
+	 * @param LoggerInterface $logger
+	 * @param IConnectionProvider $lbFactory
+	 */
+	public function __construct(
+		LoggerInterface $logger,
+		IConnectionProvider $lbFactory
+	) {
+		$this->logger = $logger;
+		$this->lbFactory = $lbFactory;
+	}
+
+	/**
+	 * Log when the user enables their own access
+	 *
+	 * @param UserIdentity $performer
+	 */
+	public function logAccessEnabled( UserIdentity $performer ): void {
+		$this->log( $performer, $performer->getName(), self::ACTION_CHANGE_ACCESS_ENABLED );
+	}
+
+	/**
+	 * Log when the user disables their own access
+	 *
+	 * @param UserIdentity $performer
+	 */
+	public function logAccessDisabled( UserIdentity $performer ): void {
+		$this->log( $performer, $performer->getName(), self::ACTION_CHANGE_ACCESS_DISABLED );
+	}
+
+	/**
+	 * @param UserIdentity $performer
+	 * @param string $target
+	 * @param string $action
+	 * @param array|null $params
+	 */
+	private function log(
+		UserIdentity $performer,
+		string $target,
+		string $action,
+		?array $params = []
+	): void {
+		$logEntry = $this->createManualLogEntry( $action );
+		$logEntry->setPerformer( $performer );
+		$logEntry->setTarget( Title::makeTitle( NS_USER, $target ) );
+		$logEntry->setParameters( $params );
+
+		try {
+			$dbw = $this->lbFactory->getPrimaryDatabase();
+			$logEntry->insert( $dbw );
+		} catch ( DBError $e ) {
+			$this->logger->critical(
+				'AbuseFilter proctected variable log entry was not recorded. ' .
+				'This means access to IPs can occur without being auditable. ' .
+				'Immediate fix required.'
+			);
+
+			throw $e;
+		}
+	}
+
+	/**
+	 * @internal
+	 *
+	 * @param string $subtype
+	 * @return ManualLogEntry
+	 */
+	protected function createManualLogEntry( string $subtype ): ManualLogEntry {
+		return new ManualLogEntry( self::LOG_TYPE, $subtype );
+	}
+}

includes/ServiceWiring.php

@@ -259,7 +259,8 @@ return [
 				$services->getMainConfig()
 			),
 			WikiMap::getCurrentWikiDbDomain()->getId(),
-			RequestContext::getMain()->getRequest()->getIP()
+			RequestContext::getMain()->getRequest()->getIP(),
+			LoggerFactory::getInstance( 'AbuseFilter' )
 		);
 	},
 	UpdateHitCountWatcher::SERVICE_NAME => static function ( MediaWikiServices $services ): UpdateHitCountWatcher {

tests/phpunit/unit/AbuseLoggerFactoryTest.php

@@ -9,12 +9,14 @@ use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
 use MediaWiki\Extension\AbuseFilter\CentralDBManager;
 use MediaWiki\Extension\AbuseFilter\EditRevUpdater;
 use MediaWiki\Extension\AbuseFilter\FilterLookup;
+use MediaWiki\Extension\AbuseFilter\ProtectedVarsAccessLogger;
 use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
 use MediaWiki\Extension\AbuseFilter\Variables\VariablesBlobStore;
 use MediaWiki\Extension\AbuseFilter\Variables\VariablesManager;
 use MediaWiki\Title\Title;
 use MediaWiki\User\User;
 use MediaWikiUnitTestCase;
+use Psr\Log\LoggerInterface;
 use Wikimedia\Rdbms\LBFactory;
 
 /**
@@ -40,7 +42,8 @@ class AbuseLoggerFactoryTest extends MediaWikiUnitTestCase {
 				]
 			),
 			'wikiID',
-			'1.2.3.4'
+			'1.2.3.4',
+			$this->createMock( LoggerInterface::class )
 		);
 		$logger = $factory->newLogger(
 			$this->createMock( Title::class ),
@@ -49,6 +52,9 @@ class AbuseLoggerFactoryTest extends MediaWikiUnitTestCase {
 		);
 		$this->assertInstanceOf( AbuseLogger::class, $logger, 'valid' );
 
+		$protectedVarsLogger = $factory->getProtectedVarsAccessLogger();
+		$this->assertInstanceOf( ProtectedVarsAccessLogger::class, $protectedVarsLogger, 'valid' );
+
 		$this->expectException( InvalidArgumentException::class );
 		$factory->newLogger(
 			$this->createMock( Title::class ),