Add preference for viewing protected variables in AbuseFilter

Users need to enable a preference before gaining access to the IPs
from `user_unnamed_ip`, a protected variable.

- Add a preference that the user can check to toggle their access
- Check for the preference and the view right for logs that reveal
  protected variables on:
  + AbuseFilterViewExamine
  + SpecialAbuseLog
  + QueryAbuseLog

Bug: T371798
Change-Id: I5363380d999118982b216585ea73ee4274a6eac1
This commit is contained in:
STran 2024-08-14 07:11:10 -07:00
parent 86370ce996
commit bd819b98a2
11 changed files with 354 additions and 8 deletions

View file

@ -365,6 +365,12 @@
"AbuseFilterEditRevUpdater"
]
},
"preferences": {
"class": "MediaWiki\\Extension\\AbuseFilter\\Hooks\\Handlers\\PreferencesHandler",
"services": [
"PermissionManager"
]
},
"RecentChangeSave": {
"class": "MediaWiki\\Extension\\AbuseFilter\\Hooks\\Handlers\\RecentChangeSaveHandler",
"services": [
@ -421,7 +427,8 @@
"UserMergeAccountFields": "UserMerge",
"BeforeCreateEchoEvent": "Echo",
"ParserOutputStashForEdit": "FilteredActions",
"JsonValidateSave": "EditPermission"
"JsonValidateSave": "EditPermission",
"GetPreferences": "preferences"
},
"ServiceWiringFiles": [
"includes/ServiceWiring.php"

View file

@ -44,6 +44,7 @@
"action-abusefilter-view-private": "view abuse filters marked as private",
"action-abusefilter-log-private": "view logs of abuse filters marked as private",
"action-abusefilter-log-protected": "view logs of abuse filters marked as protected",
"action-abusefilter-log-protected-access": "view logs with exposed protected variables without having first enabled the option in your preferences",
"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",
@ -554,6 +555,7 @@
"abusefilter-examine-notfound": "The change you requested could not be found.",
"abusefilter-examine-incompatible": "The change you requested is not supported by the Abuse Filter",
"abusefilter-examine-noresults": "No results were found for the search parameters you provided.",
"abusefilter-examine-protected-vars-permission": "To view logs that contain protected variables, please accept the agreement in [[Special:Preferences|your preferences]].",
"abusefilter-topnav": "'''Abuse Filter navigation'''",
"abusefilter-topnav-home": "Home",
"abusefilter-topnav-recentchanges": "Recent filter changes",
@ -627,5 +629,7 @@
"action-abusefilter-bypass-blocked-external-domains": "bypass blocked external domain",
"abusefilter-blocked-domains-cannot-edit-directly": "Create or modify what external domains are blocked from being linked must be done through [[Special:BlockedExternalDomains|the special page]].",
"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"
"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"
}

View file

@ -89,6 +89,7 @@
"action-abusefilter-view-private": "{{doc-action|abusefilter-view-private}}",
"action-abusefilter-log-private": "{{doc-action|abusefilter-log-private}}",
"action-abusefilter-log-protected": "{{doc-action|abusefilter-access-protected-vars}}",
"action-abusefilter-log-protected-access": "{{doc-action|abusefilter-log-protected-access}}",
"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}}",
@ -599,6 +600,7 @@
"abusefilter-examine-notfound": "Used as warning on [[Special:AbuseFilter/examine]]",
"abusefilter-examine-incompatible": "Used as error message on [[Special:AbuseFilter/examine]]",
"abusefilter-examine-noresults": "Used as warning on [[Special:AbuseFilter/examine]]",
"abusefilter-examine-protected-vars-permission": "Used as warning on [[Special:AbuseFilter/examine]]",
"abusefilter-topnav": "Used as header for navigation links which have the following link texts:\n* {{msg-mw|Abusefilter-topnav-home}}\n* {{msg-mw|Abusefilter-topnav-test}}\n* {{msg-mw|Abusefilter-topnav-examine}}\n* {{msg-mw|Abusefilter-topnav-log}}\n* {{msg-mw|Abusefilter-topnav-tools}}\n* {{msg-mw|Abusefilter-topnav-import}}",
"abusefilter-topnav-home": "Used as link text. The link points to [[Special:AbuseFilter]].\n{{Identical|Home}}",
"abusefilter-topnav-recentchanges": "Used as link text in the navigation toolbar. The link points to the page that shows recent changes in all the filters. [[Special:AbuseFilter/history]].\nSee {{msg-mw|abusefilter-filter-log}}.",
@ -672,5 +674,7 @@
"action-abusefilter-bypass-blocked-external-domains": "{{doc-action|abusefilter-bypass-blocked-external-domains}}",
"abusefilter-blocked-domains-cannot-edit-directly": "Error message shown when someone tries to edit the list of blocked domains directly and bypass the Special page.",
"right-abusefilter-access-protected-vars": "{{doc-right|abusefilter-access-protected-vars}}",
"action-abusefilter-access-protected-vars": "{{doc-action|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."
}

