REST: Respond 429 when hitting the edit rate limit

jakobw · jakobw · commit 8b1d8b6e73cb · 2024-09-24T11:27:17.000Z
Bug: T374971 Change-Id: I0b519b01f0e42dd494cb4c1f3850b0a67ae878a6
diff --git a/repo/config/Wikibase.ci.php b/repo/config/Wikibase.ci.php
@@ -79,3 +79,7 @@
 $headerMaxSize = $request->getHeader( 'X-Wikibase-CI-MAX-ENTITY-SIZE', WebRequest::GETHEADER_LIST );
 
 $wgWBRepoSettings['maxSerializedEntitySize'] = (int)( $headerMaxSize ?: $originalMaxSize );
+
+if ( $request->getHeader( 'X-Wikibase-CI-Anon-Rate-Limit-Zero', WebRequest::GETHEADER_LIST ) ) {
+	$wgRateLimits = [ 'edit' => [ 'anon' => [ 0, 60 ] ] ];
+}
diff --git a/repo/rest-api/README.md b/repo/rest-api/README.md
@@ -116,6 +116,7 @@ The following needs to be correctly set up in order for all the tests to pass:
   - `X-Wikibase-CI-Badges`
   - `X-Wikibase-CI-Redirect-Badges`
   - `X-Wikibase-Ci-Tempuser-Config`
+  - `X-Wikibase-CI-Anon-Rate-Limit-Zero`
 
 
 [1]: https://www.mediawiki.org/wiki/MediaWiki_API_integration_tests
diff --git a/repo/rest-api/src/Application/UseCases/RemoveItemDescription/RemoveItemDescription.php b/repo/rest-api/src/Application/UseCases/RemoveItemDescription/RemoveItemDescription.php
@@ -6,6 +6,7 @@
 use Wikibase\Repo\RestApi\Application\UseCases\AssertItemExists;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
 use Wikibase\Repo\RestApi\Application\UseCases\ItemRedirect;
+use Wikibase\Repo\RestApi\Application\UseCases\UpdateExceptionHandler;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\DescriptionEditSummary;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
@@ -16,6 +17,7 @@
  * @license GPL-2.0-or-later
  */
 class RemoveItemDescription {
+	use UpdateExceptionHandler;
 
 	private RemoveItemDescriptionValidator $useCaseValidator;
 	private AssertItemExists $assertItemExists;
@@ -64,7 +66,7 @@ public function execute( RemoveItemDescriptionRequest $request ): void {
 			DescriptionEditSummary::newRemoveSummary( $providedEditMetadata->getComment(), $description )
 		);
 		// @phan-suppress-next-line PhanTypeMismatchArgumentNullable
-		$this->itemUpdater->update( $item, $editMetadata );
+		$this->executeWithExceptionHandling( fn() => $this->itemUpdater->update( $item, $editMetadata ) );
 	}
 
 }
diff --git a/repo/rest-api/src/Application/UseCases/RemoveItemLabel/RemoveItemLabel.php b/repo/rest-api/src/Application/UseCases/RemoveItemLabel/RemoveItemLabel.php
@@ -5,6 +5,7 @@
 use OutOfBoundsException;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertItemExists;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
+use Wikibase\Repo\RestApi\Application\UseCases\UpdateExceptionHandler;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
 use Wikibase\Repo\RestApi\Domain\Model\LabelEditSummary;
@@ -15,6 +16,7 @@
  * @license GPL-2.0-or-later
  */
 class RemoveItemLabel {
+	use UpdateExceptionHandler;
 
 	private RemoveItemLabelValidator $useCaseValidator;
 	private AssertItemExists $assertItemExists;
@@ -58,10 +60,10 @@ public function execute( RemoveItemLabelRequest $request ): void {
 
 		$summary = LabelEditSummary::newRemoveSummary( $providedEditMetadata->getComment(), $label );
 
-		$this->itemUpdater->update(
+		$this->executeWithExceptionHandling( fn() => $this->itemUpdater->update(
 			$item, // @phan-suppress-current-line PhanTypeMismatchArgumentNullable
 			new EditMetadata( $providedEditMetadata->getTags(), $providedEditMetadata->isBot(), $summary )
-		);
+		) );
 	}
 
 }
