Skip to content

Commit 4f94c26

Browse files
jenkins-botGerrit Code Review
authored andcommitted
Merge "REST: Respond 403 when AbuseFilter rejects edit"
2 parents 07c8c6a + 587db15 commit 4f94c26

File tree

6 files changed

+166
-3
lines changed

6 files changed

+166
-3
lines changed

repo/rest-api/src/Application/UseCases/UpdateExceptionHandler.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Wikibase\Repo\RestApi\Application\UseCases;
44

5+
use Wikibase\Repo\RestApi\Domain\Services\Exceptions\AbuseFilterException;
56
use Wikibase\Repo\RestApi\Domain\Services\Exceptions\ResourceTooLargeException;
67

78
/**
@@ -24,6 +25,11 @@ public function executeWithExceptionHandling( callable $callback ) {
2425
"Edit resulted in a resource that exceeds the size limit of $maxSizeInKb kB",
2526
[ UseCaseError::CONTEXT_LIMIT => $maxSizeInKb ]
2627
);
28+
} catch ( AbuseFilterException $e ) {
29+
throw UseCaseError::newPermissionDenied( UseCaseError::PERMISSION_DENIED_REASON_ABUSE_FILTER, [
30+
'filter_id' => $e->getFilterId(),
31+
'filter_description' => $e->getFilterDescription(),
32+
] );
2733
}
2834
}
2935

repo/rest-api/src/Application/UseCases/UseCaseError.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class UseCaseError extends UseCaseException {
3333
public const PERMISSION_DENIED_REASON_UNAUTHORIZED_BOT_EDIT = 'unauthorized-bot-edit';
3434
public const PERMISSION_DENIED_REASON_PAGE_PROTECTED = 'resource-protected';
3535
public const PERMISSION_DENIED_REASON_USER_BLOCKED = 'blocked-user';
36+
public const PERMISSION_DENIED_REASON_ABUSE_FILTER = 'abuse-filter';
3637
public const PERMISSION_DENIED_UNKNOWN_REASON = 'permission-denied-unknown-reason';
3738
public const POLICY_VIOLATION_ITEM_LABEL_DESCRIPTION_DUPLICATE = 'item-label-description-duplicate';
3839
public const POLICY_VIOLATION_PROPERTY_LABEL_DUPLICATE = 'property-label-duplicate';
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php declare( strict_types=1 );
2+
3+
namespace Wikibase\Repo\RestApi\Domain\Services\Exceptions;
4+
5+
use Exception;
6+
use Throwable;
7+
8+
/**
9+
* @license GPL-2.0-or-later
10+
*/
11+
class AbuseFilterException extends Exception {
12+
13+
private int $filterId;
14+
private string $filterDescription;
15+
16+
public function __construct(
17+
int $filterId,
18+
string $filterDescription,
19+
string $message = '',
20+
int $code = 0,
21+
?Throwable $previous = null
22+
) {
23+
parent::__construct( $message, $code, $previous );
24+
$this->filterId = $filterId;
25+
$this->filterDescription = $filterDescription;
26+
}
27+
28+
public function getFilterId(): int {
29+
return $this->filterId;
30+
}
31+
32+
public function getFilterDescription(): string {
33+
return $this->filterDescription;
34+
}
35+
36+
}