View file

@ -6,6 +6,7 @@ use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\AbuseFilter\Filter\AbstractFilter;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Permissions\Authority;
use MediaWiki\User\Options\UserOptionsLookup;
/**
* This class simplifies the interactions between the AbuseFilter code and Authority, knowing
@ -23,13 +24,18 @@ class AbuseFilterPermissionManager {
*/
private $protectedVariables;
private UserOptionsLookup $userOptionsLookup;
/**
* @param ServiceOptions $options
* @param UserOptionsLookup $userOptionsLookup
*/
public function __construct(
ServiceOptions $options
ServiceOptions $options,
UserOptionsLookup $userOptionsLookup
) {
$this->protectedVariables = $options->get( 'AbuseFilterProtectedVariables' );
$this->userOptionsLookup = $userOptionsLookup;
}
/**
@ -103,6 +109,20 @@ class AbuseFilterPermissionManager {
);
}
/**
* @param Authority $performer
* @return bool
*/
public function canViewProtectedVariableValues( Authority $performer ) {
return (
$this->canViewProtectedVariables( $performer ) &&
$this->userOptionsLookup->getOption(
$performer->getUser(),
'abusefilter-protected-vars-view-agreement'
)
);
}
/**
* 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.
@ -120,6 +140,15 @@ class AbuseFilterPermissionManager {
return $usedProtectedVariables;
}
/**
* Return an array of protected variables (originally defined in configuration)
*
* @return string[]
*/
public function getProtectedVariables() {
return $this->protectedVariables;
}
/**
* @param Authority $performer
* @return bool

View file

@ -129,7 +129,6 @@ class QueryAbuseLog extends ApiQueryBase {
if ( !is_array( $params['filter'] ) ) {
$params['filter'] = [ $params['filter'] ];
}
$foundInvalid = false;
foreach ( $params['filter'] as $filter ) {
try {
@ -142,7 +141,8 @@ class QueryAbuseLog extends ApiQueryBase {
$canViewPrivate = $this->afPermManager->canViewPrivateFiltersLogs( $performer );
$canViewProtected = $this->afPermManager->canViewProtectedVariables( $performer );
if ( !$canViewPrivate || !$canViewProtected ) {
$canViewProtectedValues = $this->afPermManager->canViewProtectedVariableValues( $performer );
if ( !$canViewPrivate || !$canViewProtected || !$canViewProtectedValues ) {
foreach ( $searchFilters as [ $filterID, $global ] ) {
try {
$privacyLevel = $lookup->getFilter( $filterID, $global )->getPrivacyLevel();
@ -163,6 +163,11 @@ class QueryAbuseLog extends ApiQueryBase {
[ 'apierror-permissiondenied', $this->msg( 'action-abusefilter-log-protected' ) ]
);
}
if ( !$canViewProtectedValues && ( Flags::FILTER_USES_PROTECTED_VARS & $privacyLevel ) ) {
$this->dieWithError(
[ 'apierror-permissiondenied', $this->msg( 'action-abusefilter-log-protected-access' ) ]
);
}
}
}
@ -339,6 +344,14 @@ class QueryAbuseLog extends ApiQueryBase {
$vars = $this->afVariablesBlobStore->loadVarDump( $row );
$varManager = $this->afVariablesManager;
$entry['details'] = $varManager->exportAllVars( $vars );
if ( !$this->afPermManager->canViewProtectedVariableValues( $performer ) ) {
foreach ( $this->afPermManager->getProtectedVariables() as $protectedVariable ) {
if ( isset( $entry['details'][$protectedVariable] ) ) {
$entry['details'][$protectedVariable] = '';
}
}
}
}
}

View file

@ -0,0 +1,30 @@
<?php
namespace MediaWiki\Extension\AbuseFilter\Hooks\Handlers;
use MediaWiki\Permissions\PermissionManager;
use MediaWiki\Preferences\Hook\GetPreferencesHook;
class PreferencesHandler implements GetPreferencesHook {
private PermissionManager $permissionManager;
public function __construct(
PermissionManager $permissionManager
) {
$this->permissionManager = $permissionManager;
}
/** @inheritDoc */
public function onGetPreferences( $user, &$preferences ): void {
if ( !$this->permissionManager->userHasRight( $user, 'abusefilter-access-protected-vars' ) ) {
return;
}
$preferences['abusefilter-protected-vars-view-agreement'] = [
'type' => 'toggle',
'label-message' => 'abusefilter-preference-protected-vars-view-agreement',
'section' => 'personal/abusefilter',
'noglobal' => true,
];
}
}

