Merge "REST: Check authorization in PatchPropertyLabels"

wikimedia · Oct 28, 2023 · b5fa46f · b5fa46f
2 parents 5eb7fe1 + 37ba5d4
commit b5fa46f
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 4 deletions.
diff --git a/repo/rest-api/src/Application/UseCases/PatchPropertyLabels/PatchPropertyLabels.php b/repo/rest-api/src/Application/UseCases/PatchPropertyLabels/PatchPropertyLabels.php
@@ -5,6 +5,7 @@
 use Wikibase\Repo\RestApi\Application\Serialization\LabelsDeserializer;
 use Wikibase\Repo\RestApi\Application\Serialization\LabelsSerializer;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertPropertyExists;
+use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
 use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
 use Wikibase\Repo\RestApi\Domain\Model\LabelsEditSummary;
@@ -26,6 +27,7 @@ class PatchPropertyLabels {
 	private PropertyUpdater $propertyUpdater;
 	private PatchPropertyLabelsValidator $useCaseValidator;
 	private AssertPropertyExists $assertPropertyExists;
+	private AssertUserIsAuthorized $assertUserIsAuthorized;
 
 	public function __construct(
 		PropertyLabelsRetriever $labelsRetriever,
@@ -35,7 +37,8 @@ public function __construct(
 		PropertyRetriever $propertyRetriever,
 		PropertyUpdater $propertyUpdater,
 		PatchPropertyLabelsValidator $useCaseValidator,
-		AssertPropertyExists $assertPropertyExists
+		AssertPropertyExists $assertPropertyExists,
+		AssertUserIsAuthorized $assertUserIsAuthorized
 	) {
 		$this->labelsRetriever = $labelsRetriever;
 		$this->labelsSerializer = $labelsSerializer;
@@ -45,6 +48,7 @@ public function __construct(
 		$this->propertyUpdater = $propertyUpdater;
 		$this->useCaseValidator = $useCaseValidator;
 		$this->assertPropertyExists = $assertPropertyExists;
+		$this->assertUserIsAuthorized = $assertUserIsAuthorized;
 	}
 
 	/**
@@ -56,6 +60,11 @@ public function execute( PatchPropertyLabelsRequest $request ): PatchPropertyLab
 
 		$this->assertPropertyExists->execute( $propertyId );
 
+		$this->assertUserIsAuthorized->execute(
+			$deserializedRequest->getPropertyId(),
+			$deserializedRequest->getEditMetadata()->getUser()->getUsername()
+		);
+
 		$property = $this->propertyRetriever->getProperty( $propertyId );
 		$originalLabels = $property->getLabels();
 

diff --git a/repo/rest-api/src/RouteHandlers/PatchPropertyLabelsRouteHandler.php b/repo/rest-api/src/RouteHandlers/PatchPropertyLabelsRouteHandler.php
@@ -58,7 +58,8 @@ public static function factory(): Handler {
 				WbRestApi::getPropertyDataRetriever(),
 				WbRestApi::getPropertyUpdater(),
 				WbRestApi::getValidatingRequestDeserializer(),
-				WbRestApi::getAssertPropertyExists()
+				WbRestApi::getAssertPropertyExists(),
+				WbRestApi::getAssertUserIsAuthorized()
 			),
 			$serializer,
 			new ResponseFactory()

diff --git a/repo/rest-api/tests/mocha/api-testing/AuthTest.js b/repo/rest-api/tests/mocha/api-testing/AuthTest.js
@@ -120,7 +120,20 @@ describe( 'Auth', () => {
 			} );
 		} );
 
-		editRequestsWithInputs.forEach( ( { newRequestBuilder, requestInputs } ) => {
+		[
+			...editRequestsWithInputs,
+			{
+				newRequestBuilder: () => rbf.newPatchItemAliasesRequestBuilder( itemRequestInputs.itemId, [] ),
+				requestInputs: itemRequestInputs
+			},
+			{
+				newRequestBuilder: () => rbf.newPatchPropertyLabelsRequestBuilder(
+					propertyRequestInputs.propertyId,
+					[]
+				),
+				requestInputs: propertyRequestInputs
+			}
+		].forEach( ( { newRequestBuilder, requestInputs } ) => {
 			describe( 'Protected entity page', () => {
 				before( async () => {
 					await changeEntityProtectionStatus( requestInputs.mainTestSubject, 'sysop' ); // protect

diff --git a/...st-api/tests/phpunit/Application/UseCases/PatchPropertyLabels/PatchPropertyLabelsTest.php b/...st-api/tests/phpunit/Application/UseCases/PatchPropertyLabels/PatchPropertyLabelsTest.php
@@ -8,6 +8,7 @@
 use Wikibase\Repo\RestApi\Application\Serialization\LabelsDeserializer;
 use Wikibase\Repo\RestApi\Application\Serialization\LabelsSerializer;
 use Wikibase\Repo\RestApi\Application\UseCases\AssertPropertyExists;
+use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
 use Wikibase\Repo\RestApi\Application\UseCases\PatchPropertyLabels\PatchPropertyLabels;
 use Wikibase\Repo\RestApi\Application\UseCases\PatchPropertyLabels\PatchPropertyLabelsRequest;
 use Wikibase\Repo\RestApi\Application\UseCases\PatchPropertyLabels\PatchPropertyLabelsValidator;
@@ -48,6 +49,7 @@ class PatchPropertyLabelsTest extends TestCase {
 	private PropertyUpdater $propertyUpdater;
 	private PatchPropertyLabelsValidator $validator;
 	private AssertPropertyExists $assertPropertyExists;
+	private AssertUserIsAuthorized $assertUserIsAuthorized;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -60,6 +62,7 @@ protected function setUp(): void {
 		$this->propertyUpdater = $this->createStub( PropertyUpdater::class );
 		$this->validator = new TestValidatingRequestDeserializer();
 		$this->assertPropertyExists = $this->createStub( AssertPropertyExists::class );
+		$this->assertUserIsAuthorized = $this->createStub( AssertUserIsAuthorized::class );
 	}
 
 	public function testHappyPath(): void {
@@ -151,6 +154,26 @@ public function testGivenPropertyNotFound_throws(): void {
 		}
 	}
 
+	public function testGivenUnauthorizedRequest_throws(): void {
+		$user = 'bad-user';
+		$propertyId = new NumericPropertyId( 'P123' );
+		$request = new PatchPropertyLabelsRequest( "$propertyId", [], [], false, null, $user );
+		$expectedException = $this->createStub( UseCaseError::class );
+
+		$this->assertUserIsAuthorized = $this->createMock( AssertUserIsAuthorized::class );
+		$this->assertUserIsAuthorized->expects( $this->once() )
+			->method( 'execute' )
+			->with( $propertyId, $user )
+			->willThrowException( $expectedException );
+
+		try {
+			$this->newUseCase()->execute( $request );
+			$this->fail( 'expected exception was not thrown' );
+		} catch ( UseCaseError $e ) {
+			$this->assertSame( $expectedException, $e );
+		}
+	}
+
 	private function newUseCase(): PatchPropertyLabels {
 		return new PatchPropertyLabels(
 			$this->labelsRetriever,
@@ -160,7 +183,8 @@ private function newUseCase(): PatchPropertyLabels {
 			$this->propertyRetriever,
 			$this->propertyUpdater,
 			$this->validator,
-			$this->assertPropertyExists
+			$this->assertPropertyExists,
+			$this->assertUserIsAuthorized
 		);
 	}