Merge "Log specific views of protected variables"

This commit is contained in:
jenkins-bot 2024-10-03 14:37:48 +00:00 committed by Gerrit Code Review
commit 743bb64924
16 changed files with 442 additions and 59 deletions

View file

@ -85,7 +85,8 @@
"AbuseFilterVariablesBlobStore",
"AbuseFilterSpecsFormatter",
"AbuseFilterVariablesFormatter",
"AbuseFilterVariablesManager"
"AbuseFilterVariablesManager",
"AbuseFilterAbuseLoggerFactory"
]
},
"AbuseFilter": {
@ -133,7 +134,12 @@
"suppress/unhide-afl": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterSuppressLogFormatter",
"rights/blockautopromote": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterRightsLogFormatter",
"rights/restoreautopromote": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\AbuseFilterRightsLogFormatter",
"abusefilter-protected-vars/*": "LogFormatter"
"abusefilter-protected-vars/*": {
"class": "MediaWiki\\Extension\\AbuseFilter\\LogFormatter\\ProtectedVarsAccessLogFormatter",
"services": [
"UserFactory"
]
}
},
"ActionFilteredLogs": {
"abusefilter": {
@ -162,6 +168,9 @@
"change-access": [
"change-access-enable",
"change-access-disable"
],
"view-protected-variable-value": [
"view-protected-variable-value"
]
}
},
@ -231,7 +240,8 @@
"AbuseFilterPermissionManager",
"AbuseFilterVariablesBlobStore",
"AbuseFilterVariablesManager",
"UserFactory"
"UserFactory",
"AbuseFilterAbuseLoggerFactory"
]
},
"abusefilters": {

View file

@ -638,6 +638,8 @@
"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",
"logentry-abusefilter-protected-vars-view-protected-var-value": "$1 viewed protected variables associated with $3",
"log-action-filter-abusefilter-protected-vars": "Type of action:",
"log-action-filter-abusefilter-protected-vars-change-access": "Change access"
"log-action-filter-abusefilter-protected-vars-change-access": "Change access",
"log-action-filter-abusefilter-protected-vars-view-protected-variable-value": "View protected variable values"
}

View file

@ -683,6 +683,8 @@
"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.\n\n{{Doc-singularthey}}",
"logentry-abusefilter-protected-vars-change-access-disable": "{{Logentry|[[Special:Log/abusefilter-protected-vars]]}}\n\nUsed if a user's state of access was disabled.",
"logentry-abusefilter-protected-vars-view-protected-var-value": "{{Logentry|[[Special:Log/abusefilter-protected-vars]]}}\n\nUsed if user viewed the value of a protected variable.",
"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}}"
"log-action-filter-abusefilter-protected-vars-change-access": "{{doc-log-action-filter-action|abusefilter-protected-vars|change-access}}",
"log-action-filter-abusefilter-protected-vars-view-protected-variable-value": "{{doc-log-action-filter-action|abusefilter-protected-vars|view-protected-variable-value}}"
}

View file

