Allow variables to be restricted by user right

Some exposed variables (eg. `user_ip`) used in filters are sensitive
and need to only be available to restricted groups of users.

Back-end changes:
- Add `AbuseFilterProtectedVariables` which defines what variables are
  protected by the new right `abusefilter-access-protected-vars`
- Add the concept of a `protected` variable, the use of which will
  denote the entire filter as protected via a flag on `af_hidden`

New UX features:
- Display changes to the protected status of filters on history and diff
  pages
- Check for protected variables and the right to see them in filter
  validation and don't allow a filter to be saved if it uses a variable
  that the user doesn't have access to
- Check for the right to view protected variables before allowing access
  and edits to existing filters that use them

Bug: T364465
Bug: T363906
Change-Id: I828bbb4015e87040f69a8e10c7888273c4f24dd3
This commit is contained in:
STran 2024-05-23 07:49:17 -07:00
parent ca23e9f06b
commit bf28dbce0e
35 changed files with 576 additions and 45 deletions

View file

@ -29,7 +29,8 @@
"abusefilter-hide-log",
"abusefilter-modify-global",
"abusefilter-modify-blocked-external-domains",
"abusefilter-bypass-blocked-external-domains"
"abusefilter-bypass-blocked-external-domains",
"abusefilter-access-protected-vars"
],
"GroupPermissions": {
"*": {
@ -575,6 +576,12 @@
"AbuseFilterEnableBlockedExternalDomain": {
"value": false,
"description": "Temporary config value to disable Special:BlockedExternalDomains"
},
"AbuseFilterProtectedVariables": {
"value": [
"user_unnamed_ip"
],
"description": "Array of variables that are be considered protected (limited access) and require the abusefilter-access-protected-vars right to use/view."
}
},
"load_composer_autoloader": true,

View file

@ -43,6 +43,7 @@
"action-abusefilter-revert": "revert all changes by a given abuse filter",
"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-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",
@ -143,6 +144,7 @@
"abusefilter-list-lastmodified": "Last modified",
"abusefilter-list-group": "Filter group",
"abusefilter-hidden": "Private",
"abusefilter-protected": "Protected",
"abusefilter-unhidden": "Public",
"abusefilter-enabled": "Enabled",
"abusefilter-deleted": "Deleted",
@ -264,6 +266,7 @@
"abusefilter-block-user": "block registered users",
"abusefilter-block-talk": "talk page blocked",
"abusefilter-edit-denied": "You may not view details of this filter, because it is hidden from public view.",
"abusefilter-edit-denied-protected-vars": "You may not view details of this filter because it uses protected variables and is hidden from public view.",
"abusefilter-edit-main": "Filter parameters",
"abusefilter-edit-done-subtitle": "Filter edited",
"abusefilter-edit-done": "[[Special:AbuseFilter/history/$1/diff/prev/$2|Your changes]] to [[Special:AbuseFilter/$1|filter $3]] have been saved.",
@ -271,6 +274,7 @@
"abusefilter-edit-missingfields": "The following fields are required and must be filled: $1",
"abusefilter-edit-deleting-enabled": "You cannot mark an active filter as deleted.",
"abusefilter-edit-restricted": "You cannot edit this filter, because it contains one or more restricted actions.\nPlease ask a user with permission to add restricted actions to make the change for you.",
"abusefilter-edit-protected-variable": "You cannot save this filter because you don't have permission to use the following variables: $1",
"abusefilter-edit-viewhistory": "View this filter's history",
"abusefilter-edit-history": "History:",
"abusefilter-edit-check": "Check syntax",
@ -439,6 +443,7 @@
"abusefilter-history": "Change history for Abuse Filter #$1",
"abusefilter-history-foruser": "Changes by $1",
"abusefilter-history-hidden": "Hidden",
"abusefilter-history-protected": "Protected",
"abusefilter-history-enabled": "Enabled",
"abusefilter-history-global": "Global",
"abusefilter-history-timestamp": "Time",
@ -613,5 +618,7 @@
"action-abusefilter-modify-blocked-external-domains": "create or modify what external domains are blocked from being linked",
"right-abusefilter-bypass-blocked-external-domains": "Bypass blocked external domains",
"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]]."
"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"
}

View file

@ -88,6 +88,7 @@
"action-abusefilter-revert": "{{doc-action|abusefilter-revert}}",
"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-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}}",
@ -188,6 +189,7 @@
"abusefilter-list-lastmodified": "Column header in abuse filter overview for the last modified timestamp for a filter.\n{{Identical|Last modified}}",
"abusefilter-list-group": "The filter group the edit filter is in.",
"abusefilter-hidden": "Abuse filter status.\n{{Identical|Private}}",
"abusefilter-protected": "Abuse filter status.\n{{Identical|Protected}}",
"abusefilter-unhidden": "Abuse filter status.\n{{Identical|Public}}",
"abusefilter-enabled": "Abuse filter status.\n{{Identical|Enabled}}",
"abusefilter-deleted": "Abuse filter status.\n{{Identical|Deleted}}",
@ -309,6 +311,7 @@
"abusefilter-block-user": "Specification for type of block 'Block' is a verb.",
"abusefilter-block-talk": "Specification for type of block. See {{msg-mw|Blocklist-nousertalk}}",
"abusefilter-edit-denied": "Text used when a user has to access to filter details.",
"abusefilter-edit-denied-protected-vars": "Text used when a user is denied access to filter details because they don't have the viewing right.",
"abusefilter-edit-main": "Fieldset legend for edit filter form.",
"abusefilter-edit-done-subtitle": "Page subtitle when as filter was edited and saved.",
"abusefilter-edit-done": "Text displayed to a user after editing a filter. Parameters:\n* $1 - a filter ID\n* $2 - the ID of the change itself\n* $3 - localized filter ID",
@ -316,6 +319,7 @@
"abusefilter-edit-missingfields": "Message to warn a user that a filter could not be edited for a given reason. Parameters:\n* $1 is a list of missing fields.",
"abusefilter-edit-deleting-enabled": "Message to warn a user that an active filter can't be marked as deleted",
"abusefilter-edit-restricted": "Message to warn a user that a filter could not be edited for a given reason.",
"abusefilter-edit-protected-variable": "Message to warn a user that a filter could not be edited for a given reason. Parameters:\n* $1 is the variable that raised the error.",
"abusefilter-edit-viewhistory": "Link description for link that leads to a revision overview for a filter.",
"abusefilter-edit-history": "Field label for {{msg-mw|abusefilter-edit-viewhistory}}.\n{{Identical|History}}",
"abusefilter-edit-check": "Button text for checking abuse filter syntax.\n\nUsed in {{msg-mw|Abusefilter-test-syntaxerr}}.",
@ -484,6 +488,7 @@
"abusefilter-history": "Used as page title.\n\n\"Change history\" is the \"history of changes\"\n\nParameters:\n* $1 - filter ID\n\nIf the filter ID is not specified, {{msg-mw|Abusefilter-filter-log}} will be used.",
"abusefilter-history-foruser": "Parameters:\n* $1 - a link to the changing user's page\n* $2 - (Optional) the plain text username",
"abusefilter-history-hidden": "{{Identical|Hidden}}",
"abusefilter-history-protected": "{{Identical|Protected}}",
"abusefilter-history-enabled": "{{Identical|Enabled}}",
"abusefilter-history-global": "{{Identical|Global}}",
"abusefilter-history-timestamp": "Used as table column header in history page of a filter.\n\nTranslate \"Time\" as \"Timestamp\" (time and date).\n{{Identical|Time}}",
@ -658,5 +663,7 @@
"action-abusefilter-modify-blocked-external-domains": "{{doc-action|abusefilter-modify-blocked-external-domains}}",
"right-abusefilter-bypass-blocked-external-domains": "{{doc-right|abusefilter-bypass-blocked-external-domains}}",
"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."
"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}}"
}

