wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-11-23 13:46:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improve workflow for protecting filters with protected variables

Browse source 
Why:

* Protected variables were introduced to support temporary accounts
  so that temporary users could be filtered based on their IP address.
* Filters that use protected variables are protected in order to
  preserve privacy. This can't be undone.
* The current workflow for protecting filters is to have a 'protect'
  checkbox that can be checked for any filter, even one without
  protected variables, to the discretion of the editor. This has
  led to mistakes (see T37765).

What:

* Do not show the 'protect' checkbox on the form by default. Instead,
  show it when saving a filter with protected variables, after form
  submission. The user must check it to save the filter.
* Still show the checkbox in a disabled state with a warning message
  if a filter is already protected, so that the editor can easily see
  it is protected.

Bug: T377765
Change-Id: Ie5c94ac1399860ccdca4482508dd37ff07309764
This commit is contained in:
Thalia 2024-11-13 12:10:20 +00:00
parent 6a14ea54fb
commit 8de5b4e1aa
3 changed files with 161 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
i18n/en.json
View file

@ -216,7 +216,7 @@
"abusefilter-edit-enabled": "Enable this filter",
"abusefilter-edit-deleted": "Mark as deleted",
"abusefilter-edit-hidden": "Hide details of this filter from public view",
"abusefilter-edit-protected": "Enable the use of [https://www.mediawiki.org/wiki/Special:MyLanguage/Extension:AbuseFilter/Rules_format#Protected_variables protected variables] in this filter",
"abusefilter-edit-protected": "<strong>Enable the use of [https://www.mediawiki.org/wiki/Special:MyLanguage/Extension:AbuseFilter/Rules_format#Protected_variables protected variables] in this filter</strong>",
"abusefilter-edit-protected-variable-already-protected": "Details of this filter are hidden from users who cannot see protected variables",
"abusefilter-edit-protected-help-message": "Details of this filter will be hidden from users who cannot see protected variables. This action is permanent and cannot be undone.",
"abusefilter-edit-global": "Global filter",

19
includes/View/AbuseFilterViewEdit.php
View file

