From 47dec90c358e1a8ab3051a2ff2cf7c18aeeb1b01 Mon Sep 17 00:00:00 2001 From: Martin Eiber Date: Thu, 21 Nov 2024 12:24:23 +0100 Subject: [PATCH] [User Management] Upload user image endpoint (#563) * Refactor RequestBody to OpenAPI namespace. * Add upload user image endpoint. * Add Image UploadService Test. * Translation. * Apply php-cs-fixer changes * Remove unused service. * Apply php-cs-fixer changes --------- Co-authored-by: martineiber --- config/users.yaml | 4 + src/Asset/Controller/Upload/AddController.php | 4 +- .../Controller/Upload/ReplaceController.php | 4 +- src/Asset/Controller/Upload/ZipController.php | 4 +- .../Request/MultipartFormDataRequestBody.php} | 4 +- src/User/Controller/UpdateUserController.php | 2 +- .../Controller/UploadUserImageController.php | 99 +++++++++++++ src/User/Service/ImageUploadService.php | 58 ++++++++ .../Service/ImageUploadServiceInterface.php | 27 ++++ .../User/Service/ImageUploadServiceTest.php | 130 ++++++++++++++++++ translations/studio_api_docs.en.yaml | 3 +- 11 files changed, 329 insertions(+), 10 deletions(-) rename src/{Asset/Attribute/Request/AddAssetRequestBody.php => OpenApi/Attribute/Request/MultipartFormDataRequestBody.php} (88%) create mode 100644 src/User/Controller/UploadUserImageController.php create mode 100644 src/User/Service/ImageUploadService.php create mode 100644 src/User/Service/ImageUploadServiceInterface.php create mode 100644 tests/Unit/User/Service/ImageUploadServiceTest.php diff --git a/config/users.yaml b/config/users.yaml index 956043aff..605760de2 100644 --- a/config/users.yaml +++ b/config/users.yaml @@ -47,6 +47,10 @@ services: Pimcore\Bundle\StudioBackendBundle\User\Service\KeyBindingServiceInterface: class: Pimcore\Bundle\StudioBackendBundle\User\Service\KeyBindingService + Pimcore\Bundle\StudioBackendBundle\User\Service\ImageUploadServiceInterface: + class: Pimcore\Bundle\StudioBackendBundle\User\Service\ImageUploadService + + Pimcore\Bundle\StudioBackendBundle\User\Service\MailServiceInterface: class: Pimcore\Bundle\StudioBackendBundle\User\Service\MailService diff --git a/src/Asset/Controller/Upload/AddController.php b/src/Asset/Controller/Upload/AddController.php index 3a908348c..dc30fcbf0 100644 --- a/src/Asset/Controller/Upload/AddController.php +++ b/src/Asset/Controller/Upload/AddController.php @@ -19,7 +19,6 @@ use League\Flysystem\FilesystemException; use OpenApi\Attributes\Post; use OpenApi\Attributes\Property; -use Pimcore\Bundle\StudioBackendBundle\Asset\Attribute\Request\AddAssetRequestBody; use Pimcore\Bundle\StudioBackendBundle\Asset\Service\UploadServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\AccessDeniedException; @@ -29,6 +28,7 @@ use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\UserNotFoundException; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Parameter\Path\IdParameter; +use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Request\MultipartFormDataRequestBody; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\Content\IdJson; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\DefaultResponses; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\SuccessResponse; @@ -80,7 +80,7 @@ public function __construct( content: new IdJson('ID of created asset') )] #[IdParameter(type: ElementTypes::TYPE_ASSET, name: 'parentId')] - #[AddAssetRequestBody( + #[MultipartFormDataRequestBody( [ new Property( property: 'file', diff --git a/src/Asset/Controller/Upload/ReplaceController.php b/src/Asset/Controller/Upload/ReplaceController.php index aae41c7d6..07122dc55 100644 --- a/src/Asset/Controller/Upload/ReplaceController.php +++ b/src/Asset/Controller/Upload/ReplaceController.php @@ -18,7 +18,6 @@ use OpenApi\Attributes\Post; use OpenApi\Attributes\Property; -use Pimcore\Bundle\StudioBackendBundle\Asset\Attribute\Request\AddAssetRequestBody; use Pimcore\Bundle\StudioBackendBundle\Asset\Service\UploadServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\AccessDeniedException; @@ -29,6 +28,7 @@ use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\UserNotFoundException; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Parameter\Path\IdParameter; +use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Request\MultipartFormDataRequestBody; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\DefaultResponses; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\SuccessResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Config\Tags; @@ -77,7 +77,7 @@ public function __construct( description: 'asset_replace_success_response', )] #[IdParameter(type: ElementTypes::TYPE_ASSET)] - #[AddAssetRequestBody( + #[MultipartFormDataRequestBody( [ new Property( property: 'file', diff --git a/src/Asset/Controller/Upload/ZipController.php b/src/Asset/Controller/Upload/ZipController.php index 687b3aff5..a616ff595 100644 --- a/src/Asset/Controller/Upload/ZipController.php +++ b/src/Asset/Controller/Upload/ZipController.php @@ -18,7 +18,6 @@ use OpenApi\Attributes\Post; use OpenApi\Attributes\Property; -use Pimcore\Bundle\StudioBackendBundle\Asset\Attribute\Request\AddAssetRequestBody; use Pimcore\Bundle\StudioBackendBundle\Asset\Service\ExecutionEngine\ZipServiceInterface; use Pimcore\Bundle\StudioBackendBundle\Controller\AbstractApiController; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\AccessDeniedException; @@ -27,6 +26,7 @@ use Pimcore\Bundle\StudioBackendBundle\Exception\Api\NotFoundException; use Pimcore\Bundle\StudioBackendBundle\Exception\Api\UserNotFoundException; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Parameter\Path\IdParameter; +use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Request\MultipartFormDataRequestBody; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\Content\IdJson; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\CreatedResponse; use Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Response\DefaultResponses; @@ -78,7 +78,7 @@ public function __construct( content: new IdJson('ID of created jobRun', 'jobRunId') )] #[IdParameter(type: ElementTypes::TYPE_ASSET, name: 'parentId')] - #[AddAssetRequestBody( + #[MultipartFormDataRequestBody( [ new Property( property: self::FILE_KEY, diff --git a/src/Asset/Attribute/Request/AddAssetRequestBody.php b/src/OpenApi/Attribute/Request/MultipartFormDataRequestBody.php similarity index 88% rename from src/Asset/Attribute/Request/AddAssetRequestBody.php rename to src/OpenApi/Attribute/Request/MultipartFormDataRequestBody.php index 320594444..871558f7c 100644 --- a/src/Asset/Attribute/Request/AddAssetRequestBody.php +++ b/src/OpenApi/Attribute/Request/MultipartFormDataRequestBody.php @@ -14,7 +14,7 @@ * @license http://www.pimcore.org/license GPLv3 and PCL */ -namespace Pimcore\Bundle\StudioBackendBundle\Asset\Attribute\Request; +namespace Pimcore\Bundle\StudioBackendBundle\OpenApi\Attribute\Request; use Attribute; use OpenApi\Attributes\MediaType; @@ -25,7 +25,7 @@ * @internal */ #[Attribute(Attribute::TARGET_METHOD)] -final class AddAssetRequestBody extends RequestBody +final class MultipartFormDataRequestBody extends RequestBody { public function __construct(array $properties, array $required = []) { diff --git a/src/User/Controller/UpdateUserController.php b/src/User/Controller/UpdateUserController.php index a32914608..f40d66b66 100644 --- a/src/User/Controller/UpdateUserController.php +++ b/src/User/Controller/UpdateUserController.php @@ -60,7 +60,7 @@ public function __construct( /** * @throws NotFoundException|DatabaseException|ForbiddenException|ParseException */ - #[Route('/user/{id}', name: 'pimcore_studio_api_user_update', methods: ['PUT'])] + #[Route('/user/{id}', name: 'pimcore_studio_api_user_update', requirements: ['id' => '\d+'], methods: ['PUT'])] #[IsGranted(UserPermissions::USER_MANAGEMENT->value)] #[Put( path: self::PREFIX . '/user/{id}', diff --git a/src/User/Controller/UploadUserImageController.php b/src/User/Controller/UploadUserImageController.php new file mode 100644 index 000000000..78b942ed5 --- /dev/null +++ b/src/User/Controller/UploadUserImageController.php @@ -0,0 +1,99 @@ +value)] + #[Post( + path: self::PREFIX . '/user/upload-image/{id}', + operationId: 'user_upload_image', + summary: 'user_upload_image_summary', + tags: [Tags::User->value] + )] + #[IdParameter(type: 'User')] + #[SuccessResponse] + #[MultipartFormDataRequestBody( + [ + new Property( + property: 'userImage', + description: 'User image to upload', + type: 'string', + format: 'binary' + ), + ], + ['userImage'] + )] + #[DefaultResponses([ + HttpResponseCodes::NOT_FOUND, + HttpResponseCodes::FORBIDDEN, + ])] + public function uploadUserImage( + int $id, + // TODO: Symfony 7.1 change to https://symfony.com/blog/new-in-symfony-7-1-mapuploadedfile-attribute + Request $request + ): Response { + $file = $request->files->get('userImage'); + if (!$file instanceof UploadedFile) { + throw new EnvironmentException('Invalid file found in the request'); + } + + $this->imageUploadService->uploadUserImage($file, $id); + + return new Response(); + } +} diff --git a/src/User/Service/ImageUploadService.php b/src/User/Service/ImageUploadService.php new file mode 100644 index 000000000..6fd24b78a --- /dev/null +++ b/src/User/Service/ImageUploadService.php @@ -0,0 +1,58 @@ +userRepository->getUserById($userId); + $currentUser = $this->securityService->getCurrentUser(); + + if ($user->isAdmin() && !$currentUser->isAdmin()) { + throw new ForbiddenException('You are not allowed to upload an image for an admin user'); + } + + $fileType = $this->assetResolver->getTypeFromMimeMapping($file->getMimeType(), $file->getFilename()); + + if ($fileType !== 'image') { + throw new ForbiddenException('Only images are allowed'); + } + + $user->setImage($file->getPathname()); + } +} diff --git a/src/User/Service/ImageUploadServiceInterface.php b/src/User/Service/ImageUploadServiceInterface.php new file mode 100644 index 000000000..182f39bdb --- /dev/null +++ b/src/User/Service/ImageUploadServiceInterface.php @@ -0,0 +1,27 @@ +makeEmpty(UserInterface::class, [ + 'isAdmin' => true, + ]); + + $currentUserMock = $this->makeEmpty(UserInterface::class, [ + 'isAdmin' => false, + ]); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $userMock, + ]); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class, [ + 'getCurrentUser' => $currentUserMock, + ]); + + $assetResolver = $this->makeEmpty(AssetResolverInterface::class); + + $imageUploadService = new ImageUploadService($userRepositoryMock, $securityServiceMock, $assetResolver); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('You are not allowed to upload an image for an admin user'); + $imageUploadService->uploadUserImage($this->makeEmpty(UploadedFile::class), 1); + } + + public function testWrongFileType(): void + { + $userMock = $this->makeEmpty(UserInterface::class, [ + 'isAdmin' => true, + ]); + + $currentUserMock = $this->makeEmpty(UserInterface::class, [ + 'isAdmin' => true, + ]); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $userMock, + ]); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class, [ + 'getCurrentUser' => $currentUserMock, + ]); + + $assetResolver = $this->makeEmpty(AssetResolverInterface::class, [ + 'getTypeFromMimeMapping' => 'document', + ]); + + $fileMock = $this->makeEmpty(UploadedFile::class, [ + 'getMimeType' => 'application/pdf', + 'getFilename' => 'test.pdf', + ]); + + $imageUploadService = new ImageUploadService($userRepositoryMock, $securityServiceMock, $assetResolver); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('Only images are allowed'); + $imageUploadService->uploadUserImage($fileMock, 1); + } + + public function testSetImageOfUserIsCalled(): void + { + $userMock = $this->makeEmpty(UserInterface::class, [ + 'isAdmin' => true, + 'setImage' => Expected::once(function (string $path) { + $this->assertSame('/tmp/test.png', $path); + }), + ]); + + $currentUserMock = $this->makeEmpty(UserInterface::class, [ + 'isAdmin' => true, + ]); + + $userRepositoryMock = $this->makeEmpty(UserRepositoryInterface::class, [ + 'getUserById' => $userMock, + ]); + + $securityServiceMock = $this->makeEmpty(SecurityServiceInterface::class, [ + 'getCurrentUser' => $currentUserMock, + ]); + + $assetResolver = $this->makeEmpty(AssetResolverInterface::class, [ + 'getTypeFromMimeMapping' => 'image', + ]); + + $fileMock = $this->makeEmpty(UploadedFile::class, [ + 'getMimeType' => 'image/png', + 'getFilename' => 'test.png', + 'getPathname' => '/tmp/test.png', + ]); + + $imageUploadService = new ImageUploadService($userRepositoryMock, $securityServiceMock, $assetResolver); + + $imageUploadService->uploadUserImage($fileMock, 1); + } +} diff --git a/translations/studio_api_docs.en.yaml b/translations/studio_api_docs.en.yaml index 709fc860d..307e92b98 100644 --- a/translations/studio_api_docs.en.yaml +++ b/translations/studio_api_docs.en.yaml @@ -596,4 +596,5 @@ user_search_response: List of users user_default_key_bindings_description: | Get default key bindings for user management user_default_key_bindings_summary: Get default key bindings -user_default_key_bindings_response: List of default key bindings \ No newline at end of file +user_default_key_bindings_response: List of default key bindings +user_upload_image_summary: Upload user image \ No newline at end of file