diff --git a/repo/rest-api/src/Application/UseCases/RemovePropertyDescription/RemovePropertyDescription.php b/repo/rest-api/src/Application/UseCases/RemovePropertyDescription/RemovePropertyDescription.php
@@ -5,6 +5,7 @@
 use OutOfBoundsException;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertPropertyExists;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
+use Wikibase\Repo\RestApi\Application\UseCases\UpdateExceptionHandler;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\DescriptionEditSummary;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
@@ -15,6 +16,7 @@
  * @license GPL-2.0-or-later
  */
 class RemovePropertyDescription {
+	use UpdateExceptionHandler;
 
 	private RemovePropertyDescriptionValidator $requestValidator;
 	private AssertPropertyExists $assertPropertyExists;
@@ -60,10 +62,10 @@ public function execute( RemovePropertyDescriptionRequest $request ): void {
 		$property->getDescriptions()->removeByLanguage( $languageCode );
 
 		$summary = DescriptionEditSummary::newRemoveSummary( $providedEditMetadata->getComment(), $description );
-		$this->propertyUpdater->update(
+		$this->executeWithExceptionHandling( fn() => $this->propertyUpdater->update(
 			$property, // @phan-suppress-current-line PhanTypeMismatchArgumentNullable
 			new EditMetadata( $providedEditMetadata->getTags(), $providedEditMetadata->isBot(), $summary )
-		);
+		) );
 	}
 
 }
diff --git a/repo/rest-api/src/Application/UseCases/RemovePropertyLabel/RemovePropertyLabel.php b/repo/rest-api/src/Application/UseCases/RemovePropertyLabel/RemovePropertyLabel.php
@@ -5,6 +5,7 @@
 use OutOfBoundsException;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertPropertyExists;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
+use Wikibase\Repo\RestApi\Application\UseCases\UpdateExceptionHandler;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
 use Wikibase\Repo\RestApi\Domain\Model\LabelEditSummary;
@@ -15,6 +16,7 @@
  * @license GPL-2.0-or-later
  */
 class RemovePropertyLabel {
+	use UpdateExceptionHandler;
 
 	private RemovePropertyLabelValidator $requestValidator;
 	private AssertPropertyExists $assertPropertyExists;
@@ -60,10 +62,10 @@ public function execute( RemovePropertyLabelRequest $request ): void {
 
 		$summary = LabelEditSummary::newRemoveSummary( $providedEditMetadata->getComment(), $label );
 
-		$this->propertyUpdater->update(
+		$this->executeWithExceptionHandling( fn() => $this->propertyUpdater->update(
 			$property, // @phan-suppress-current-line PhanTypeMismatchArgumentNullable
 			new EditMetadata( $providedEditMetadata->getTags(), $providedEditMetadata->isBot(), $summary )
-		);
+		) );
 	}
 
 }
diff --git a/repo/rest-api/src/Application/UseCases/RemoveSitelink/RemoveSitelink.php b/repo/rest-api/src/Application/UseCases/RemoveSitelink/RemoveSitelink.php
@@ -5,6 +5,7 @@
 use Wikibase\Repo\RestApi\Application\UseCases\AssertItemExists;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
 use Wikibase\Repo\RestApi\Application\UseCases\ItemRedirect;
+use Wikibase\Repo\RestApi\Application\UseCases\UpdateExceptionHandler;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
 use Wikibase\Repo\RestApi\Domain\Model\SitelinkEditSummary;
@@ -15,6 +16,7 @@
  * @license GPL-2.0-or-later
  */
 class RemoveSitelink {
+	use UpdateExceptionHandler;
 
 	private ItemWriteModelRetriever $itemRetriever;
 	private ItemUpdater $itemUpdater;
@@ -57,14 +59,14 @@ public function execute( RemoveSitelinkRequest $request ): void {
 
 		$removedSitelink = $item->getSiteLink( $siteId );
 		$item->removeSiteLink( $siteId );
-		$this->itemUpdater->update(
+		$this->executeWithExceptionHandling( fn() => $this->itemUpdater->update(
 			$item, // @phan-suppress-current-line PhanTypeMismatchArgumentNullable
 			new EditMetadata(
 				$editMetadata->getTags(),
 				$editMetadata->isBot(),
 				SitelinkEditSummary::newRemoveSummary( $editMetadata->getComment(), $removedSitelink )
 			)
-		);
+		) );
 	}
 
 }