View file

@ -2,6 +2,7 @@
namespace MediaWiki\Extension\AbuseFilter;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\AbuseFilter\Filter\AbstractFilter;
use MediaWiki\Permissions\Authority;
@ -12,6 +13,24 @@ use MediaWiki\Permissions\Authority;
class AbuseFilterPermissionManager {
public const SERVICE_NAME = 'AbuseFilterPermissionManager';
public const CONSTRUCTOR_OPTIONS = [
'AbuseFilterProtectedVariables',
];
/**
* @var string[] Protected variables defined in config via AbuseFilterProtectedVariables
*/
private $protectedVariables;
/**
* @param ServiceOptions $options
*/
public function __construct(
ServiceOptions $options
) {
$this->protectedVariables = $options->get( 'AbuseFilterProtectedVariables' );
}
/**
* @param Authority $performer
* @return bool
@ -71,6 +90,44 @@ class AbuseFilterPermissionManager {
);
}
/**
* @param Authority $performer
* @return bool
*/
public function canViewProtectedVariables( Authority $performer ) {
$block = $performer->getBlock();
return (
!( $block && $block->isSitewide() ) &&
$performer->isAllowed( 'abusefilter-access-protected-vars' )
);
}
/**
* Check if the filter should be protected:
* - Return false if it uses no protected variables
* - Return true if it uses protected variables and the performer has view permissions
* - Return an array of used protected variables if the performer doesn't have
* view permissions
*
* @param Authority $performer
* @param string[] $usedVariables
* @return string[]|bool
*/
public function shouldProtectFilter( Authority $performer, $usedVariables ) {
$usedProtectedVariables = array_intersect( $usedVariables, $this->protectedVariables );
// Protected variables aren't used
if ( count( $usedProtectedVariables ) === 0 ) {
return false;
} else {
// Check for permissions if they are
if ( $this->canViewProtectedVariables( $performer ) ) {
return true;
} else {
return $usedProtectedVariables;
}
}
}
/**
* @param Authority $performer
* @return bool
@ -111,9 +168,24 @@ class AbuseFilterPermissionManager {
* @return bool
*/
public function canSeeLogDetailsForFilter( Authority $performer, int $privacyLevel ): bool {
if ( FilterUtils::isHidden( $privacyLevel ) ) {
return $this->canSeeLogDetails( $performer )
&& $this->canViewPrivateFiltersLogs( $performer );
if ( $privacyLevel ) {
$res = true;
if ( FilterUtils::isHidden( $privacyLevel ) ) {
if (
!$this->canSeeLogDetails( $performer ) ||
!$this->canViewPrivateFiltersLogs( $performer )
) {
$res = false;
}
}
if ( FilterUtils::isProtected( $privacyLevel ) ) {
if ( !$this->canViewProtectedVariables( $performer ) ) {
$res = false;
}
}
return $res;
}
return $this->canSeeLogDetails( $performer );

View file

@ -76,6 +76,7 @@ class QueryAbuseFilters extends ApiQueryBase {
$fld_time = isset( $prop['lastedittime'] );
$fld_status = isset( $prop['status'] );
$fld_private = isset( $prop['private'] );
$fld_protected = isset( $prop['protected'] );
$result = $this->getResult();
@ -125,11 +126,20 @@ class QueryAbuseFilters extends ApiQueryBase {
$this->getDb()->bitAnd( 'af_hidden', Flags::FILTER_HIDDEN ) . ' != 0',
isset( $show['private'] )
);
$this->addWhereIf(
$this->getDb()->bitAnd( 'af_hidden', Flags::FILTER_USES_PROTECTED_VARS ) . ' != 0',
isset( $show['!protected'] )
);
$this->addWhereIf(
$this->getDb()->bitAnd( 'af_hidden', Flags::FILTER_USES_PROTECTED_VARS ) . ' = 0',
isset( $show['!protected'] )
);
}
$res = $this->select( __METHOD__ );
$showhidden = $this->afPermManager->canViewPrivateFilters( $this->getAuthority() );
$showProtected = $this->afPermManager->canViewProtectedVariables( $this->getAuthority() );
$count = 0;
foreach ( $res as $row ) {
@ -146,7 +156,11 @@ class QueryAbuseFilters extends ApiQueryBase {
if ( $fld_desc ) {
$entry['description'] = $row->af_public_comments;
}
if ( $fld_pattern && ( !FilterUtils::isHidden( $row->af_hidden ) || $showhidden ) ) {
if (
$fld_pattern &&
( !FilterUtils::isHidden( $row->af_hidden ) || $showhidden ) &&
( !FilterUtils::isProtected( $row->af_hidden ) || $showProtected )
) {
$entry['pattern'] = $row->af_pattern;
}
if ( $fld_actions ) {
@ -155,7 +169,11 @@ class QueryAbuseFilters extends ApiQueryBase {
if ( $fld_hits ) {
$entry['hits'] = intval( $row->af_hit_count );
}
if ( $fld_comments && ( !FilterUtils::isHidden( $row->af_hidden ) || $showhidden ) ) {
if (
$fld_comments &&
( !FilterUtils::isHidden( $row->af_hidden ) || $showhidden ) &&
( !FilterUtils::isProtected( $row->af_hidden ) || $showProtected )
) {
$entry['comments'] = $row->af_comments;
}
if ( $fld_user ) {
@ -168,6 +186,9 @@ class QueryAbuseFilters extends ApiQueryBase {
if ( $fld_private && FilterUtils::isHidden( $row->af_hidden ) ) {
$entry['private'] = '';
}
if ( $fld_protected && FilterUtils::isProtected( $row->af_hidden ) ) {
$entry['protected'] = '';
}
if ( $fld_status ) {
if ( $row->af_enabled ) {
$entry['enabled'] = '';
@ -216,6 +237,8 @@ class QueryAbuseFilters extends ApiQueryBase {
'!deleted',
'private',
'!private',
'protected',
'!protected',
],
],
'limit' => [
@ -238,6 +261,7 @@ class QueryAbuseFilters extends ApiQueryBase {
'lastedittime',
'status',
'private',
'protected',
],
ParamValidator::PARAM_ISMULTI => true
]

View file

@ -144,8 +144,8 @@ class QueryAbuseLog extends ApiQueryBase {
try {
$privacyLevel = $lookup->getFilter( $filterID, $global )->getPrivacyLevel();
} catch ( CentralDBNotAvailableException $_ ) {
// Conservatively assume it's hidden, like in SpecialAbuseLog
$privacyLevel = Flags::FILTER_HIDDEN;
// Conservatively assume it's hidden and protected, like in SpecialAbuseLog
$privacyLevel = Flags::FILTER_HIDDEN & Flags::FILTER_USES_PROTECTED_VARS;
} catch ( FilterNotFoundException $_ ) {
$privacyLevel = Flags::FILTER_PUBLIC;
$foundInvalid = true;
@ -155,6 +155,11 @@ class QueryAbuseLog extends ApiQueryBase {
[ 'apierror-permissiondenied', $this->msg( 'action-abusefilter-log-private' ) ]
);
}
if ( Flags::FILTER_USES_PROTECTED_VARS & $privacyLevel ) {
$this->dieWithError(
[ 'apierror-permissiondenied', $this->msg( 'action-abusefilter-log-protected' ) ]
);
}
}
}

View file

@ -116,6 +116,13 @@ class AbstractFilter {
return $this->flags->getHidden();
}
/**
* @return bool
*/
public function isProtected(): bool {
return $this->flags->getProtected();
}
/**
* @return int
*/

View file

@ -12,6 +12,8 @@ class Flags {
private $deleted;
/** @var bool */
private $hidden;
/** @var bool */
private $protected;
/** @var int */
private $privacyLevel;
/** @var bool */
@ -19,6 +21,7 @@ class Flags {
public const FILTER_PUBLIC = 0b00;
public const FILTER_HIDDEN = 0b01;
public const FILTER_USES_PROTECTED_VARS = 0b10;
/**
* @param bool $enabled
@ -30,6 +33,7 @@ class Flags {
$this->enabled = $enabled;
$this->deleted = $deleted;
$this->hidden = (bool)( self::FILTER_HIDDEN & $hidden );
$this->protected = (bool)( self::FILTER_USES_PROTECTED_VARS & $hidden );
$this->privacyLevel = $hidden;
$this->global = $global;
}
@ -77,9 +81,25 @@ class Flags {
$this->updatePrivacyLevel();
}
/**
* @return bool
*/
public function getProtected(): bool {
return $this->protected;
}
/**
* @param bool $protected
*/
public function setProtected( bool $protected ): void {
$this->protected = $protected;
$this->updatePrivacyLevel();
}
private function updatePrivacyLevel() {
$hidden = $this->hidden ? self::FILTER_HIDDEN : 0;
$this->privacyLevel = $hidden;
$protected = $this->protected ? self::FILTER_USES_PROTECTED_VARS : 0;
$this->privacyLevel = $hidden | $protected;
}
/**

View file

@ -116,6 +116,13 @@ class MutableFilter extends Filter {
$this->flags->setHidden( $hidden );
}
/**
* @param bool $protected
*/
public function setProtected( bool $protected ): void {
$this->flags->setProtected( $protected );
}
/**
* @param bool $global
*/

View file

@ -391,7 +391,10 @@ class FilterLookup implements IDBAccessObject {
$hidden = in_array( 'hidden', $flags, true ) ?
Flags::FILTER_HIDDEN :
0;
return $hidden;
$protected = in_array( 'protected', $flags, true ) ?
Flags::FILTER_USES_PROTECTED_VARS :
0;
return $hidden | $protected;
}
/**

View file

@ -6,6 +6,8 @@ use ManualLogEntry;
use MediaWiki\Extension\AbuseFilter\ChangeTags\ChangeTagsManager;
use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
use MediaWiki\Extension\AbuseFilter\Filter\Filter;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
use MediaWiki\Extension\AbuseFilter\Filter\MutableFilter;
use MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseFilter;
use MediaWiki\Permissions\Authority;
use MediaWiki\Status\Status;
@ -104,6 +106,12 @@ class FilterStore {
return $validationStatus;
}
// Filter would have thrown an error if the user didn't have permission to use protected variables
// We only need to check if the filter is using protected variables and force-set the flag to protected
// TODO: T364485 will use a checkbox to set the protected flag. Remove this when that's implemented.
$newFilter = MutableFilter::newFromParentFilter( $newFilter );
$newFilter->setProtected( $this->filterValidator->usesProtectedVars( $newFilter ) );
// Check for non-changes
$differences = $this->filterCompare->compareVersions( $newFilter, $originalFilter );
if ( !$differences ) {
@ -113,7 +121,7 @@ class FilterStore {
// Everything went fine, so let's save the filter
$wasGlobal = $originalFilter->isGlobal();
[ $newID, $historyID ] = $this->doSaveFilter(
$performer->getUser(), $newFilter, $differences, $filterId, $wasGlobal );
$performer->getUser(), $newFilter, $originalFilter, $differences, $filterId, $wasGlobal );
return Status::newGood( [ $newID, $historyID ] );
}
@ -122,6 +130,7 @@ class FilterStore {
*
* @param UserIdentity $userIdentity
* @param Filter $newFilter
* @param Filter $originalFilter
* @param array $differences
* @param int|null $filterId
* @param bool $wasGlobal
@ -130,12 +139,13 @@ class FilterStore {
private function doSaveFilter(
UserIdentity $userIdentity,
Filter $newFilter,
Filter $originalFilter,
array $differences,
?int $filterId,
bool $wasGlobal
): array {
$dbw = $this->lbFactory->getPrimaryDatabase();
$newRow = $this->filterToDatabaseRow( $newFilter );
$newRow = $this->filterToDatabaseRow( $newFilter, $originalFilter );
// Set last modifier.
$newRow['af_timestamp'] = $dbw->timestamp();
@ -208,6 +218,9 @@ class FilterStore {
if ( FilterUtils::isHidden( $newRow['af_hidden'] ) ) {
$flags[] = 'hidden';
}
if ( FilterUtils::isProtected( $newRow['af_hidden'] ) ) {
$flags[] = 'protected';
}
if ( $newRow['af_enabled'] ) {
$flags[] = 'enabled';
}
@ -280,8 +293,16 @@ class FilterStore {
* @param Filter $filter
* @return array
*/
private function filterToDatabaseRow( Filter $filter ): array {
private function filterToDatabaseRow( Filter $filter, Filter $originalFilter ): array {
// T67807: integer 1's & 0's might be better understood than booleans
// If the filter is already protected, it must remain protected even if
// the current filter doesn't use a protected variable anymore
$privacyLevel = $filter->getPrivacyLevel();
if ( $originalFilter->isProtected() ) {
$privacyLevel |= Flags::FILTER_USES_PROTECTED_VARS;
}
return [
'af_id' => $filter->getID(),
'af_pattern' => $filter->getRules(),
@ -291,7 +312,7 @@ class FilterStore {
'af_actions' => implode( ',', $filter->getActionsNames() ),
'af_enabled' => (int)$filter->isEnabled(),
'af_deleted' => (int)$filter->isDeleted(),
'af_hidden' => $filter->getPrivacyLevel(),
'af_hidden' => $privacyLevel,
'af_global' => (int)$filter->isGlobal(),
'af_timestamp' => $filter->getTimestamp(),
'af_hit_count' => $filter->getHitCount(),

View file

@ -17,4 +17,14 @@ class FilterUtils {
public static function isHidden( int $privacyLevel ) {
return (bool)( Flags::FILTER_HIDDEN & $privacyLevel );
}
/**
* Given a bitmask, return if the protected flag is set
*
* @param int $privacyLevel
* @return bool
*/
public static function isProtected( int $privacyLevel ) {
return (bool)( Flags::FILTER_USES_PROTECTED_VARS & $privacyLevel );
}
}

View file

@ -19,7 +19,8 @@ class FilterValidator {
public const CONSTRUCTOR_OPTIONS = [
'AbuseFilterValidGroups',
'AbuseFilterActionRestrictions'
'AbuseFilterActionRestrictions',
'AbuseFilterProtectedVariables',
];
/** @var ChangeTagValidator */
@ -37,6 +38,11 @@ class FilterValidator {
/** @var string[] */
private $validGroups;
/**
* @var string[] Protected variables defined in config via AbuseFilterProtectedVariables
*/
private $protectedVariables;
/**
* @param ChangeTagValidator $changeTagValidator
* @param RuleCheckerFactory $ruleCheckerFactory
@ -54,6 +60,7 @@ class FilterValidator {
$this->permManager = $permManager;
$this->restrictedActions = array_keys( array_filter( $options->get( 'AbuseFilterActionRestrictions' ) ) );
$this->validGroups = $options->get( 'AbuseFilterValidGroups' );
$this->protectedVariables = $options->get( 'AbuseFilterProtectedVariables' );
}
/**
@ -102,6 +109,11 @@ class FilterValidator {
}
}
$protectedVarsPermissionStatus = $this->checkCanViewProtectedVariables( $performer, $newFilter );
if ( !$protectedVarsPermissionStatus->isGood() ) {
return $protectedVarsPermissionStatus;
}
$globalPermStatus = $this->checkGlobalFilterEditPermission( $performer, $newFilter, $originalFilter );
if ( !$globalPermStatus->isGood() ) {
return $globalPermStatus;
@ -345,6 +357,37 @@ class FilterValidator {
return $ret;
}
/**
* @param Authority $performer
* @param AbstractFilter $filter
* @return Status
*/
public function checkCanViewProtectedVariables( Authority $performer, AbstractFilter $filter ): Status {
$ret = Status::newGood();
$ruleChecker = $this->ruleCheckerFactory->newRuleChecker();
$usedVars = $ruleChecker->getUsedVars( $filter->getRules() );
$missingRights = $this->permManager->shouldProtectFilter( $performer, $usedVars );
if ( is_array( $missingRights ) ) {
$ret->error( 'abusefilter-edit-protected-variable', Message::listParam( $missingRights ) );
}
return $ret;
}
/**
* TODO: Remove this function when T364485 is implemented, as it makes this function obselete
* This is a stop-gap function used to check if a filter should be marked as protected
*
* @param AbstractFilter $filter
* @return bool
*/
public function usesProtectedVars( AbstractFilter $filter ): bool {
$ruleChecker = $this->ruleCheckerFactory->newRuleChecker();
$usedVariables = (array)$ruleChecker->getUsedVars( $filter->getRules() );
$usedProtectedVariables = array_intersect( $usedVariables, $this->protectedVariables );
return count( $usedProtectedVariables ) > 0;
}
/**
* @param AbstractFilter $filter
* @return Status

View file

@ -41,6 +41,9 @@ class AbuseFilterHistoryPager extends TablePager {
/** @var bool */
private $canViewPrivateFilters;
/** @var bool */
private $canViewProtectedVars;
/**
* @param IContextSource $context
* @param LinkRenderer $linkRenderer
@ -50,6 +53,7 @@ class AbuseFilterHistoryPager extends TablePager {
* @param ?int $filter
* @param ?string $user User name
* @param bool $canViewPrivateFilters
* @param bool $canViewProtectedVars
*/
public function __construct(
IContextSource $context,
@ -59,7 +63,8 @@ class AbuseFilterHistoryPager extends TablePager {
SpecsFormatter $specsFormatter,
?int $filter,
?string $user,
bool $canViewPrivateFilters = false
bool $canViewPrivateFilters = false,
bool $canViewProtectedVars = false
) {
// needed by parent's constructor call
$this->filter = $filter;
@ -69,6 +74,7 @@ class AbuseFilterHistoryPager extends TablePager {
$this->specsFormatter = $specsFormatter;
$this->user = $user;
$this->canViewPrivateFilters = $canViewPrivateFilters;
$this->canViewProtectedVars = $canViewProtectedVars;
$this->mDefaultDirection = true;
}
@ -157,17 +163,27 @@ class AbuseFilterHistoryPager extends TablePager {
break;
case 'afh_id':
// Set a link to a diff with the previous version if this isn't the first edit to the filter.
// Like in AbuseFilterViewDiff, don't show it if the user cannot see private filters and any
// of the versions is hidden.
// Like in AbuseFilterViewDiff, don't show it if:
// - the user cannot see private filters and any of the versions is hidden
// - the user cannot see protected variables and any of the versions is protected
$formatted = '';
if ( $this->filterLookup->getFirstFilterVersionID( $row->afh_filter ) !== (int)$value ) {
// @todo Should we also hide actions?
$prevFilter = $this->filterLookup->getClosestVersion(
$row->afh_id, $row->afh_filter, FilterLookup::DIR_PREV );
if ( $this->canViewPrivateFilters ||
if (
( $this->canViewPrivateFilters ||
(
!in_array( 'hidden', explode( ',', $row->afh_flags ) ) &&
!$prevFilter->isHidden()
)
) &&
(
!in_array( 'hidden', explode( ',', $row->afh_flags ) ) &&
!$prevFilter->isHidden()
$this->canViewProtectedVars ||
(
!in_array( 'protected', explode( ',', $row->afh_flags ) ) &&
!$prevFilter->isProtected()
)
)
) {
$title = SpecialAbuseFilter::getTitleForSubpage(
@ -233,6 +249,11 @@ class AbuseFilterHistoryPager extends TablePager {
$info['conds'][] = $this->mDb->bitAnd( 'af_hidden', Flags::FILTER_HIDDEN ) . ' = 0';
}
if ( !$this->canViewProtectedVars ) {
// Hide data the user can't see.
$info['conds'][] = 'af_hidden & ' . Flags::FILTER_USES_PROTECTED_VARS . ' = 0';
}
return $info;
}

View file

@ -291,6 +291,9 @@ class AbuseFilterPager extends TablePager {
if ( FilterUtils::isHidden( (int)$value ) ) {
$flagMsgs[] = $this->msg( 'abusefilter-hidden' )->parse();
}
if ( FilterUtils::isProtected( (int)$value ) ) {
$flagMsgs[] = $this->msg( 'abusefilter-protected' )->parse();
}
if ( !$flagMsgs ) {
return $this->msg( 'abusefilter-unhidden' )->parse();
}

View file

@ -183,9 +183,9 @@ class AbuseLogPager extends ReverseChronologicalPager {
$privacyLevel = $filterObj->getPrivacyLevel();
} catch ( CentralDBNotAvailableException $_ ) {
$escaped_comments = $this->msg( 'abusefilter-log-description-not-available' )->escaped();
// either hide all filters, including not hidden, or show all, including hidden
// either hide all filters, including not hidden/protected, or show all, including hidden/protected
// we choose the former
$privacyLevel = Flags::FILTER_HIDDEN;
$privacyLevel = Flags::FILTER_HIDDEN & Flags::FILTER_USES_PROTECTED_VARS;
}
} else {
$escaped_comments = Sanitizer::escapeHtmlAllowEntities(

View file

@ -64,8 +64,8 @@ class GlobalAbuseFilterPager extends AbuseFilterPager {
return $lang->commaList( $statuses );
case 'af_hit_count':
// If the rule is hidden, don't show it, even to privileged local admins
if ( FilterUtils::isHidden( $row->af_hidden ) ) {
// If the rule is hidden or protected, don't show it, even to privileged local admins
if ( FilterUtils::isHidden( $row->af_hidden ) || FilterUtils::isProtected( $row->af_hidden ) ) {
return '';
}
return $this->msg( 'abusefilter-hitcount' )->numParams( $value )->parse();

View file

@ -189,6 +189,11 @@ class FilterEvaluator {
*/
private $equivset;
/**
* @var array AFPToken::TID values found during node evaluation
*/
private $usedVars = [];
/**
* Create a new instance
*
@ -287,6 +292,7 @@ class FilterEvaluator {
$this->mAllowShort = true;
$this->mFilter = null;
$this->warnings = [];
$this->usedVars = [];
}
/**
@ -438,6 +444,20 @@ class FilterEvaluator {
return $ret;
}
/**
* Parse a filter and return the variables used.
* All variables are AFPToken::TID and are found during the node stepthrough in evaluation
* and saved to self::usedVars to be returned to the caller in this function.
*
* @param string $filter
* @return array
*/
public function getUsedVars( $filter ) {
$this->resetState();
$this->evaluateExpression( $filter );
return array_unique( $this->usedVars );
}
/**
* Evaluate the value of the specified AST node.
*
@ -812,6 +832,8 @@ class FilterEvaluator {
// allowing it to result in DUNDEFINED.
$allowMissingVariables = !$this->varExists( $var ) || $this->allowMissingVariables;
array_push( $this->usedVars, $var );
// It's a built-in, non-disabled variable (either set or unset), or a set custom variable
$flags = $allowMissingVariables
? VariablesManager::GET_LAX

View file

@ -71,8 +71,13 @@ return [
);
},
PermManager::SERVICE_NAME => static function ( MediaWikiServices $services ): PermManager {
// No longer has any dependencies
return new PermManager();
return new PermManager(
new ServiceOptions(
PermManager::CONSTRUCTOR_OPTIONS,
$services->getMainConfig(),
[ 'AbuseFilterProtectedVariables' => [] ]
)
);
},
ChangeTagger::SERVICE_NAME => static function ( MediaWikiServices $services ): ChangeTagger {
return new ChangeTagger(
@ -166,7 +171,8 @@ return [
$services->get( PermManager::SERVICE_NAME ),
new ServiceOptions(
FilterValidator::CONSTRUCTOR_OPTIONS,
$services->getMainConfig()
$services->getMainConfig(),
[ 'AbuseFilterProtectedVariables' => [] ]
)
);
},

View file

@ -723,8 +723,8 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
$privacyLevel = AbuseFilterServices::getFilterLookup()->getFilter( $filterID, $global )
->getPrivacyLevel();
} catch ( CentralDBNotAvailableException $_ ) {
// Conservatively assume that it's hidden, like in formatRow
$privacyLevel = Flags::FILTER_HIDDEN;
// Conservatively assume that it's hidden and protected, like in formatRow
$privacyLevel = Flags::FILTER_HIDDEN & Flags::FILTER_USES_PROTECTED_VARS;
}
} else {
$privacyLevel = $row->af_hidden;

View file

@ -147,6 +147,7 @@ class SpecsFormatter {
'enabled' => $filter->isEnabled(),
'deleted' => $filter->isDeleted(),
'hidden' => $filter->isHidden(),
'protected' => $filter->isProtected(),
'global' => $filter->isGlobal()
] );
$flagsDisplay = [];

View file

@ -157,6 +157,13 @@ class AbuseFilterViewDiff extends AbuseFilterView {
return false;
}
if ( !$this->afPermManager->canViewProtectedVariables( $this->getAuthority() ) &&
( $this->oldVersion->isProtected() || $this->newVersion->isProtected() )
) {
$this->getOutput()->addWikiMsg( 'abusefilter-history-error-protected' );
return false;
}
try {
$this->nextHistoryId = $this->filterLookup->getClosestVersion(
$this->newVersion->getHistoryID(),

View file

@ -299,6 +299,12 @@ class AbuseFilterViewEdit extends AbuseFilterView {
return;
}
// Filters that use protected variables should always be hidden from public view
if ( $filterObj->isProtected() && !$this->afPermManager->canViewProtectedVariables( $user ) ) {
$out->addHTML( $this->msg( 'abusefilter-edit-denied-protected-vars' )->escaped() );
return;
}
if ( $isCreatingNewFilter ) {
$title = $this->msg( 'abusefilter-add' );
} elseif ( $this->afPermManager->canEditFilter( $user, $filterObj ) ) {

View file

@ -268,8 +268,8 @@ class AbuseFilterViewExamine extends AbuseFilterView {
try {
$privacyLevel = $this->filterLookup->getFilter( $row->afl_filter_id, $row->afl_global )->getPrivacyLevel();
} catch ( CentralDBNotAvailableException $_ ) {
// Conservatively assume that it's hidden, like in SpecialAbuseLog
$privacyLevel = Flags::FILTER_HIDDEN;
// Conservatively assume that it's hidden and protected, like in SpecialAbuseLog
$privacyLevel = Flags::FILTER_HIDDEN & Flags::FILTER_USES_PROTECTED_VARS;
}
if ( !$this->afPermManager->canSeeLogDetailsForFilter( $performer, $privacyLevel ) ) {
$out->addWikiMsg( 'abusefilter-log-cannot-see-details' );

View file

@ -71,6 +71,7 @@ class AbuseFilterViewHistory extends AbuseFilterView {
$out->enableOOUI();
$filter = $this->getRequest()->getIntOrNull( 'filter' ) ?: $this->filter;
$canViewPrivate = $this->afPermManager->canViewPrivateFilters( $this->getAuthority() );
$canViewProtectedVars = $this->afPermManager->canViewProtectedVariables( $this->getAuthority() );
if ( $filter ) {
try {
@ -82,6 +83,10 @@ class AbuseFilterViewHistory extends AbuseFilterView {
$out->addWikiMsg( 'abusefilter-history-error-hidden' );
return;
}
if ( isset( $filterObj ) && $filterObj->isProtected() && !$canViewProtectedVars ) {
$out->addWikiMsg( 'abusefilter-history-error-hidden' );
return;
}
}
if ( $filter ) {
@ -162,7 +167,8 @@ class AbuseFilterViewHistory extends AbuseFilterView {
$this->specsFormatter,
$filter,
$user,
$canViewPrivate
$canViewPrivate,
$canViewProtectedVars
);
$out->addParserOutputContent( $pager->getFullOutput() );

View file

@ -44,10 +44,11 @@ class FilterStoreTest extends MediaWikiIntegrationTestCase {
$row = self::DEFAULT_VALUES;
$row['timestamp'] = $this->db->timestamp( $row['timestamp'] );
$filter = $this->getFilterFromSpecs( [ 'id' => $id ] + $row );
$oldFilter = MutableFilter::newDefault();
// Use some black magic to bypass checks
/** @var FilterStore $filterStore */
$filterStore = TestingAccessWrapper::newFromObject( AbuseFilterServices::getFilterStore() );
$row = $filterStore->filterToDatabaseRow( $filter );
$row = $filterStore->filterToDatabaseRow( $filter, $oldFilter );
$row += AbuseFilterServices::getActorMigration()->getInsertValues(
$this->db,
'af_user',
@ -172,4 +173,35 @@ class FilterStoreTest extends MediaWikiIntegrationTestCase {
$this->assertStatusGood( $status );
$this->assertFalse( $status->getValue(), 'Status value should be false' );
}
public function testSaveFilter_usesProtectedVars() {
$row = [
'id' => '2',
'rules' => "ip_in_range( user_unnamed_ip, '1.2.3.4' )",
'name' => 'Mock filter with protected variable used'
];
$origFilter = MutableFilter::newDefault();
$newFilter = $this->getFilterFromSpecs( $row );
// Try to save filter without right to use protected variables
$user = $this->getTestUser()->getUser();
$status = AbuseFilterServices::getFilterStore()->saveFilter( $user, $row['id'], $newFilter, $origFilter );
$expectedError = 'abusefilter-edit-protected-variable';
$this->assertStatusWarning( $expectedError, $status );
// Save filter with right, with 'protected' flag enabled
$this->overrideUserPermissions( $user, [ 'abusefilter-access-protected-vars', 'abusefilter-modify' ] );
$row = [
'id' => '3',
'rules' => "ip_in_range( user_unnamed_ip, '1.2.3.4' )",
'name' => 'Mock filter with protected variable used',
'hidden' => Flags::FILTER_USES_PROTECTED_VARS,
];
$newFilter = $this->getFilterFromSpecs( $row );
$status = AbuseFilterServices::getFilterStore()->saveFilter(
$user, $row['id'], $newFilter, $origFilter
);
$this->assertStatusGood( $status );
}
}

View file

@ -31,7 +31,8 @@ class FilterValidatorTest extends MediaWikiIntegrationTestCase {
FilterValidator::CONSTRUCTOR_OPTIONS,
[
'AbuseFilterActionRestrictions' => [],
'AbuseFilterValidGroups' => [ 'default' ]
'AbuseFilterValidGroups' => [ 'default' ],
'AbuseFilterProtectedVariables' => [],
]
)
);

View file

@ -4,6 +4,7 @@ namespace MediaWiki\Extension\AbuseFilter\Tests\Unit;
use Generator;
use MediaWiki\Block\DatabaseBlock;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\Filter\AbstractFilter;
use MediaWiki\Extension\AbuseFilter\Filter\Flags;
@ -18,8 +19,14 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
use MockAuthorityTrait;
private function getPermMan(): AbuseFilterPermissionManager {
// No longer has any dependencies
return new AbuseFilterPermissionManager();
return new AbuseFilterPermissionManager(
new ServiceOptions(
AbuseFilterPermissionManager::CONSTRUCTOR_OPTIONS,
[
'AbuseFilterProtectedVariables' => [ 'user_unnamed_ip' ]
]
)
);
}
public function provideCanEdit(): Generator {
@ -166,14 +173,94 @@ class AbuseFilterPermissionManagerTest extends MediaWikiUnitTestCase {
public static function provideCanSeeLogDetailsForFilter(): Generator {
$details = [ 0 => 'abusefilter-log-detail' ];
$private = [ 1 => 'abusefilter-log-private' ];
$protected = [ 2 => 'abusefilter-access-protected-vars' ];
yield 'filter hidden, not privileged' => [ Flags::FILTER_HIDDEN, [], false ];
yield 'filter hidden, details only' => [ Flags::FILTER_HIDDEN, $details, false ];
yield 'filter hidden, private logs only' => [ Flags::FILTER_HIDDEN, $private, false ];
yield 'filter hidden, details and private logs' => [ Flags::FILTER_HIDDEN, $details + $private, true ];
yield 'filter protected, not privileged' => [ Flags::FILTER_USES_PROTECTED_VARS, [], false ];
yield 'filter protected, privileged' => [ Flags::FILTER_USES_PROTECTED_VARS, $protected, true ];
yield 'filter hidden and protected, details and private only' => [
Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS, $details + $private, false
];
yield 'filter hidden and protected, protected only' => [
Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS, $protected, false
];
yield 'filter visible, not privileged' => [ Flags::FILTER_PUBLIC, [], false ];
yield 'filter visible, privileged' => [ Flags::FILTER_PUBLIC, $details, true ];
}
public function provideCanViewProtectedVariables(): Generator {
$block = $this->createMock( DatabaseBlock::class );
$block->method( 'isSiteWide' )->willReturn( true );
yield 'not privileged, blocked' => [ $block, [], false ];
yield 'not privileged, not blocked' => [ null, [], false ];
yield 'has right, blocked' => [ $block, [ 'abusefilter-access-protected-vars' ], false ];
yield 'has right, not blocked' => [ null, [ 'abusefilter-access-protected-vars' ], true ];
}
/**
* @dataProvider provideCanViewProtectedVariables
*/
public function testCanViewProtectedVariables( ?DatabaseBlock $block, array $rights, bool $expected ) {
if ( $block !== null ) {
$performer = $this->mockUserAuthorityWithBlock(
$this->mockRegisteredUltimateAuthority()->getUser(),
$block,
$rights
);
} else {
$performer = $this->mockRegisteredAuthorityWithPermissions( $rights );
}
$this->assertSame(
$expected,
$this->getPermMan()->canViewProtectedVariables( $performer )
);
}
public static function provideShouldProtectFilter(): Generator {
yield 'cannot view, protected vars' => [
[
'rights' => [],
'usedVars' => [ 'user_unnamed_ip' ]
],
[ 'user_unnamed_ip' ]
];
yield 'cannot view, no protected vars' => [
[
'rights' => [],
'usedVars' => []
],
false
];
yield 'can view, protected vars' => [
[
'rights' => [ 'abusefilter-access-protected-vars' ],
'usedVars' => [ 'user_unnamed_ip' ]
],
true
];
yield 'can view, no protected vars' => [
[
'rights' => [ 'abusefilter-access-protected-vars' ],
'usedVars' => []
],
false
];
}
/**
* @dataProvider provideShouldProtectFilter
*/
public function testShouldProtectFilter( array $data, $expected ) {
$performer = $this->mockRegisteredAuthorityWithPermissions( $data[ 'rights' ] );
$this->assertSame(
$expected,
$this->getPermMan()->shouldProtectFilter( $performer, $data[ 'usedVars' ] )
);
}
/**
* @param int $privacyLevel
* @param array $rights

View file

@ -47,7 +47,7 @@ class AbstractFilterTest extends MediaWikiUnitTestCase {
$group = 'group';
$enabled = true;
$deleted = false;
$privacyLevel = Flags::FILTER_HIDDEN;
$privacyLevel = Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS;
$global = false;
$filter = new AbstractFilter(
new Specs( $rules, $comments, $name, $actionsNames, $group ),
@ -63,6 +63,7 @@ class AbstractFilterTest extends MediaWikiUnitTestCase {
$this->assertSame( $enabled, $filter->isEnabled(), 'enabled' );
$this->assertSame( $deleted, $filter->isDeleted(), 'deleted' );
$this->assertSame( true, $filter->isHidden(), 'hidden' );
$this->assertSame( true, $filter->isProtected(), 'uses protected vars' );
$this->assertSame( $privacyLevel, $filter->getPrivacyLevel(), 'privacy level' );
$this->assertSame( $global, $filter->isGlobal(), 'global' );
}

View file

@ -14,13 +14,14 @@ class FlagsTest extends MediaWikiUnitTestCase {
public function testGetters() {
$enabled = true;
$deleted = false;
$privacyLevel = Flags::FILTER_HIDDEN;
$privacyLevel = Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS;
$global = false;
$flags = new Flags( $enabled, $deleted, $privacyLevel, $global );
$this->assertSame( $enabled, $flags->getEnabled(), 'enabled' );
$this->assertSame( $deleted, $flags->getDeleted(), 'deleted' );
$this->assertSame( true, $flags->getHidden(), 'hidden' );
$this->assertSame( true, $flags->getProtected(), 'protected variables' );
$this->assertSame( $privacyLevel, $flags->getPrivacyLevel(), 'privacy level' );
$this->assertSame( $global, $flags->getGlobal(), 'global' );
}
@ -32,7 +33,7 @@ class FlagsTest extends MediaWikiUnitTestCase {
* @dataProvider provideSetters
*/
public function testSetters( $value, string $setter, string $getter ) {
$flags = new Flags( true, true, Flags::FILTER_HIDDEN, true );
$flags = new Flags( true, true, Flags::FILTER_HIDDEN | Flags::FILTER_USES_PROTECTED_VARS, true );
$flags->$setter( $value );
$this->assertSame( $value, $flags->$getter() );
@ -46,6 +47,7 @@ class FlagsTest extends MediaWikiUnitTestCase {
'enabled' => [ true, 'setEnabled', 'getEnabled' ],
'deleted' => [ false, 'setDeleted', 'getDeleted' ],
'hidden' => [ true, 'setHidden', 'getHidden' ],
'protected' => [ true, 'setProtected', 'getProtected' ],
'global' => [ false, 'setGlobal', 'getGlobal' ],
];
}

View file

@ -49,6 +49,7 @@ class MutableFilterTest extends MediaWikiUnitTestCase {
'enabled' => [ true, 'setEnabled', 'isEnabled' ],
'deleted' => [ false, 'setDeleted', 'isDeleted' ],
'hidden' => [ true, 'setHidden', 'isHidden' ],
'protected' => [ true, 'setProtected', 'isProtected' ],
'global' => [ false, 'setGlobal', 'isGlobal' ],
'actions' => [ [ 'foo' => [] ], 'setActions', 'getActions' ],
'user ID' => [ 163, 'setUserID', 'getUserID' ],

View file

@ -168,7 +168,7 @@ class FilterLookupTest extends MediaWikiUnitTestCase {
foreach ( $filters as $filter ) {
$flags = [];
foreach ( [ 'enabled', 'deleted', 'hidden', 'global' ] as $flag ) {
foreach ( [ 'enabled', 'deleted', 'hidden', 'protected', 'global' ] as $flag ) {
$method = 'is' . ucfirst( $flag );
if ( $filter->$method() ) {
$flags[] = $flag;

View file

@ -16,6 +16,7 @@ use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerFactory;
use MediaWiki\Permissions\Authority;
use MediaWiki\Status\Status;
use MediaWiki\Tests\Unit\Permissions\MockAuthorityTrait;
use MediaWikiUnitTestCase;
use PHPUnit\Framework\MockObject\MockObject;
@ -26,6 +27,7 @@ use PHPUnit\Framework\MockObject\MockObject;
* @covers \MediaWiki\Extension\AbuseFilter\FilterValidator
*/
class FilterValidatorTest extends MediaWikiUnitTestCase {
use MockAuthorityTrait;
/**
* @param AbuseFilterPermissionManager|null $permissionManager
@ -45,6 +47,10 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
$ruleChecker->method( 'checkSyntax' )->willReturn(
new ParserStatus( null, [], 1 )
);
$ruleChecker->method( 'getUsedVars' )->willReturnMap( [
[ 'user_unnamed_ip', [ 'user_unnamed_ip' ] ],
[ 'user_name', [] ]
] );
}
$checkerFactory = $this->createMock( RuleCheckerFactory::class );
$checkerFactory->method( 'newRuleChecker' )->willReturn( $ruleChecker );
@ -60,7 +66,8 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
FilterValidator::CONSTRUCTOR_OPTIONS,
[
'AbuseFilterActionRestrictions' => array_fill_keys( $restrictions, true ),
'AbuseFilterValidGroups' => $validFilterGroups
'AbuseFilterValidGroups' => $validFilterGroups,
'AbuseFilterProtectedVariables' => [ 'user_unnamed_ip' ],
]
)
);
@ -312,6 +319,86 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
[ $unrestricted, $restricted, $restrictions, $canModifyRestrictedPM, null ];
}
public function testUsesProtectedVars() {
$filterProtectedVarsTrue = $this->createMock( AbstractFilter::class );
$filterProtectedVarsTrue->method( 'getRules' )->willReturn( 'user_unnamed_ip' );
$this->assertTrue(
$this->getFilterValidator()->usesProtectedVars( $filterProtectedVarsTrue )
);
$filterProtectedVarsFalse = $this->createMock( AbstractFilter::class );
$filterProtectedVarsFalse->method( 'getRules' )->willReturn( 'user_name' );
$this->assertFalse(
$this->getFilterValidator()->usesProtectedVars( $filterProtectedVarsFalse )
);
}
/**
* @dataProvider provideCheckCanViewProtectedVariables
*/
public function testCheckCanViewProtectedVariables( $data ) {
$performer = $this->mockRegisteredAuthorityWithPermissions( $data[ 'rights' ] );
$permManager = $this->createMock( AbuseFilterPermissionManager::class );
$permManager->method( 'shouldProtectFilter' )->willReturn( false );
$filter = $this->createMock( AbstractFilter::class );
$filter->method( 'getRules' )->willReturn( $data[ 'rules' ] );
$this->assertStatusGood( $this->getFilterValidator( $permManager )
->checkCanViewProtectedVariables( $performer, $filter )
);
}
/**
* @dataProvider provideCheckCanViewProtectedVariablesError
*/
public function testCheckCanViewProtectedVariablesError( $data ) {
$performer = $this->mockRegisteredAuthorityWithPermissions( $data[ 'rights' ] );
$permManager = $this->createMock( AbuseFilterPermissionManager::class );
$permManager->method( 'shouldProtectFilter' )->willReturn( [ 'user_unnamed_ip' ] );
$filter = $this->createMock( AbstractFilter::class );
$filter->method( 'getRules' )->willReturn( $data[ 'rules' ] );
$this->assertStatusMessageParams(
'abusefilter-edit-protected-variable',
$this->getFilterValidator( $permManager )->checkCanViewProtectedVariables( $performer, $filter )
);
}
public static function provideCheckCanViewProtectedVariables(): array {
return [
'cannot view, no protected vars' => [
[
'rights' => [],
'rules' => 'user_name'
],
0
],
'can view, protected vars' => [
[
'rights' => [ 'abusefilter-access-protected-vars' ],
'rules' => 'user_unnamed_ip'
],
0
],
'can view, no protected vars' => [
[
'rights' => [ 'abusefilter-access-protected-vars' ],
'rules' => 'user_name'
],
0
]
];
}
public static function provideCheckCanViewProtectedVariablesError(): array {
return [
'cannot view, protected vars' => [
[
'rights' => [],
'rules' => 'user_unnamed_ip'
]
],
];
}
public function testCheckAllTags_noTags() {
$this->assertStatusMessageParams( 'tags-create-no-name', $this->getFilterValidator()->checkAllTags( [] ) );
}

View file

@ -1162,4 +1162,15 @@ class ParserTest extends ParserTestCase {
$wrapper = TestingAccessWrapper::newFromObject( $parser );
$this->assertSame( $vars, $wrapper->mVariables );
}
/**
* @covers \MediaWiki\Extension\AbuseFilter\Parser\FilterEvaluator
*/
public function testGetUsedVars() {
$parser = $this->getParser();
/** @var FilterEvaluator $wrapper */
$wrapper = TestingAccessWrapper::newFromObject( $parser );
$this->assertSame( [ 'user_name' ], $wrapper->getUsedVars( 'ip_in_range( user_name, "1.2.3.4" )' ) );
$this->assertSame( [ 'user_name' ], $wrapper->getUsedVars( 'ip_in_range( USER_NAME, "1.2.3.4" )' ) );
}
}

View file

@ -160,7 +160,11 @@ class SpecsFormatterTest extends MediaWikiUnitTestCase {
$multiple = MutableFilter::newDefault();
$multiple->setHidden( true );
yield 'multiple' => [ $multiple, 'abusefilter-history-enabled,abusefilter-history-hidden' ];
$multiple->setProtected( true );
yield 'multiple' => [
$multiple,
'abusefilter-history-enabled,abusefilter-history-hidden,abusefilter-history-protected'
];
}
/**