@ -429,9 +429,21 @@ class AbuseFilterViewEdit extends AbuseFilterView {
] );
// Build checkboxes
$checkboxes = [ 'hidden', 'enabled', 'protected', 'deleted' ];
$checkboxes = [ 'hidden', 'enabled', 'deleted' ];
$flags = '';
// Show the 'protected' check box either to indicate that the filter is protected, or
// to allow a user to protect the filter, if the filter needs to be protected.
if (
$filterObj->isProtected() ||
(
$status !== null &&
$status->hasMessage( 'abusefilter-edit-protected-variable-not-protected' )
)
) {
$checkboxes[] = 'protected';
}
if ( $this->getConfig()->get( 'AbuseFilterIsCentral' ) ) {
$checkboxes[] = 'global';
}
@ -488,10 +500,7 @@ class AbuseFilterViewEdit extends AbuseFilterView {
}
if ( $checkboxId == 'protected' ) {
if ( !$this->afPermManager->canViewProtectedVariables( $user ) ) {
$checkboxAttribs['classes'] = [ 'oo-ui-element-hidden' ];
$labelAttribs['classes'] = [ 'oo-ui-element-hidden' ];
} elseif ( $filterObj->isProtected() ) {
if ( $filterObj->isProtected() ) {
$checkboxAttribs['disabled'] = true;
$labelAttribs['label'] = $this->msg(
'abusefilter-edit-protected-variable-already-protected'

167
tests/phpunit/integration/Special/SpecialAbuseFilterTest.php
View file

@ -2,6 +2,7 @@
namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Special;
use MediaWiki\Context\RequestContext;
use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
use MediaWiki\Extension\AbuseFilter\Filter\Filter;
@ -54,26 +55,6 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
protected function setUp(): void {
parent::setUp();
// 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 );
// Create the user to query for filters
$user = $this->getTestSysop()->getUser();
@ -99,6 +80,53 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
);
}
/**
* @inheritDoc
*/
public function addDBDataOnce() {
// Add filters to query for
$filters = [
[
'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',
],
[
'id' => '2',
'rules' => 'user_name = "1.2.3.4"',
'name' => 'Filter without protected variables',
'hidden' => Flags::FILTER_PUBLIC,
'user' => 0,
'user_text' => 'FilterTester',
'timestamp' => '20000101000000',
'enabled' => 1,
'comments' => '',
'hit_count' => 0,
'throttled' => 0,
'deleted' => 0,
'actions' => [],
'global' => 0,
'group' => 'default',
],
];
foreach ( $filters as $filter ) {
$this->createFilter( $filter );
}
}
/**
* @dataProvider provideInstantiateView
*/
@ -239,6 +267,104 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
);
}
public function testViewEditProtectedVarsCheckboxPresentForProtectedFilter() {
[ $html, ] = $this->executeSpecialPage(
'1',
new FauxRequest(),
null,
$this->authorityCanUseProtectedVar
);
$this->assertStringNotContainsString(
'abusefilter-edit-protected-help-message',
$html,
'The enabled checkbox to protect the filter was not present.'
);
$this->assertStringContainsString(
'abusefilter-edit-protected-variable-already-protected',
$html,
'The disabled checkbox explaining that the filter is protected was not present.'
);
}
public function testViewEditProtectedVarsCheckboxAbsentForUnprotectedFilter() {
[ $html, ] = $this->executeSpecialPage(
'2',
new FauxRequest(),
null,
$this->authorityCanUseProtectedVar
);
$this->assertStringNotContainsString(
'abusefilter-edit-protected',
$html,
'Elements related to protected filters were present.'
);
}
public function testViewEditProtectedVarsSave() {
$authority = $this->authorityCanUseProtectedVar;
$user = $authority->getUser();
// Set the abuse filter editor to the context user, so that the edit token matches
RequestContext::getMain()->getRequest()->getSession()->setUser( $user );
[ $html, ] = $this->executeSpecialPage(
'new',
new FauxRequest(
[
'wpFilterDescription' => 'Uses protected variable',
'wpFilterRules' => 'user_unnamed_ip = "1.2.3.4"',
'wpFilterNotes' => '',
'wpEditToken' => $user->getEditToken( [ 'abusefilter', 'new' ] ),
],
// This was posted
true,
),
null,
$authority
);
$this->assertStringContainsString(
'abusefilter-edit-protected-variable-not-protected',
$html,
'The error message about protecting the filter was not present.'
);
$this->assertStringContainsString(
'abusefilter-edit-protected-help-message',
$html,
'The enabled checkbox to protect the filter was not present.'
);
}
public function testViewEditProtectedVarsSaveSuccess() {
$authority = $this->authorityCanUseProtectedVar;
$user = $authority->getUser();
// Set the abuse filter editor to the context user, so that the edit token matches
RequestContext::getMain()->getRequest()->getSession()->setUser( $user );
[ $html, $response ] = $this->executeSpecialPage(
'new',
new FauxRequest(
[
'wpFilterDescription' => 'Uses protected variable',
'wpFilterRules' => 'user_unnamed_ip = "1.2.3.4"',
'wpFilterProtected' => '1',
'wpFilterNotes' => '',
'wpEditToken' => $user->getEditToken( [ 'abusefilter', 'new' ] ),
],
// This was posted
true,
),
null,
$authority
);
// On saving successfully, the page redirects
$this->assertSame( '', $html );
$this->assertStringContainsString( 'result=success', $response->getHeader( 'location' ) );
}
public function testViewTestBatchProtectedVarsFilterVisibility() {
// Assert that the user who cannot see protected variables cannot load the filter
[ $html, ] = $this->executeSpecialPage(
@ -292,5 +418,4 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
);
$this->assertStringContainsString( '1.2.3.4', $html );
}
}