diff --git a/repo/rest-api/src/Application/UseCases/RemoveStatement/RemoveStatement.php b/repo/rest-api/src/Application/UseCases/RemoveStatement/RemoveStatement.php
@@ -5,6 +5,7 @@
 use Wikibase\Repo\RestApi\Application\UseCases\AssertStatementSubjectExists;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
 use Wikibase\Repo\RestApi\Application\UseCases\ItemRedirect;
+use Wikibase\Repo\RestApi\Application\UseCases\UpdateExceptionHandler;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
 use Wikibase\Repo\RestApi\Domain\Model\StatementEditSummary;
@@ -15,6 +16,7 @@
  * @license GPL-2.0-or-later
  */
 class RemoveStatement {
+	use UpdateExceptionHandler;
 
 	private RemoveStatementValidator $validator;
 	private AssertUserIsAuthorized $assertUserIsAuthorized;
@@ -61,7 +63,9 @@ public function execute( RemoveStatementRequest $request ): void {
 			StatementEditSummary::newRemoveSummary( $deserializedRequest->getEditMetadata()->getComment(), $statementToRemove )
 		);
 
-		$this->statementRemover->remove( $deserializedRequest->getStatementId(), $editMetadata );
+		$this->executeWithExceptionHandling(
+			fn() => $this->statementRemover->remove( $deserializedRequest->getStatementId(), $editMetadata )
+		);
 	}
 
 }
diff --git a/repo/rest-api/src/Application/UseCases/UpdateExceptionHandler.php b/repo/rest-api/src/Application/UseCases/UpdateExceptionHandler.php
@@ -3,6 +3,7 @@
 namespace Wikibase\Repo\RestApi\Application\UseCases;
 
 use Wikibase\Repo\RestApi\Domain\Services\Exceptions\AbuseFilterException;