@ -123,6 +123,16 @@ class AbuseFilterPermissionManager {
);
}
/**
* Return all used protected variables from an array of variables. Ignore user permissions.
*
* @param string[] $usedVariables
* @return string[]
*/
public function getUsedProtectedVariables( array $usedVariables ): array {
return array_intersect( $usedVariables, $this->protectedVariables );
}
/**
* Check if the filter uses variables that the user is not allowed to use (i.e., variables that are protected, if
* the user can't view protected variables), and return them.

View file

@ -7,6 +7,7 @@ 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\ActorStore;
use MediaWiki\User\User;
use Psr\Log\LoggerInterface;
use Wikimedia\Rdbms\LBFactory;
@ -14,6 +15,14 @@ use Wikimedia\Rdbms\LBFactory;
class AbuseLoggerFactory {
public const SERVICE_NAME = 'AbuseFilterAbuseLoggerFactory';
/**
* The default amount of time after which a duplicate log entry can be inserted. 24 hours (in
* seconds).
*
* @var int
*/
private const DEFAULT_DEBOUNCE_DELAY = 24 * 60 * 60;
/** @var CentralDBManager */
private $centralDBManager;
/** @var FilterLookup */
@ -26,6 +35,8 @@ class AbuseLoggerFactory {
private $editRevUpdater;
/** @var LBFactory */
private $lbFactory;
/** @var ActorStore */
private $actorStore;
/** @var ServiceOptions */
private $options;
/** @var string */
@ -42,6 +53,7 @@ class AbuseLoggerFactory {
* @param VariablesManager $varManager
* @param EditRevUpdater $editRevUpdater
* @param LBFactory $lbFactory
* @param ActorStore $actorStore
* @param ServiceOptions $options
* @param string $wikiID
* @param string $requestIP
@ -54,6 +66,7 @@ class AbuseLoggerFactory {
VariablesManager $varManager,
EditRevUpdater $editRevUpdater,
LBFactory $lbFactory,
ActorStore $actorStore,
ServiceOptions $options,
string $wikiID,
string $requestIP,
@ -65,6 +78,7 @@ class AbuseLoggerFactory {
$this->varManager = $varManager;
$this->editRevUpdater = $editRevUpdater;
$this->lbFactory = $lbFactory;
$this->actorStore = $actorStore;
$this->options = $options;
$this->wikiID = $wikiID;
$this->requestIP = $requestIP;
@ -72,12 +86,17 @@ class AbuseLoggerFactory {
}
/**
* @param int $delay
* @return ProtectedVarsAccessLogger
*/
public function getProtectedVarsAccessLogger() {
public function getProtectedVarsAccessLogger(
int $delay = self::DEFAULT_DEBOUNCE_DELAY
) {
return new ProtectedVarsAccessLogger(
$this->logger,
$this->lbFactory
$this->lbFactory,
$this->actorStore,
$delay
);
}

View file

@ -23,6 +23,7 @@ use ApiQuery;
use ApiQueryBase;
use InvalidArgumentException;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\Filter\FilterNotFoundException;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
@ -64,6 +65,8 @@ class QueryAbuseLog extends ApiQueryBase {
/** @var UserFactory */
private $userFactory;
private AbuseLoggerFactory $abuseLoggerFactory;
/**
* @param ApiQuery $query
* @param string $moduleName
@ -72,6 +75,7 @@ class QueryAbuseLog extends ApiQueryBase {
* @param VariablesBlobStore $afVariablesBlobStore
* @param VariablesManager $afVariablesManager
* @param UserFactory $userFactory
* @param AbuseLoggerFactory $abuseLoggerFactory
*/
public function __construct(
ApiQuery $query,
@ -80,7 +84,8 @@ class QueryAbuseLog extends ApiQueryBase {
AbuseFilterPermissionManager $afPermManager,
VariablesBlobStore $afVariablesBlobStore,
VariablesManager $afVariablesManager,
UserFactory $userFactory
UserFactory $userFactory,
AbuseLoggerFactory $abuseLoggerFactory
) {
parent::__construct( $query, $moduleName, 'afl' );
$this->afFilterLookup = $afFilterLookup;
@ -88,6 +93,7 @@ class QueryAbuseLog extends ApiQueryBase {
$this->afVariablesBlobStore = $afVariablesBlobStore;
$this->afVariablesManager = $afVariablesManager;
$this->userFactory = $userFactory;
$this->abuseLoggerFactory = $abuseLoggerFactory;
}
/**
@ -120,6 +126,10 @@ class QueryAbuseLog extends ApiQueryBase {
$this->checkUserRightsAny( 'abusefilter-log-detail' );
}
$canViewPrivate = $this->afPermManager->canViewPrivateFiltersLogs( $performer );
$canViewProtected = $this->afPermManager->canViewProtectedVariables( $performer );
$canViewProtectedValues = $this->afPermManager->canViewProtectedVariableValues( $performer );
// Map of [ [ id, global ], ... ]
$searchFilters = [];
// Match permissions for viewing events on private filters to SpecialAbuseLog (bug 42814)
@ -139,9 +149,6 @@ class QueryAbuseLog extends ApiQueryBase {
}
}
$canViewPrivate = $this->afPermManager->canViewPrivateFiltersLogs( $performer );
$canViewProtected = $this->afPermManager->canViewProtectedVariables( $performer );
$canViewProtectedValues = $this->afPermManager->canViewProtectedVariableValues( $performer );
if ( !$canViewPrivate || !$canViewProtected || !$canViewProtectedValues ) {
foreach ( $searchFilters as [ $filterID, $global ] ) {
try {
@ -345,12 +352,42 @@ class QueryAbuseLog extends ApiQueryBase {
$varManager = $this->afVariablesManager;
$entry['details'] = $varManager->exportAllVars( $vars );
if ( !$this->afPermManager->canViewProtectedVariableValues( $performer ) ) {
foreach ( $this->afPermManager->getProtectedVariables() as $protectedVariable ) {
$usedProtectedVars = $this->afPermManager
->getUsedProtectedVariables( array_keys( $entry['details'] ) );
if ( $usedProtectedVars ) {
// Unset the variable if the user can't see protected variables
// Additionally, a protected variable is considered used if the key exists
// but since it can have a null value, check isset before logging access
$shouldLog = false;
foreach ( $usedProtectedVars as $protectedVariable ) {
if ( isset( $entry['details'][$protectedVariable] ) ) {
$entry['details'][$protectedVariable] = '';
if ( $canViewProtectedValues ) {
$shouldLog = true;
} else {
$entry['details'][$protectedVariable] = '';
}
}
}
if ( $shouldLog ) {
// phan check - user_name should always exist but guarantee it does and just in case
// if it doesn't, unset the protected variables since they shouldn't be accessed if
// the access isn't logged
if ( isset( $entry['details']['user_name'] ) ) {
$logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger();
$logger->logViewProtectedVariableValue(
$performer->getUser(),
$entry['details']['user_name']
);
} else {
foreach ( $usedProtectedVars as $protectedVariable ) {
if ( isset( $entry['details'][$protectedVariable] ) ) {
$entry['details'][$protectedVariable] = '';
}
}
}
}
}
}
}

View file

@ -0,0 +1,41 @@
<?php
namespace MediaWiki\Extension\AbuseFilter\LogFormatter;
use LogEntry;
use LogFormatter;
use MediaWiki\Extension\AbuseFilter\ProtectedVarsAccessLogger;
use MediaWiki\Linker\Linker;
use MediaWiki\Message\Message;
use MediaWiki\User\UserFactory;
class ProtectedVarsAccessLogFormatter extends LogFormatter {
private UserFactory $userFactory;
public function __construct(
LogEntry $entry,
UserFactory $userFactory
) {
parent::__construct( $entry );
$this->userFactory = $userFactory;
}
/**
* @inheritDoc
*/
protected function getMessageParameters() {
$params = parent::getMessageParameters();
// Replace temporary user page link with contributions page link.
// Don't use LogFormatter::makeUserLink, because that adds tools links.
if ( $this->entry->getSubtype() === ProtectedVarsAccessLogger::ACTION_VIEW_PROTECTED_VARIABLE_VALUE ) {
$tempUserName = $this->entry->getTarget()->getText();
$params[2] = Message::rawParam(
Linker::userLink( 0, $this->userFactory->newUnsavedTempUser( $tempUserName ) )
);
}
return $params;
}
}

View file

@ -5,8 +5,10 @@ namespace MediaWiki\Extension\AbuseFilter;
use ManualLogEntry;
use MediaWiki\MediaWikiServices;
use MediaWiki\Title\Title;
use MediaWiki\User\ActorStore;
use MediaWiki\User\UserIdentity;
use Psr\Log\LoggerInterface;
use Wikimedia\Assert\Assert;
use Wikimedia\Rdbms\DBError;
use Wikimedia\Rdbms\IConnectionProvider;
@ -31,6 +33,13 @@ class ProtectedVarsAccessLogger {
*/
public const ACTION_CHANGE_ACCESS_DISABLED = 'change-access-disable';
/**
* Represents a user viewing the value of a protected variable
*
* @var string
*/
public const ACTION_VIEW_PROTECTED_VARIABLE_VALUE = 'view-protected-var-value';
/**
* @var string
*/
@ -38,17 +47,28 @@ class ProtectedVarsAccessLogger {
private LoggerInterface $logger;
private IConnectionProvider $lbFactory;
private ActorStore $actorStore;
private int $delay;
/**
* @param LoggerInterface $logger
* @param IConnectionProvider $lbFactory
* @param ActorStore $actorStore
* @param int $delay The number of seconds after which a duplicate log entry can be
* created for a debounced log
*/
public function __construct(
LoggerInterface $logger,
IConnectionProvider $lbFactory
IConnectionProvider $lbFactory,
ActorStore $actorStore,
int $delay
) {
Assert::parameter( $delay > 0, 'delay', 'delay must be positive' );
$this->logger = $logger;
$this->lbFactory = $lbFactory;
$this->actorStore = $actorStore;
$this->delay = $delay;
}
/**
@ -57,7 +77,7 @@ class ProtectedVarsAccessLogger {
* @param UserIdentity $performer
*/
public function logAccessEnabled( UserIdentity $performer ): void {
$this->log( $performer, $performer->getName(), self::ACTION_CHANGE_ACCESS_ENABLED );
$this->log( $performer, $performer->getName(), self::ACTION_CHANGE_ACCESS_ENABLED, false );
}
/**
@ -66,21 +86,54 @@ class ProtectedVarsAccessLogger {
* @param UserIdentity $performer
*/
public function logAccessDisabled( UserIdentity $performer ): void {
$this->log( $performer, $performer->getName(), self::ACTION_CHANGE_ACCESS_DISABLED );
$this->log( $performer, $performer->getName(), self::ACTION_CHANGE_ACCESS_DISABLED, false );
}
/**
* Log when the user views the values of protected variables
*
* @param UserIdentity $performer
* @param string $target
* @param int|null $timestamp
*/
public function logViewProtectedVariableValue(
UserIdentity $performer,
string $target,
?int $timestamp = null
): void {
if ( !$timestamp ) {
$timestamp = (int)wfTimestamp();
}
$this->log(
$performer,
$target,
self::ACTION_VIEW_PROTECTED_VARIABLE_VALUE,
true,
$timestamp
);
}
/**
* @param UserIdentity $performer
* @param string $target
* @param string $action
* @param bool $shouldDebounce
* @param int|null $timestamp
* @param array|null $params
*/
private function log(
UserIdentity $performer,
string $target,
string $action,
bool $shouldDebounce,
?int $timestamp = null,
?array $params = []
): void {
if ( !$timestamp ) {
$timestamp = (int)wfTimestamp();
}
// Log to CheckUser's temporary accounts log if CU is installed
if ( MediaWikiServices::getInstance()->getExtensionRegistry()->isLoaded( 'CheckUser' ) ) {
// Add the extension name to the action so that CheckUser has a clearer
// reference to the source in the message key
@ -93,25 +146,63 @@ class ProtectedVarsAccessLogger {
$performer,
$target,
$action,
$params
$params,
$shouldDebounce,
$timestamp
);
} else {
$logEntry = $this->createManualLogEntry( $action );
$logEntry->setPerformer( $performer );
$logEntry->setTarget( Title::makeTitle( NS_USER, $target ) );
$logEntry->setParameters( $params );
$dbw = $this->lbFactory->getPrimaryDatabase();
$shouldLog = false;
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.'
);
// If the log is debounced, check against the logging table before logging
if ( $shouldDebounce ) {
$timestampMinusDelay = $timestamp - $this->delay;
$actorId = $this->actorStore->findActorId( $performer, $dbw );
if ( !$actorId ) {
$shouldLog = true;
} else {
$logline = $dbw->newSelectQueryBuilder()
->select( '*' )
->from( 'logging' )
->where( [
'log_type' => self::LOG_TYPE,
'log_action' => $action,
'log_actor' => $actorId,
'log_namespace' => NS_USER,
'log_title' => $target,
$dbw->expr( 'log_timestamp', '>', $dbw->timestamp( $timestampMinusDelay ) ),
] )
->caller( __METHOD__ )
->fetchRow();
throw $e;
if ( !$logline ) {
$shouldLog = true;
}
}
} else {
// If the log isn't debounced then it should always be logged
$shouldLog = true;
}
// Actually write to logging table
if ( $shouldLog ) {
$logEntry = $this->createManualLogEntry( $action );
$logEntry->setPerformer( $performer );
$logEntry->setTarget( Title::makeTitle( NS_USER, $target ) );
$logEntry->setParameters( $params );
$logEntry->setTimestamp( wfTimestamp( TS_MW, $timestamp ) );
try {
$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;
}
}
}
}

View file

@ -254,6 +254,7 @@ return [
$services->get( VariablesManager::SERVICE_NAME ),
$services->get( EditRevUpdater::SERVICE_NAME ),
$services->getDBLoadBalancerFactory(),
$services->getActorStore(),
new ServiceOptions(
AbuseLogger::CONSTRUCTOR_OPTIONS,
$services->getMainConfig()

View file

@ -3,6 +3,7 @@
namespace MediaWiki\Extension\AbuseFilter\Special;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
use MediaWiki\Extension\AbuseFilter\CentralDBManager;
use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesFactory;
use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
@ -67,6 +68,7 @@ class SpecialAbuseFilter extends AbuseFilterSpecialPage {
VariablesFormatter::SERVICE_NAME,
VariablesManager::SERVICE_NAME,
VariableGeneratorFactory::SERVICE_NAME,
AbuseLoggerFactory::SERVICE_NAME
],
AbuseFilterViewHistory::class => [
'UserNameUtils',

View file

@ -9,6 +9,7 @@ use ManualLogEntry;
use MediaWiki\Cache\LinkBatchFactory;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
use MediaWiki\Extension\AbuseFilter\Filter\FilterNotFoundException;
@ -133,6 +134,8 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
/** @var VariablesManager */
private $varManager;
private AbuseLoggerFactory $abuseLoggerFactory;
/**
* @param LBFactory $lbFactory
* @param LinkBatchFactory $linkBatchFactory
@ -144,6 +147,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
* @param SpecsFormatter $specsFormatter
* @param VariablesFormatter $variablesFormatter
* @param VariablesManager $varManager
* @param AbuseLoggerFactory $abuseLoggerFactory
*/
public function __construct(
LBFactory $lbFactory,
@ -155,7 +159,8 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
VariablesBlobStore $varBlobStore,
SpecsFormatter $specsFormatter,
VariablesFormatter $variablesFormatter,
VariablesManager $varManager
VariablesManager $varManager,
AbuseLoggerFactory $abuseLoggerFactory
) {
parent::__construct( self::PAGE_NAME, 'abusefilter-log', $afPermissionManager );
$this->lbFactory = $lbFactory;
@ -169,6 +174,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$this->variablesFormatter = $variablesFormatter;
$this->variablesFormatter->setMessageLocalizer( $this );
$this->varManager = $varManager;
$this->abuseLoggerFactory = $abuseLoggerFactory;
}
/**
@ -738,12 +744,14 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
->fetchRow();
$error = null;
$privacyLevel = Flags::FILTER_PUBLIC;
if ( !$row ) {
$error = 'abusefilter-log-nonexistent';
} else {
$filterID = $row->afl_filter_id;
$global = $row->afl_global;
$privacyLevel = $row->af_hidden;
if ( $global ) {
try {
$privacyLevel = AbuseFilterServices::getFilterLookup()->getFilter( $filterID, $global )
@ -752,8 +760,6 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
// Conservatively assume that it's hidden and protected, like in AbuseLogPager::doFormatRow
$privacyLevel = Flags::FILTER_HIDDEN & Flags::FILTER_USES_PROTECTED_VARS;
}
} else {
$privacyLevel = $row->af_hidden;
}
if ( !$this->afPermissionManager->canSeeLogDetailsForFilter( $performer, $privacyLevel ) ) {
@ -794,6 +800,8 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
// Load data
$vars = $this->varBlobStore->loadVarDump( $row );
$varsArray = $this->varManager->dumpAllVars( $vars, true );
$shouldLogProtectedVarAccess = false;
// If a non-protected filter and a protected filter have overlapping conditions,
// it's possible for a hit to contain a protected variable and for that variable
@ -802,17 +810,40 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
// We shouldn't block access to the details of an otherwise public filter hit so
// instead only check for access to the protected variables and redact them if the user
// shouldn't see them.
if ( !$this->afPermissionManager->canViewProtectedVariableValues( $performer ) ) {
$varsArray = $this->varManager->dumpAllVars( $vars, true );
foreach ( $this->afPermissionManager->getProtectedVariables() as $protectedVariable ) {
if ( isset( $varsArray[$protectedVariable] ) ) {
$userAuthority = $this->getAuthority();
$canViewProtectedVars = $this->afPermissionManager->canViewProtectedVariableValues( $userAuthority );
foreach ( $this->afPermissionManager->getProtectedVariables() as $protectedVariable ) {
if ( isset( $varsArray[$protectedVariable] ) ) {
if ( !$canViewProtectedVars ) {
$varsArray[$protectedVariable] = '';
} else {
// Protected variables in protected filters logs access in the general permission check
// Log access to non-protected filters that happen to expose protected variables here
if ( !FilterUtils::isProtected( $privacyLevel ) ) {
$shouldLogProtectedVarAccess = true;
}
}
}
$vars = VariableHolder::newFromArray( $varsArray );
}
$vars = VariableHolder::newFromArray( $varsArray );
// Log if protected variables are accessed
if (
FilterUtils::isProtected( $privacyLevel ) &&
$canViewProtectedVars
) {
$shouldLogProtectedVarAccess = true;
}
$out->addJsConfigVars( 'wgAbuseFilterVariables', $this->varManager->dumpAllVars( $vars, true ) );
if ( $shouldLogProtectedVarAccess ) {
$logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger();
$logger->logViewProtectedVariableValue(
$userAuthority->getUser(),
$varsArray['user_name']
);
}
$out->addJsConfigVars( 'wgAbuseFilterVariables', $varsArray );
$out->addModuleStyles( 'mediawiki.interface.helpers.styles' );
// Diff, if available
@ -886,7 +917,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
* or an error and no row.
*/
public static function getPrivateDetailsRow( Authority $authority, $id ) {
$afPermManager = AbuseFilterServices::getPermissionManager();
$afPermissionManager = AbuseFilterServices::getPermissionManager();
$dbr = MediaWikiServices::getInstance()->getConnectionProvider()->getReplicaDatabase();
$row = $dbr->newSelectQueryBuilder()
@ -914,7 +945,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$privacyLevel = $row->af_hidden;
}
if ( !$afPermManager->canSeeLogDetailsForFilter( $authority, $privacyLevel ) ) {
if ( !$afPermissionManager->canSeeLogDetailsForFilter( $authority, $privacyLevel ) ) {
$status->fatal( 'abusefilter-log-cannot-see-details' );
return $status;
}
@ -1167,15 +1198,15 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
/**
* @param stdClass $row
* @param Authority $authority
* @param AbuseFilterPermissionManager $afPermManager
* @param AbuseFilterPermissionManager $afPermissionManager
* @return string One of the self::VISIBILITY_* constants
*/
public static function getEntryVisibilityForUser(
stdClass $row,
Authority $authority,
AbuseFilterPermissionManager $afPermManager
AbuseFilterPermissionManager $afPermissionManager
): string {
if ( $row->afl_deleted && !$afPermManager->canSeeHiddenLogEntries( $authority ) ) {
if ( $row->afl_deleted && !$afPermissionManager->canSeeHiddenLogEntries( $authority ) ) {
return self::VISIBILITY_HIDDEN;
}
if ( !$row->afl_rev_id ) {

View file

@ -7,6 +7,7 @@ use LogicException;
use MediaWiki\Context\IContextSource;
use MediaWiki\Extension\AbuseFilter\AbuseFilterChangesList;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\EditBox\EditBoxBuilderFactory;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
@ -62,6 +63,8 @@ class AbuseFilterViewExamine extends AbuseFilterView {
*/
private $varGeneratorFactory;
private AbuseLoggerFactory $abuseLoggerFactory;
/**
* @param LBFactory $lbFactory
* @param AbuseFilterPermissionManager $afPermManager
@ -71,6 +74,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
* @param VariablesFormatter $variablesFormatter
* @param VariablesManager $varManager
* @param VariableGeneratorFactory $varGeneratorFactory
* @param AbuseLoggerFactory $abuseLoggerFactory
* @param IContextSource $context
* @param LinkRenderer $linkRenderer
* @param string $basePageName
@ -85,6 +89,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
VariablesFormatter $variablesFormatter,
VariablesManager $varManager,
VariableGeneratorFactory $varGeneratorFactory,
AbuseLoggerFactory $abuseLoggerFactory,
IContextSource $context,
LinkRenderer $linkRenderer,
string $basePageName,
@ -99,6 +104,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
$this->variablesFormatter->setMessageLocalizer( $context );
$this->varManager = $varManager;
$this->varGeneratorFactory = $varGeneratorFactory;
$this->abuseLoggerFactory = $abuseLoggerFactory;
}
/**
@ -290,19 +296,22 @@ class AbuseFilterViewExamine extends AbuseFilterView {
return;
}
$shouldLogProtectedVarAccess = false;
// Logs that reveal the values of protected variables are gated behind:
// 1. the `abusefilter-access-protected-vars` right
// 2. agreement to the `abusefilter-protected-vars-view-agreement` preference
$userAuthority = $this->getAuthority();
if ( FilterUtils::isProtected( $privacyLevel ) &&
!$this->afPermManager->canViewProtectedVariableValues( $userAuthority )
) {
$out->addWikiMsg( 'abusefilter-examine-protected-vars-permission' );
return;
$canViewProtectedVars = $this->afPermManager->canViewProtectedVariableValues( $userAuthority );
if ( FilterUtils::isProtected( $privacyLevel ) ) {
if ( !$canViewProtectedVars ) {
$out->addWikiMsg( 'abusefilter-examine-protected-vars-permission' );
return;
} else {
$shouldLogProtectedVarAccess = true;
}
}
$vars = $this->varBlobStore->loadVarDump( $row );
// If a non-protected filter and a protected filter have overlapping conditions,
// it's possible for a hit to contain a protected variable and for that variable
// to be dumped and displayed on a detail page that wouldn't be considered
@ -310,18 +319,34 @@ class AbuseFilterViewExamine extends AbuseFilterView {
// We shouldn't block access to the details of an otherwise public filter hit so
// instead only check for access to the protected variables and redact them if the
// user shouldn't see them.
if ( !$this->afPermManager->canViewProtectedVariableValues( $userAuthority ) ) {
$varsArray = $this->varManager->dumpAllVars( $vars, true );
foreach ( $this->afPermManager->getProtectedVariables() as $protectedVariable ) {
if ( isset( $varsArray[$protectedVariable] ) ) {
$vars = $this->varBlobStore->loadVarDump( $row );
$varsArray = $this->varManager->dumpAllVars( $vars, true );
foreach ( $this->afPermManager->getProtectedVariables() as $protectedVariable ) {
if ( isset( $varsArray[$protectedVariable] ) ) {
if ( !$canViewProtectedVars ) {
$varsArray[$protectedVariable] = '';
} else {
// Protected variable in protected filters logs access in the general permission check
// Log access to non-protected filters that happen to expose protected variables here
if ( !FilterUtils::isProtected( $privacyLevel ) ) {
$shouldLogProtectedVarAccess = true;
}
}
}
$vars = VariableHolder::newFromArray( $varsArray );
}
$vars = VariableHolder::newFromArray( $varsArray );
if ( $shouldLogProtectedVarAccess ) {
$logger = $this->abuseLoggerFactory->getProtectedVarsAccessLogger();
$logger->logViewProtectedVariableValue(
$userAuthority->getUser(),
$varsArray['user_name']
);
}
$out->addJsConfigVars( [
'wgAbuseFilterVariables' => $this->varManager->dumpAllVars( $vars, true ),
'wgAbuseFilterVariables' => $varsArray,
'abuseFilterExamine' => [ 'type' => 'log', 'id' => $logid ]
] );
$this->showExaminer( $vars );

View file

@ -11,6 +11,7 @@ use MediaWiki\Extension\AbuseFilter\Filter\Specs;
use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
use MediaWiki\Permissions\SimpleAuthority;
use MediaWiki\Tests\Api\ApiTestCase;
use MediaWiki\User\ActorStore;
use MediaWiki\User\Options\StaticUserOptionsLookup;
use MediaWiki\User\UserIdentityValue;
use Wikimedia\TestingAccessWrapper;
@ -59,6 +60,7 @@ class QueryAbuseLogTest extends ApiTestCase {
VariableHolder::newFromArray( [
'action' => 'edit',
'user_unnamed_ip' => '1.2.3.4',
'user_name' => 'User1',
] )
)->addLogEntries( [ 1 => [ 'warn' ] ] );
@ -108,6 +110,11 @@ class QueryAbuseLogTest extends ApiTestCase {
);
$this->setService( 'UserOptionsLookup', $userOptions );
// Actor store needs to return a valid actor_id for the logs querying generates
$actorStore = $this->createMock( ActorStore::class );
$actorStore->method( 'acquireActorId' )->willReturn( 1 );
$this->setService( 'ActorStore', $actorStore );
// Assert that the ip is now visible
$result = $this->doApiRequest( [
'action' => 'query',
@ -116,6 +123,8 @@ class QueryAbuseLogTest extends ApiTestCase {
], null, null, $authorityCanViewProtectedVar );
$result = $result[0]['query']['abuselog'];
$this->assertSame( '1.2.3.4', $result[0]['details']['user_unnamed_ip'] );
$this->resetServices();
}
/**

View file

@ -117,4 +117,84 @@ class ProtectedVarsAccessLoggerTest extends MediaWikiIntegrationTestCase {
->fetchField()
);
}
public function testDebouncedLogs_CUDisabled() {
$extensionRegistry = $this->createMock( ExtensionRegistry::class );
$extensionRegistry->method( 'isLoaded' )->with( 'CheckUser' )->willReturn( false );
$this->setService( 'ExtensionRegistry', $extensionRegistry );
// Run the same action twice
$performer = $this->getTestSysop();
AbuseFilterServices::getAbuseLoggerFactory()
->getProtectedVarsAccessLogger()
->logViewProtectedVariableValue( $performer->getUserIdentity(), '~2024-01', (int)wfTimestamp() );
AbuseFilterServices::getAbuseLoggerFactory()
->getProtectedVarsAccessLogger()
->logViewProtectedVariableValue( $performer->getUserIdentity(), '~2024-01', (int)wfTimestamp() );
// Assert that the action wasn't inserted into CheckUsers' temp account logging table
$this->assertSame(
0,
(int)$this->getDb()->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'logging' )
->where( [
'log_action' => 'af-view-protected-var-value',
'log_type' => TemporaryAccountLogger::LOG_TYPE,
] )
->fetchField()
);
// and also that it only inserted once into abusefilter's protected vars logging table
$this->assertSame(
1,
(int)$this->getDb()->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'logging' )
->where( [
'log_action' => 'view-protected-var-value',
'log_type' => ProtectedVarsAccessLogger::LOG_TYPE,
] )
->fetchField()
);
$this->resetServices();
}
public function testDebouncedLogs_CUEnabled() {
$this->markTestSkippedIfExtensionNotLoaded( 'CheckUser' );
// Run the same action twice
$performer = $this->getTestSysop();
AbuseFilterServices::getAbuseLoggerFactory()
->getProtectedVarsAccessLogger()
->logViewProtectedVariableValue( $performer->getUserIdentity(), '~2024-01', (int)wfTimestamp() );
AbuseFilterServices::getAbuseLoggerFactory()
->getProtectedVarsAccessLogger()
->logViewProtectedVariableValue( $performer->getUserIdentity(), '~2024-01', (int)wfTimestamp() );
// Assert that the action only inserted once into CheckUsers' temp account logging table
$this->assertSame(
1,
(int)$this->getDb()->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'logging' )
->where( [
'log_action' => 'af-view-protected-var-value',
'log_type' => TemporaryAccountLogger::LOG_TYPE,
] )
->fetchField()
);
// and also that it wasn't inserted into abusefilter's protected vars logging table
$this->assertSame(
0,
(int)$this->getDb()->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'logging' )
->where( [
'log_action' => 'view-protected-var-value',
'log_type' => ProtectedVarsAccessLogger::LOG_TYPE,
] )
->fetchField()
);
}
}

View file

@ -291,6 +291,27 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
);
}
public function provideTestGetUsedProtectedVariables(): Generator {
$userCheckedPreference = new UserIdentityValue( 1, 'User1' );
$userUncheckedPreference = new UserIdentityValue( 2, 'User2' );
yield 'uses protected variables' => [
[ 'user_unnamed_ip', 'user_name' ], [ 'user_unnamed_ip' ]
];
yield 'no protected variables' => [
[ 'user_name' ], []
];
}
/**
* @dataProvider provideTestGetUsedProtectedVariables
*/
public function testGetUsedProtectedVariables( array $usedVariables, $expected ) {
$this->assertSame(
$expected,
$this->getPermMan()->getUsedProtectedVariables( $usedVariables )
);
}
public static function provideGetForbiddenVariables(): Generator {
yield 'cannot view, protected vars' => [
[

View file

@ -14,6 +14,7 @@ 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\ActorStore;
use MediaWiki\User\User;
use MediaWikiUnitTestCase;
use Psr\Log\LoggerInterface;
@ -33,6 +34,7 @@ class AbuseLoggerFactoryTest extends MediaWikiUnitTestCase {
$this->createMock( VariablesManager::class ),
$this->createMock( EditRevUpdater::class ),
$this->createMock( LBFactory::class ),
$this->createMock( ActorStore::class ),
new ServiceOptions(
AbuseLogger::CONSTRUCTOR_OPTIONS,
[