Merge "REST: Add Authorization to CreateProperty use case"

wikimedia · Oct 22, 2024 · b0dcf28 · b0dcf28
2 parents b2a61e4 + 9974ea0
commit b0dcf28
Show file tree

Hide file tree

Showing 10 changed files with 282 additions and 54 deletions.
diff --git a/repo/rest-api/src/Application/UseCases/AssertUserIsAuthorized.php b/repo/rest-api/src/Application/UseCases/AssertUserIsAuthorized.php
@@ -48,4 +48,18 @@ public function checkCreateItemPermissions( User $user ): void {
 		);
 	}
 
+	public function checkCreatePropertyPermissions( User $user ): void {
+		$permissionCheckResult = $this->permissionChecker->canCreateProperty( $user );
+		if ( !$permissionCheckResult->isDenied() ) {
+			return;
+		} elseif ( $permissionCheckResult->getDenialReason() === PermissionCheckResult::DENIAL_REASON_USER_BLOCKED ) {
+			throw UseCaseError::newPermissionDenied( UseCaseError::PERMISSION_DENIED_REASON_USER_BLOCKED );
+		}
+
+		throw new UseCaseError(
+			UseCaseError::PERMISSION_DENIED_UNKNOWN_REASON,
+			'You have no permission to create a property'
+		);
+	}
+
 }
diff --git a/repo/rest-api/src/Application/UseCases/CreateProperty/CreateProperty.php b/repo/rest-api/src/Application/UseCases/CreateProperty/CreateProperty.php
@@ -3,8 +3,10 @@
 namespace Wikibase\Repo\RestApi\Application\UseCases\CreateProperty;
 
 use Wikibase\Repo\RestApi\Application\Serialization\PropertyDeserializer;
+use Wikibase\Repo\RestApi\Application\UseCases\AssertUserIsAuthorized;
 use Wikibase\Repo\RestApi\Domain\Model\CreatePropertyEditSummary;
 use Wikibase\Repo\RestApi\Domain\Model\EditMetadata;