+use Wikibase\Repo\RestApi\Domain\Services\Exceptions\RateLimitReached;
 use Wikibase\Repo\RestApi\Domain\Services\Exceptions\ResourceTooLargeException;
 
 /**
@@ -30,6 +31,8 @@ public function executeWithExceptionHandling( callable $callback ) {
 				'filter_id' => $e->getFilterId(),
 				'filter_description' => $e->getFilterDescription(),
 			] );
+		} catch ( RateLimitReached $e ) {
+			throw UseCaseError::newRateLimitReached( UseCaseError::REQUEST_LIMIT_REASON_RATE_LIMIT );
 		}
 	}
 
diff --git a/repo/rest-api/src/Application/UseCases/UseCaseError.php b/repo/rest-api/src/Application/UseCases/UseCaseError.php
@@ -41,6 +41,8 @@ class UseCaseError extends UseCaseException {
 	public const POLICY_VIOLATION_SITELINK_CONFLICT = 'sitelink-conflict';
 	public const PROPERTY_STATEMENT_ID_MISMATCH = 'property-statement-id-mismatch';
 	public const REFERENCED_RESOURCE_NOT_FOUND = 'referenced-resource-not-found';
+	public const REQUEST_LIMIT_REACHED = 'request-limit-reached';
+	public const REQUEST_LIMIT_REASON_RATE_LIMIT = 'rate-limit-reached';
 	public const RESOURCE_NOT_FOUND = 'resource-not-found';
 	public const RESOURCE_TOO_LARGE = 'resource-too-large';
 	public const STATEMENT_GROUP_PROPERTY_ID_MISMATCH = 'statement-group-property-id-mismatch';
@@ -50,6 +52,8 @@ class UseCaseError extends UseCaseException {
 	public const CONTEXT_ACTUAL_VALUE = 'actual_value';
 	public const CONTEXT_CONFLICTING_ITEM_ID = 'conflicting_item_id';
 	public const CONTEXT_CONFLICTING_PROPERTY_ID = 'conflicting_property_id';
+	public const CONTEXT_DENIAL_REASON = 'denial_reason';
+	public const CONTEXT_DENIAL_CONTEXT = 'denial_context';
 	public const CONTEXT_DESCRIPTION = 'description';
 	public const CONTEXT_FIELD = 'field';
 	public const CONTEXT_ITEM_ID = 'item_id';
@@ -60,8 +64,7 @@ class UseCaseError extends UseCaseException {
 	public const CONTEXT_PARAMETER = 'parameter';
 	public const CONTEXT_PATH = 'path';
 	public const CONTEXT_PROPERTY_ID = 'property_id';
-	public const CONTEXT_DENIAL_REASON = 'denial_reason';
-	public const CONTEXT_DENIAL_CONTEXT = 'denial_context';
+	public const CONTEXT_REASON = 'reason';
 	public const CONTEXT_REDIRECT_TARGET = 'redirect_target';
 	public const CONTEXT_RESOURCE_TYPE = 'resource_type';
 	public const CONTEXT_SITE_ID = 'site_id';
@@ -102,6 +105,7 @@ class UseCaseError extends UseCaseException {
 		self::PERMISSION_DENIED_UNKNOWN_REASON => [],
 		self::PROPERTY_STATEMENT_ID_MISMATCH => [ self::CONTEXT_PROPERTY_ID, self::CONTEXT_STATEMENT_ID ],
 		self::REFERENCED_RESOURCE_NOT_FOUND => [ self::CONTEXT_PATH ],
+		self::REQUEST_LIMIT_REACHED => [ self::CONTEXT_REASON ],
 		self::RESOURCE_NOT_FOUND => [ self::CONTEXT_RESOURCE_TYPE ],
 		self::RESOURCE_TOO_LARGE => [ self::CONTEXT_LIMIT ],
 		self::STATEMENT_GROUP_PROPERTY_ID_MISMATCH => [
@@ -223,6 +227,14 @@ public static function newPermissionDenied( string $reason, array $denialContext
 		return $error;
 	}
 
+	public static function newRateLimitReached( string $reason ): self {
+		return new self(
+			self::REQUEST_LIMIT_REACHED,
+			'Exceeded the limit of actions that can be performed in a given span of time',
+			[ self::CONTEXT_REASON => $reason ]
+		);
+	}
+
 	public static function newResourceNotFound( string $resourceType ): self {
 		return new self(
 			self::RESOURCE_NOT_FOUND,
diff --git a/repo/rest-api/src/Domain/Services/Exceptions/RateLimitReached.php b/repo/rest-api/src/Domain/Services/Exceptions/RateLimitReached.php
@@ -0,0 +1,11 @@
+<?php declare( strict_types=1 );
+
+namespace Wikibase\Repo\RestApi\Domain\Services\Exceptions;
+
+use Exception;
+
+/**
+ * @license GPL-2.0-or-later
+ */
+class RateLimitReached extends Exception {
+}
diff --git a/repo/rest-api/src/Infrastructure/DataAccess/EntityUpdater.php b/repo/rest-api/src/Infrastructure/DataAccess/EntityUpdater.php
@@ -20,6 +20,7 @@
 use Wikibase\Repo\EditEntity\MediaWikiEditEntityFactory;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
 use Wikibase\Repo\RestApi\Domain\Services\Exceptions\AbuseFilterException;
+use Wikibase\Repo\RestApi\Domain\Services\Exceptions\RateLimitReached;
 use Wikibase\Repo\RestApi\Domain\Services\Exceptions\ResourceTooLargeException;
 use Wikibase\Repo\RestApi\Infrastructure\DataAccess\Exceptions\EntityUpdateFailed;
 use Wikibase\Repo\RestApi\Infrastructure\DataAccess\Exceptions\EntityUpdatePrevented;