View file

@ -76,7 +76,8 @@ return [
PermManager::CONSTRUCTOR_OPTIONS,
$services->getMainConfig(),
[ 'AbuseFilterProtectedVariables' => [] ]
)
),
$services->getUserOptionsLookup()
);
},
ChangeTagger::SERVICE_NAME => static function ( MediaWikiServices $services ): ChangeTagger {

View file

@ -13,10 +13,12 @@ use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
use MediaWiki\Extension\AbuseFilter\Filter\FilterNotFoundException;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Extension\AbuseFilter\FilterUtils;
use MediaWiki\Extension\AbuseFilter\GlobalNameUtils;
use MediaWiki\Extension\AbuseFilter\Pager\AbuseLogPager;
use MediaWiki\Extension\AbuseFilter\SpecsFormatter;
use MediaWiki\Extension\AbuseFilter\Variables\UnsetVariableException;
use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
use MediaWiki\Extension\AbuseFilter\Variables\VariablesBlobStore;
use MediaWiki\Extension\AbuseFilter\Variables\VariablesFormatter;
use MediaWiki\Extension\AbuseFilter\Variables\VariablesManager;
@ -764,6 +766,13 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$error = 'abusefilter-log-details-hidden-implicit';
}
}
if (
FilterUtils::isProtected( $privacyLevel ) &&
!$this->afPermissionManager->canViewProtectedVariableValues( $performer )
) {
$error = 'abusefilter-examine-protected-vars-permission';
}
}
if ( $error ) {
@ -783,6 +792,23 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
// Load data
$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
// protected (because it caught on the public filter).
// 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] ) ) {
$varsArray[$protectedVariable] = '';
}
}
$vars = VariableHolder::newFromArray( $varsArray );
}
$out->addJsConfigVars( 'wgAbuseFilterVariables', $this->varManager->dumpAllVars( $vars, true ) );
$out->addModuleStyles( 'mediawiki.interface.helpers.styles' );

View file