repo/rest-api/src/Infrastructure/DataAccess/EntityUpdater.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Wikibase\Lib\Store\EntityStore;
1818
use Wikibase\Repo\EditEntity\MediaWikiEditEntityFactory;
1919
use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
20+
use Wikibase\Repo\RestApi\Domain\Services\Exceptions\AbuseFilterException;
2021
use Wikibase\Repo\RestApi\Domain\Services\Exceptions\ResourceTooLargeException;
2122
use Wikibase\Repo\RestApi\Infrastructure\DataAccess\Exceptions\EntityUpdateFailed;
2223
use Wikibase\Repo\RestApi\Infrastructure\DataAccess\Exceptions\EntityUpdatePrevented;
@@ -70,6 +71,7 @@ public function update( EntityDocument $entity, EditMetadata $editMetadata ): En
7071
/**
7172
* @throws EntityUpdateFailed
7273
* @throws ResourceTooLargeException
74+
* @throws AbuseFilterException
7375
*/
7476
private function createOrUpdate(
7577
EntityDocument $entity,
@@ -105,6 +107,12 @@ private function createOrUpdate(
105107
throw new ResourceTooLargeException( $maxSizeInKiloBytes );
106108
}
107109

110+
$abuseFilterError = $this->findErrorInStatus( $status, 'abusefilter' );
111+
if ( $abuseFilterError ) {
112+
[ $filterDescription, $filterId ] = $abuseFilterError->getParams();
113+
throw new AbuseFilterException( (int)$filterId, $filterDescription );
114+
}
115+
108116
if ( $this->isPreventedEdit( $status ) ) {
109117
throw new EntityUpdatePrevented( (string)$status );
110118
}
@@ -119,8 +127,7 @@ private function createOrUpdate(
119127

120128
private function isPreventedEdit( Status $status ): bool {
121129
return $this->findErrorInStatus( $status, 'actionthrottledtext' )
122-
|| $this->findErrorInStatus( $status, 'spam-blacklisted' )
123-
|| $this->findErrorInStatus( $status, 'abusefilter' );
130+
|| $this->findErrorInStatus( $status, 'spam-blacklisted' );
124131
}
125132

126133
private function findErrorInStatus( Status $status, string $errorCode ): ?MessageSpecifier {
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
'use strict';
2+
3+
const { clientFactory, action, utils } = require( 'api-testing' );
4+
const {
5+
newCreateItemRequestBuilder,
6+
newSetItemLabelRequestBuilder,
7+
newSetPropertyLabelRequestBuilder,
8+
newAddItemStatementRequestBuilder,
9+
newAddPropertyStatementRequestBuilder
10+
} = require( '../helpers/RequestBuilderFactory' );
11+
const { assertValidError } = require( '../helpers/responseValidator' );
12+
const { createUniqueStringProperty } = require( '../helpers/entityHelper' );
13+
const { requireExtensions } = require( '../../../../../tests/api-testing/utils' );
14+
const config = require( 'api-testing/lib/config' )();
15+
16+
/**
17+
* AbuseFilter doesn't have an API to create filters. This is a very hacky way around the issue:
18+
* - get the edit token (a CSRF token salted for the AbuseFilter form)
19+
* - make a POST request that looks like it's coming from said form
20+
*
21+
* @param {string} description
22+
* @param {string} rules
23+
* @return {Promise<number>} the filter ID
24+
*/
25+
async function createAbuseFilter( description, rules ) {
26+
const rootClient = await action.root();
27+
const client = clientFactory.getHttpClient( rootClient );
28+
29+
const abuseFilterFormRequest = await client.get( `${config.base_uri}index.php?title=Special:AbuseFilter/new` );
30+
const editToken = abuseFilterFormRequest.text
31+
.match( /value="[a-z0-9]+\+\\"/ )[ 0 ] // the token is in the value attribute of an input field and ends with +\
32+
.slice( 'value="'.length, -1 ); // remove parts that were matched that aren't part of the token
33+
34+
const createFilterResponse = await client.post( `${config.base_uri}index.php` ).type( 'form' ).send( {
35+
title: 'Special:AbuseFilter/new',
36+
wpEditToken: editToken,
37+
wpFilterDescription: description,
38+
wpFilterRules: rules,
39+
wpFilterEnabled: 'true',
40+
wpFilterBuilder: 'other',
41+
wpFilterNotes: '',
42+
wpFilterWarnMessage: 'abusefilter-warning',
43+
wpFilterWarnMessageOther: 'abusefilter-warning',
44+
wpFilterActionDisallow: '',
45+
wpFilterDisallowMessage: 'abusefilter-disallowed',
46+
wpFilterDisallowMessageOther: 'abusefilter-disallowed',
47+
wpBlockAnonDuration: 'indefinite',
48+
wpBlockUserDuration: 'indefinite',
49+
wpFilterTags: ''
50+
} );
51+
52+
return parseInt( new URL( createFilterResponse.headers.location ).searchParams.get( 'changedfilter' ) );
53+
}
54+
55+
describe( 'Abuse Filter', () => {
56+
57+
const filterTriggerWord = utils.title( 'ABUSE-FILTER-TRIGGER-' );
58+
const filterDescription = `Filter: ${filterTriggerWord}`;
59+
let filterId;
60+
let testItemId;
61+
let testPropertyId;
62+
63+
before( async function () {
64+
await requireExtensions( [ 'AbuseFilter' ] ).call( this );
65+
66+
filterId = await createAbuseFilter( filterDescription, `"${filterTriggerWord}" in new_wikitext` );
67+
testItemId = ( await newCreateItemRequestBuilder( {} ).makeRequest() ).body.id;
68+
testPropertyId = ( await createUniqueStringProperty() ).entity.id;
69+
} );
70+
71+
[
72+
() => newCreateItemRequestBuilder( { labels: { en: filterTriggerWord } } ),
73+
() => newSetItemLabelRequestBuilder( testItemId, 'en', filterTriggerWord ),
74+
() => newSetPropertyLabelRequestBuilder( testPropertyId, 'en', filterTriggerWord ),
75+
() => newAddItemStatementRequestBuilder(
76+
testItemId,
77+
{ property: { id: testPropertyId }, value: { type: 'value', content: filterTriggerWord } }
78+
),
79+
() => newAddPropertyStatementRequestBuilder(
80+
testPropertyId,
81+
{ property: { id: testPropertyId }, value: { type: 'value', content: filterTriggerWord } }
82+
)
83+
].forEach( ( newRequestBuilder ) => {
84+
it( `${newRequestBuilder().getRouteDescription()} rejects edits matching an abuse filter`, async () => {
85+
const response = await newRequestBuilder().makeRequest();
86+
87+
assertValidError( response, 403, 'permission-denied', {
88+
denial_reason: 'abuse-filter',
89+
denial_context: { filter_id: filterId, filter_description: filterDescription }
90+
} );
91+
} );
92+
} );
93+
94+
} );

repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityUpdaterTest.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use Wikibase\Repo\EditEntity\MediaWikiEditEntityFactory;
3030
use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
3131
use Wikibase\Repo\RestApi\Domain\Model\EditSummary;
32+
use Wikibase\Repo\RestApi\Domain\Services\Exceptions\AbuseFilterException;
3233
use Wikibase\Repo\RestApi\Domain\Services\Exceptions\ResourceTooLargeException;
3334
use Wikibase\Repo\RestApi\Infrastructure\DataAccess\EntityUpdater;
3435
use Wikibase\Repo\RestApi\Infrastructure\DataAccess\Exceptions\EntityUpdateFailed;
@@ -225,6 +226,25 @@ public function testGivenResourceTooLarge_throwsCorrespondingException( EntityDo
225226
$this->newEntityUpdater()->update( $entity, $this->createStub( EditMetadata::class ) );
226227
}
227228

229+
/**
230+
* @dataProvider provideEntity
231+
*/
232+
public function testGivenAbuseFilterMatch_throwsCorrespondingException( EntityDocument $entity ): void {
233+
$filterId = 777;
234+
$filterDescription = 'bad word rejecting filter';
235+
236+
$errorStatus = EditEntityStatus::newFatal( 'abusefilter-disallowed', $filterDescription, $filterId );
237+
238+
$editEntity = $this->createStub( EditEntity::class );
239+
$editEntity->method( 'attemptSave' )->willReturn( $errorStatus );
240+
241+
$this->editEntityFactory = $this->createStub( MediaWikiEditEntityFactory::class );
242+
$this->editEntityFactory->method( 'newEditEntity' )->willReturn( $editEntity );
243+
244+
$this->expectExceptionObject( new AbuseFilterException( $filterId, $filterDescription ) );
245+
$this->newEntityUpdater()->update( $entity, $this->createStub( EditMetadata::class ) );
246+
}
247+
228248
/**
229249
* @dataProvider provideEntityAndErrorStatus
230250
*/
@@ -266,7 +286,6 @@ public function provideEntityAndErrorStatus(): array {
266286
$errorStatuses = [
267287
"basic 'actionthrottledtext' error" => [ EditEntityStatus::newFatal( 'actionthrottledtext' ) ],
268288
"wfMessage 'actionthrottledtext' error" => [ EditEntityStatus::newFatal( wfMessage( 'actionthrottledtext' ) ) ],
269-
"'abusefilter-disallowed' error" => [ EditEntityStatus::newFatal( 'abusefilter-disallowed' ) ],
270289
"'spam-blacklisted-link' error" => [ EditEntityStatus::newFatal( 'spam-blacklisted-link' ) ],
271290
"'spam-blacklisted-email' error" => [ EditEntityStatus::newFatal( 'spam-blacklisted-email' ) ],
272291
];

0 commit comments

Comments
 (0)