@@ -61,13 +62,19 @@ public function __construct(
 
 	/**
 	 * @throws EntityUpdateFailed
+	 * @throws ResourceTooLargeException
+	 * @throws AbuseFilterException
+	 * @throws RateLimitReached
 	 */
 	public function create( EntityDocument $entity, EditMetadata $editMetadata ): EntityRevision {
 		return $this->createOrUpdate( $entity, $editMetadata, EDIT_NEW );
 	}
 
 	/**
 	 * @throws EntityUpdateFailed
+	 * @throws ResourceTooLargeException
+	 * @throws AbuseFilterException
+	 * @throws RateLimitReached
 	 */
 	public function update( EntityDocument $entity, EditMetadata $editMetadata ): EntityRevision {
 		return $this->createOrUpdate( $entity, $editMetadata, EDIT_UPDATE );
@@ -77,6 +84,7 @@ public function update( EntityDocument $entity, EditMetadata $editMetadata ): En
 	 * @throws EntityUpdateFailed
 	 * @throws ResourceTooLargeException
 	 * @throws AbuseFilterException
+	 * @throws RateLimitReached
 	 */
 	private function createOrUpdate(
 		EntityDocument $entity,
@@ -119,6 +127,10 @@ private function createOrUpdate(
 				);
 			}
 
+			if ( $this->findErrorInStatus( $status, 'actionthrottledtext' ) ) {
+				throw new RateLimitReached();
+			}
+
 			if ( $this->isPreventedEdit( $status ) ) {
 				throw new EntityUpdatePrevented( (string)$status );
 			}
@@ -132,8 +144,7 @@ private function createOrUpdate(
 	}
 
 	private function isPreventedEdit( Status $status ): bool {
-		return $this->findErrorInStatus( $status, 'actionthrottledtext' )
-			|| $this->findErrorInStatus( $status, 'spam-blacklisted' );
+		return $this->findErrorInStatus( $status, 'spam-blacklisted' ) !== null;
 	}
 
 	private function findErrorInStatus( Status $status, string $errorCode ): ?MessageSpecifier {
diff --git a/repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php b/repo/rest-api/src/RouteHandlers/ErrorResponseToHttpStatus.php
@@ -48,6 +48,9 @@ class ErrorResponseToHttpStatus {
 		UseCaseError::PATCHED_SITELINK_URL_NOT_MODIFIABLE => 422,
 		UseCaseError::PATCHED_STATEMENT_GROUP_PROPERTY_ID_MISMATCH => 422,
 
+		// 429 errors:
+		UseCaseError::REQUEST_LIMIT_REACHED => 429,
+
 		// 500 errors:
 		UseCaseError::UNEXPECTED_ERROR => 500,
 	];
diff --git a/repo/rest-api/tests/mocha/api-testing/RateLimitTest.js b/repo/rest-api/tests/mocha/api-testing/RateLimitTest.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const { describeWithTestData } = require( '../helpers/describeWithTestData' );
+const {
+	getItemEditRequests,
+	getPropertyEditRequests
+} = require( '../helpers/happyPathRequestBuilders' );
+const { assertValidError } = require( '../helpers/responseValidator' );
+describeWithTestData( 'Rate Limiting', ( itemRequestInputs, propertyRequestInputs ) => {
+
+	[
+		...getItemEditRequests( itemRequestInputs ),
+		...getPropertyEditRequests( propertyRequestInputs )
+		// TODO createItem is special and will be handled in a follow-up
+	].forEach( ( { newRequestBuilder } ) => {
+		it( `${newRequestBuilder().getRouteDescription()} responds 429 when the edit rate limit is reached`, async () => {
+			const response = await newRequestBuilder()
+				.withHeader( 'X-Wikibase-CI-Anon-Rate-Limit-Zero', true )
+				.makeRequest();
+
+			assertValidError(
+				response,
+				429,
+				'request-limit-reached',
+				{ reason: 'rate-limit-reached' }
+			);
+		} );
+	} );
+
+} );
diff --git a/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityUpdaterTest.php b/repo/rest-api/tests/phpunit/Infrastructure/DataAccess/EntityUpdaterTest.php

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`	`namespace Wikibase\Repo\RestApi\Application\UseCases;`
`4`	`4`
`5`	`5`	`use Wikibase\Repo\RestApi\Domain\Services\Exceptions\AbuseFilterException;`
	`6`	`+use Wikibase\Repo\RestApi\Domain\Services\Exceptions\RateLimitReached;`
`6`	`7`	`use Wikibase\Repo\RestApi\Domain\Services\Exceptions\ResourceTooLargeException;`
`7`	`8`
`8`	`9`	`/**`
`@@ -30,6 +31,8 @@ public function executeWithExceptionHandling( callable $callback ) {`
`30`	`31`	`'filter_id' => $e->getFilterId(),`
`31`	`32`	`'filter_description' => $e->getFilterDescription(),`
`32`	`33`	`] );`
	`34`	`+ } catch ( RateLimitReached $e ) {`
	`35`	`+ throw UseCaseError::newRateLimitReached( UseCaseError::REQUEST_LIMIT_REASON_RATE_LIMIT );`
`33`	`36`	`}`
`34`	`37`	`}`
`35`	`38`