@ -11,6 +11,7 @@ use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException;
use MediaWiki\Extension\AbuseFilter\EditBox\EditBoxBuilderFactory;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Extension\AbuseFilter\FilterLookup;
use MediaWiki\Extension\AbuseFilter\FilterUtils;
use MediaWiki\Extension\AbuseFilter\Pager\AbuseFilterExaminePager;
use MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseLog;
use MediaWiki\Extension\AbuseFilter\VariableGenerator\VariableGeneratorFactory;
@ -289,8 +290,36 @@ class AbuseFilterViewExamine extends AbuseFilterView {
return;
}
// 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;
}
$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
// protected (because it caught on the public filter).
// 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] ) ) {
$varsArray[$protectedVariable] = '';
}
}
$vars = VariableHolder::newFromArray( $varsArray );
}
$out->addJsConfigVars( [
'wgAbuseFilterVariables' => $this->varManager->dumpAllVars( $vars, true ),
'abuseFilterExamine' => [ 'type' => 'log', 'id' => $logid ]

View file

@ -2,7 +2,18 @@
namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Api;
use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
use MediaWiki\Extension\AbuseFilter\Filter\Filter;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Extension\AbuseFilter\Filter\LastEditInfo;
use MediaWiki\Extension\AbuseFilter\Filter\MutableFilter;
use MediaWiki\Extension\AbuseFilter\Filter\Specs;
use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
use MediaWiki\Permissions\SimpleAuthority;
use MediaWiki\Tests\Api\ApiTestCase;
use MediaWiki\User\Options\StaticUserOptionsLookup;
use MediaWiki\User\UserIdentityValue;
use Wikimedia\TestingAccessWrapper;
/**
* @covers \MediaWiki\Extension\AbuseFilter\Api\QueryAbuseLog
@ -20,4 +31,148 @@ class QueryAbuseLogTest extends ApiTestCase {
$this->addToAssertionCount( 1 );
}
public function testProtectedVariableValueAcces() {
// Add filter to query for
$filter = [
'id' => '1',
'rules' => 'user_unnamed_ip = "1.2.3.4"',
'name' => 'Filter with protected variables',
'hidden' => Flags::FILTER_USES_PROTECTED_VARS,
'user' => 0,
'user_text' => 'FilterTester',
'timestamp' => '20190826000000',
'enabled' => 1,
'comments' => '',
'hit_count' => 0,
'throttled' => 0,
'deleted' => 0,
'actions' => [],
'global' => 0,
'group' => 'default'
];
$this->createFilter( $filter );
// Insert a hit on the filter
AbuseFilterServices::getAbuseLoggerFactory()->newLogger(
$this->getExistingTestPage()->getTitle(),
$this->getTestUser()->getUser(),
VariableHolder::newFromArray( [
'action' => 'edit',
'user_unnamed_ip' => '1.2.3.4',
] )
)->addLogEntries( [ 1 => [ 'warn' ] ] );
// Update afl_ip to a known value that can be used when it's reconstructed in the variable holder
$this->getDb()->newUpdateQueryBuilder()
->update( 'abuse_filter_log' )
->set( [ 'afl_ip' => '1.2.3.4' ] )
->where( [ 'afl_filter_id' => 1 ] )
->caller( __METHOD__ )->execute();
// Create the user to query for filters
$user = new UserIdentityValue( 1, 'User1' );
// Create an authority who can see protected variables but hasn't checked the preference
$authorityCanViewProtectedVar = new SimpleAuthority(
$user,
[
'abusefilter-log-detail',
'abusefilter-view',
'abusefilter-log',
'abusefilter-privatedetails',
'abusefilter-privatedetails-log',
'abusefilter-view-private',
'abusefilter-log-private',
'abusefilter-hidden-log',
'abusefilter-hide-log',
'abusefilter-access-protected-vars'
]
);
// Assert that the ip isn't visible in the result
$result = $this->doApiRequest( [
'action' => 'query',
'list' => 'abuselog',
'aflprop' => 'details'
], null, null, $authorityCanViewProtectedVar );
$result = $result[0]['query']['abuselog'];
$this->assertSame( '', $result[0]['details']['user_unnamed_ip'] );
// Enable the preference for the user
$userOptions = new StaticUserOptionsLookup(
[
'User1' => [
'abusefilter-protected-vars-view-agreement' => 1
]
]
);
$this->setService( 'UserOptionsLookup', $userOptions );
// Assert that the ip is now visible
$result = $this->doApiRequest( [
'action' => 'query',
'list' => 'abuselog',
'aflprop' => 'details'
], null, null, $authorityCanViewProtectedVar );
$result = $result[0]['query']['abuselog'];
$this->assertSame( '1.2.3.4', $result[0]['details']['user_unnamed_ip'] );
}
/**
* Adapted from FilterStoreTest->createFilter()
*
* @param array $row
*/
private function createFilter( array $row ): void {
$row['timestamp'] = $this->getDb()->timestamp( $row['timestamp'] );
$filter = $this->getFilterFromSpecs( $row );
$oldFilter = MutableFilter::newDefault();
// Use some black magic to bypass checks
/** @var FilterStore $filterStore */
$filterStore = TestingAccessWrapper::newFromObject( AbuseFilterServices::getFilterStore() );
$row = $filterStore->filterToDatabaseRow( $filter, $oldFilter );
$row['af_actor'] = $this->getServiceContainer()->getActorNormalization()->acquireActorId(
$this->getTestUser()->getUserIdentity(),
$this->getDb()
);
$this->getDb()->newInsertQueryBuilder()
->insertInto( 'abuse_filter' )
->row( $row )
->caller( __METHOD__ )
->execute();
}
/**
* Adapted from FilterStoreTest->getFilterFromSpecs()
*
* @param array $filterSpecs
* @param array $actions
* @return Filter
*/
private function getFilterFromSpecs( array $filterSpecs, array $actions = [] ): Filter {
return new Filter(
new Specs(
$filterSpecs['rules'],
$filterSpecs['comments'],
$filterSpecs['name'],
array_keys( $filterSpecs['actions'] ),
$filterSpecs['group']
),
new Flags(
$filterSpecs['enabled'],
$filterSpecs['deleted'],
$filterSpecs['hidden'],
$filterSpecs['global']
),
$actions,
new LastEditInfo(
$filterSpecs['user'],
$filterSpecs['user_text'],
$filterSpecs['timestamp']
),
$filterSpecs['id'],
$filterSpecs['hit_count'],
$filterSpecs['throttled']
);
}
}

View file

@ -10,6 +10,8 @@ use MediaWiki\Extension\AbuseFilter\Filter\AbstractFilter;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Extension\AbuseFilter\Filter\MutableFilter;
use MediaWiki\Tests\Unit\Permissions\MockAuthorityTrait;
use MediaWiki\User\Options\StaticUserOptionsLookup;
use MediaWiki\User\UserIdentityValue;
use MediaWikiUnitTestCase;
/**
@ -19,13 +21,24 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
use MockAuthorityTrait;
private function getPermMan(): AbuseFilterPermissionManager {
$userOptions = new StaticUserOptionsLookup(
[
'User1' => [
'abusefilter-protected-vars-view-agreement' => 1
],
'User2' => [
'abusefilter-protected-vars-view-agreement' => ''
],
]
);
return new AbuseFilterPermissionManager(
new ServiceOptions(
AbuseFilterPermissionManager::CONSTRUCTOR_OPTIONS,
[
'AbuseFilterProtectedVariables' => [ 'user_unnamed_ip' ]
]
)
),
$userOptions
);
}
@ -250,6 +263,34 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
);
}
public function provideCanViewProtectedVariableValues(): Generator {
$userCheckedPreference = new UserIdentityValue( 1, 'User1' );
$userUncheckedPreference = new UserIdentityValue( 2, 'User2' );
yield 'can view protected variables, has checked preference' => [
[ 'abusefilter-access-protected-vars' ], $userCheckedPreference, true
];
yield 'can view protected variables, has not checked preference' => [
[ 'abusefilter-access-protected-vars' ], $userUncheckedPreference, false
];
yield 'cannot view protected variables, has checked preference' => [
[], $userCheckedPreference, false
];
yield 'cannot view protected variables, has not checked preference' => [
[], $userUncheckedPreference, false
];
}
/**
* @dataProvider provideCanViewProtectedVariableValues
*/
public function testCanViewProtectedVariableValues( array $rights, UserIdentityValue $user, bool $expected ) {
$performer = $this->mockUserAuthorityWithPermissions( $user, $rights );
$this->assertSame(
$expected,
$this->getPermMan()->canViewProtectedVariableValues( $performer )
);
}
public static function provideGetForbiddenVariables(): Generator {
yield 'cannot view, protected vars' => [
[
@ -292,6 +333,13 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
);
}
public function testGetProtectedVariables() {
$this->assertSame(
[ 'user_unnamed_ip' ],
$this->getPermMan()->getProtectedVariables()
);
}
public static function provideSimpleCases(): array {
return [
'not allowed' => [ false ],