+use Wikibase\Repo\RestApi\Domain\Model\User;
 use Wikibase\Repo\RestApi\Domain\Services\PropertyCreator;
 
 /**
@@ -14,13 +16,23 @@ class CreateProperty {
 
 	private PropertyDeserializer $propertyDeserializer;
 	private PropertyCreator $propertyCreator;
+	private AssertUserIsAuthorized $assertUserIsAuthorized;
 
-	public function __construct( PropertyDeserializer $propertyDeserializer, PropertyCreator $propertyCreator ) {
+	public function __construct(
+		PropertyDeserializer $propertyDeserializer,
+		PropertyCreator $propertyCreator,
+		AssertUserIsAuthorized $assertUserIsAuthorized
+	) {
 		$this->propertyDeserializer = $propertyDeserializer;
 		$this->propertyCreator = $propertyCreator;
+		$this->assertUserIsAuthorized = $assertUserIsAuthorized;
 	}
 
 	public function execute( CreatePropertyRequest $request ): CreatePropertyResponse {
+		// @phan-suppress-next-line PhanTypeMismatchArgumentNullable $request->getUsername() is checked
+		$user = $request->getUsername() ? User::withUsername( $request->getUsername() ) : User::newAnonymous();
+		$this->assertUserIsAuthorized->checkCreatePropertyPermissions( $user );
+
 		$revision = $this->propertyCreator->create(
 			$this->propertyDeserializer->deserialize( $request->getProperty() ),
 			new EditMetadata(

diff --git a/repo/rest-api/src/Domain/Services/PermissionChecker.php b/repo/rest-api/src/Domain/Services/PermissionChecker.php
@@ -13,6 +13,8 @@ interface PermissionChecker {
 
 	public function canCreateItem( User $user ): PermissionCheckResult;
 
+	public function canCreateProperty( User $user ): PermissionCheckResult;
+
 	public function canEdit( User $user, EntityId $id ): PermissionCheckResult;
 
 }
diff --git a/repo/rest-api/src/Infrastructure/DataAccess/WikibaseEntityPermissionChecker.php b/repo/rest-api/src/Infrastructure/DataAccess/WikibaseEntityPermissionChecker.php
@@ -7,6 +7,7 @@
 use MessageSpecifier;
 use Wikibase\DataModel\Entity\EntityId;
 use Wikibase\DataModel\Entity\Item;
+use Wikibase\DataModel\Entity\Property;
 use Wikibase\Repo\RestApi\Domain\Model\User;
 use Wikibase\Repo\RestApi\Domain\ReadModel\PermissionCheckResult;
 use Wikibase\Repo\RestApi\Domain\Services\PermissionChecker;
@@ -55,6 +56,21 @@ public function canCreateItem( User $user ): PermissionCheckResult {
 		);
 	}
 
+	public function canCreateProperty( User $user ): PermissionCheckResult {
+		$mwUser = $user->isAnonymous() ?
+			$this->userFactory->newAnonymous() :
+			// @phan-suppress-next-line PhanTypeMismatchArgumentNullable isAnonymous checks for null
+			$this->userFactory->newFromName( $user->getUsername() );
+
+		return $this->newPermissionCheckResultFromStatus(
+			$this->entityPermissionChecker->getPermissionStatusForEntity(
+				$mwUser,
+				EntityPermissionChecker::ACTION_EDIT,
+				new Property( null, null, 'string' )
+			)
+		);
+	}
+
 	private function newPermissionCheckResultFromStatus( Status $status ): PermissionCheckResult {
 		if ( $status->isGood() ) {
 			return PermissionCheckResult::newAllowed();

diff --git a/repo/rest-api/src/RouteHandlers/CreatePropertyRouteHandler.php b/repo/rest-api/src/RouteHandlers/CreatePropertyRouteHandler.php
@@ -2,6 +2,7 @@
 
 namespace Wikibase\Repo\RestApi\RouteHandlers;
 
+use MediaWiki\MediaWikiServices;
 use MediaWiki\Rest\Handler;
 use MediaWiki\Rest\Response;
 use MediaWiki\Rest\SimpleHandler;
@@ -18,7 +19,12 @@
 use Wikibase\Repo\RestApi\Application\Serialization\StatementListSerializer;
 use Wikibase\Repo\RestApi\Application\UseCases\CreateProperty\CreateProperty;
 use Wikibase\Repo\RestApi\Application\UseCases\CreateProperty\CreatePropertyRequest;
+use Wikibase\Repo\RestApi\Application\UseCases\CreateProperty\CreatePropertyResponse;
+use Wikibase\Repo\RestApi\Application\UseCases\UseCaseError;
 use Wikibase\Repo\RestApi\Domain\ReadModel\PropertyParts;
+use Wikibase\Repo\RestApi\RouteHandlers\Middleware\AuthenticationMiddleware;
+use Wikibase\Repo\RestApi\RouteHandlers\Middleware\BotRightCheckMiddleware;
+use Wikibase\Repo\RestApi\RouteHandlers\Middleware\MiddlewareHandler;
 use Wikibase\Repo\RestApi\WbRestApi;
 use Wikimedia\ParamValidator\ParamValidator;
 
@@ -36,13 +42,24 @@ class CreatePropertyRouteHandler extends SimpleHandler {
 
 	private CreateProperty $useCase;
 	private PropertyPartsSerializer $propertySerializer;
-
-	public function __construct( CreateProperty $useCase, PropertyPartsSerializer $serializer ) {
+	private ResponseFactory $responseFactory;
+	private MiddlewareHandler $middlewareHandler;
+
+	public function __construct(
+		CreateProperty $useCase,
+		PropertyPartsSerializer $serializer,
+		ResponseFactory $responseFactory,
+		MiddlewareHandler $middlewareHandler
+	) {
 		$this->useCase = $useCase;
 		$this->propertySerializer = $serializer;
+		$this->responseFactory = $responseFactory;
+		$this->middlewareHandler = $middlewareHandler;
 	}
 
 	public static function factory(): Handler {
+		$responseFactory = new ResponseFactory();
+
 		return new self(
 			new CreateProperty(
 				new PropertyDeserializer(
@@ -51,62 +68,49 @@ public static function factory(): Handler {
 					new AliasesDeserializer( new AliasesInLanguageDeserializer() ),
 					WbRestApi::getStatementDeserializer()
 				),
-				WbRestApi::getPropertyUpdater()
+				WbRestApi::getPropertyUpdater(),
+				WbRestApi::getAssertUserIsAuthorized()
 			),
 			new PropertyPartsSerializer(
 				new LabelsSerializer(),
 				new DescriptionsSerializer(),
 				new AliasesSerializer(),
 				new StatementListSerializer( WbRestApi::getStatementSerializer() )
 			),
+			$responseFactory,
+			new MiddlewareHandler( [
+				new AuthenticationMiddleware( MediaWikiServices::getInstance()->getUserIdentityUtils() ),
+				new BotRightCheckMiddleware( MediaWikiServices::getInstance()->getPermissionManager(), $responseFactory ),
+			] )
 		);
 	}
 
-	public function run(): Response {
+	/**
+	 * @param mixed ...$args
+	 */
+	public function run( ...$args ): Response {
+		return $this->middlewareHandler->run( $this, [ $this, 'runUseCase' ], $args );
+	}
+
+	public function runUseCase(): Response {
 		$jsonBody = $this->getValidatedBody();
 		'@phan-var array $jsonBody'; // guaranteed to be an array per getBodyParamSettings()
 
-		$useCaseResponse = $this->useCase->execute(
-			new CreatePropertyRequest(
-				$jsonBody[self::PROPERTY_BODY_PARAM],
-				$jsonBody[self::TAGS_BODY_PARAM] ?? [],
-				$jsonBody[self::BOT_BODY_PARAM] ?? false,
-				$jsonBody[self::COMMENT_BODY_PARAM] ?? null,
-				$this->getUsername()
-			)
-		);
-
-		$response = $this->getResponseFactory()->create();
-		$response->setStatus( 201 );
-		$response->setHeader( 'Content-Type', 'application/json' );
-		$response->setHeader(
-			'Last-Modified',
-			wfTimestamp( TS_RFC2822, $useCaseResponse->getLastModified() )
-		);
-		$response->setHeader( 'ETag', "\"{$useCaseResponse->getRevisionId()}\"" );
-
-		$property = $useCaseResponse->getProperty();
-		$response->setHeader(
-			'Location',
-			$this->getRouter()->getRouteUrl(
-				GetPropertyRouteHandler::ROUTE,
-				[ GetPropertyRouteHandler::PROPERTY_ID_PATH_PARAM => $property->getId() ]
-			)
-		);
-
-		$response->setBody( new StringStream( json_encode(
-			$this->propertySerializer->serialize( new PropertyParts(
-				$property->getId(),
-				PropertyParts::VALID_FIELDS,
-				$property->getDataType(),
-				$property->getLabels(),
-				$property->getDescriptions(),
-				$property->getAliases(),
-				$property->getStatements(),
-			) )
-		) ) );
-
-		return $response;
+		try {
+			return $this->newSuccessHttpResponse(
+				$this->useCase->execute(
+					new CreatePropertyRequest(
+						$jsonBody[self::PROPERTY_BODY_PARAM],
+						$jsonBody[self::TAGS_BODY_PARAM] ?? [],
+						$jsonBody[self::BOT_BODY_PARAM] ?? false,
+						$jsonBody[self::COMMENT_BODY_PARAM] ?? null,
+						$this->getUsername()
+					)
+				)
+			);
+		} catch ( UseCaseError $e ) {
+			return $this->responseFactory->newErrorResponseFromException( $e );
+		}
 	}
 
 	/**
@@ -139,6 +143,46 @@ public function getBodyParamSettings(): array {
 		];
 	}
 
+	private function newSuccessHttpResponse( CreatePropertyResponse $useCaseResponse ): Response {
+		$response = $this->getResponseFactory()->create();
+		$response->setStatus( 201 );
+		$response->setHeader( 'Content-Type', 'application/json' );
+		$response->setHeader(
+			'Last-Modified',
+			wfTimestamp( TS_RFC2822, $useCaseResponse->getLastModified() )
+		);
+		$response->setHeader( 'ETag', "\"{$useCaseResponse->getRevisionId()}\"" );
+
+		$property = $useCaseResponse->getProperty();
+		$response->setHeader(
+			'Location',
+			$this->getRouter()->getRouteUrl(
+				GetPropertyRouteHandler::ROUTE,
+				[ GetPropertyRouteHandler::PROPERTY_ID_PATH_PARAM => $property->getId() ]
+			)
+		);
+
+		$response->setBody(
+			new StringStream(
+				json_encode(
+					$this->propertySerializer->serialize(
+						new PropertyParts(
+							$property->getId(),
+							PropertyParts::VALID_FIELDS,
+							$property->getDataType(),
+							$property->getLabels(),
+							$property->getDescriptions(),
+							$property->getAliases(),
+							$property->getStatements(),
+						)
+					)
+				)
+			)
+		);
+
+		return $response;
+	}
+
 	private function getUsername(): ?string {
 		$mwUser = $this->getAuthority()->getUser();
 		return $mwUser->isRegistered() ? $mwUser->getName() : null;

diff --git a/repo/rest-api/tests/mocha/api-testing/AuthTest.js b/repo/rest-api/tests/mocha/api-testing/AuthTest.js
@@ -12,10 +12,12 @@ const {
 	getPropertyGetRequests,
 	getItemEditRequests,
 	getPropertyEditRequests,
-	getItemCreateRequest
+	getItemCreateRequest,
+	getPropertyCreateRequest
 } = require( '../helpers/happyPathRequestBuilders' );
 const { getOrCreateAuthTestUser } = require( '../helpers/testUsers' );
 const { assertValidError } = require( '../helpers/responseValidator' );
+const { newCreatePropertyRequestBuilder } = require( '../helpers/RequestBuilderFactory' );
 
 describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, describeEachRouteWithReset ) => {
 	let user;
@@ -36,7 +38,8 @@ describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, descri
 		...editRequests,
 		...getItemGetRequests( itemRequestInputs ),
 		...getPropertyGetRequests( propertyRequestInputs ),
-		getItemCreateRequest( itemRequestInputs )
+		getItemCreateRequest( itemRequestInputs ),
+		getPropertyCreateRequest( propertyRequestInputs )
 	];
 
 	describe( 'Authentication', () => {
@@ -66,7 +69,12 @@ describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, descri
 	describe( 'Authorization', () => {
 
 		describeEachRouteWithReset(
-			[ ...editRequests, getItemCreateRequest( itemRequestInputs ) ], ( newRequestBuilder ) => {
+			[
+				...editRequests,
+				getItemCreateRequest( itemRequestInputs ),
+				getPropertyCreateRequest( propertyRequestInputs )
+			],
+			( newRequestBuilder ) => {
 				it( 'Unauthorized bot edit', async () => {
 					assertValidError(
 						await newRequestBuilder().withJsonBodyParam( 'bot', true ).makeRequest(),
@@ -79,7 +87,12 @@ describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, descri
 		);
 
 		describeEachRouteWithReset(
-			[ ...editRequests, getItemCreateRequest( itemRequestInputs ) ], ( newRequestBuilder ) => {
+			[
+				...editRequests,
+				getItemCreateRequest( itemRequestInputs ),
+				getPropertyCreateRequest( propertyRequestInputs )
+			],
+			( newRequestBuilder ) => {
 				describe( 'Blocked user', () => {
 					before( async () => {
 						await root.action( 'block', {
@@ -133,5 +146,17 @@ describeWithTestData( 'Auth', ( itemRequestInputs, propertyRequestInputs, descri
 				} );
 			} );
 		} );
+
+		it( 'cannot create a property without the property-create permission', async () => {
+			const response = await newCreatePropertyRequestBuilder( { data_type: 'string' } )
+				.withUser( user )
+				.withConfigOverride(
+					'wgGroupPermissions',
+					{ '*': { read: true, edit: true, createpage: true, 'property-create': false } }
+				)
+				.makeRequest();
+
+			expect( response ).to.have.status( 403 );
+		} );
 	} );
 } );
diff --git a/repo/rest-api/tests/mocha/helpers/happyPathRequestBuilders.js b/repo/rest-api/tests/mocha/helpers/happyPathRequestBuilders.js
@@ -190,3 +190,8 @@ module.exports.getItemCreateRequest = ( requestInput ) => ( {
 	newRequestBuilder: () => rbf.newCreateItemRequestBuilder( { labels: { en: 'new Item' } } ),
 	requestInput
 } );
+
+module.exports.getPropertyCreateRequest = ( requestInput ) => ( {
+	newRequestBuilder: () => rbf.newCreatePropertyRequestBuilder( { data_type: 'string' } ),
+	requestInput
+} );