diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a1c95a..a874b49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Latest ## Changes * [#207](https://github.com/cleverage/ui-process-bundle/issues/207) The `main` firewall configured by the bundle (`security.configure_firewall`) limits the login attempts (Symfony `login_throttling`, default settings: 5 failed attempts per minute for a username and an IP address, 25 for an IP address); the settings can be changed in the application firewall. Require `symfony/rate-limiter`. Update documentation and tests. * [#186](https://github.com/cleverage/ui-process-bundle/issues/186) `ProcessExecuteController`, `ProcessExecuteHandler`, `LaunchType`, `ProcessConfigurationsManager` and `IsValidProcessCodeValidator` are typed on `ProcessManagerInterface` / `ProcessConfigurationRegistryInterface` (process-bundle 5.4) instead of the concrete classes, `@final since 5.4`: the process manager and the process configuration registry can be decorated or replaced for the UI. +* [#209](https://github.com/cleverage/ui-process-bundle/issues/209) The files sent as input of a process (launch form, HTTP API) are deleted from the upload directory once the process has been executed, successfully or not (they were kept forever). Set the new `cleverage_ui_process.delete_uploaded_files` parameter to `false` to keep them. Update documentation, add tests. v3.3.1 ------ diff --git a/config/services/controller.yaml b/config/services/controller.yaml index 97c4806..f16388e 100644 --- a/config/services/controller.yaml +++ b/config/services/controller.yaml @@ -124,6 +124,7 @@ services: $bus: '@messenger.default_bus' $processManager: '@cleverage_process.manager.process' $processExecutionManager: '@cleverage_ui_process.manager.process_execution' + $uploadedFileManager: '@cleverage_ui_process.manager.uploaded_file' calls: - [setContainer, ['@Psr\Container\ContainerInterface']] tags: diff --git a/config/services/manager.yaml b/config/services/manager.yaml index 555f736..18a816a 100644 --- a/config/services/manager.yaml +++ b/config/services/manager.yaml @@ -17,6 +17,13 @@ services: CleverAge\UiProcessBundle\Manager\ProcessConfigurationsManager: alias: cleverage_ui_process.manager.process_configuration + cleverage_ui_process.manager.uploaded_file: + class: CleverAge\UiProcessBundle\Manager\UploadedFileManager + public: false + arguments: + - '%cleverage_ui_process.upload_directory%' + - '%cleverage_ui_process.delete_uploaded_files%' + cleverage_ui_process.stop_checker.process_execution: class: CleverAge\UiProcessBundle\StopChecker\ProcessExecutionStopChecker public: false diff --git a/config/services/message.yaml b/config/services/message.yaml index 7290502..124cb3b 100644 --- a/config/services/message.yaml +++ b/config/services/message.yaml @@ -16,5 +16,6 @@ services: arguments: - '@cleverage_process.manager.process' - '@cleverage_ui_process.monolog_handler.process' + - '@cleverage_ui_process.manager.uploaded_file' tags: [messenger.message_handler] diff --git a/config/services/parameters.yaml b/config/services/parameters.yaml index 52a1c31..0d11918 100644 --- a/config/services/parameters.yaml +++ b/config/services/parameters.yaml @@ -1,4 +1,6 @@ parameters: cleverage_ui_process.upload_directory: '%kernel.project_dir%/var/storage/uploads' + # Delete the files sent as input of a process (launch form, HTTP API) once the process has been executed + cleverage_ui_process.delete_uploaded_files: true # Deprecated since 3.3, removed in 4.0 (see UploadDirectoryParameterPass) upload_directory: '%cleverage_ui_process.upload_directory%' diff --git a/docs/cookbooks/form_file_upload.md b/docs/cookbooks/form_file_upload.md index a93ec3b..d5df0eb 100644 --- a/docs/cookbooks/form_file_upload.md +++ b/docs/cookbooks/form_file_upload.md @@ -79,8 +79,9 @@ How it works: - The execution, its report and its logs are then available in "Process > Executions" (see [process executions & logs](../reference/04-process_executions_and_logs.md)). -Uploaded files are not removed after the execution: add a cleaning step at the end of the process (e.g. a -[FileRemoverTask](https://github.com/cleverage/process-bundle/blob/main/docs/reference/tasks/file_remover_task.md)) or purge the `cleverage_ui_process.upload_directory` directory periodically. +Uploaded files are deleted once the process has been executed (see the `cleverage_ui_process.delete_uploaded_files` +[parameter](../reference/01-bundle_configuration.md#container-parameters)): copy the file in the process if it must be +kept. The same process can also be launched through the [HTTP API](../reference/06-http_api.md), with a file upload: diff --git a/docs/reference/01-bundle_configuration.md b/docs/reference/01-bundle_configuration.md index 754caf8..c7c2bfa 100644 --- a/docs/reference/01-bundle_configuration.md +++ b/docs/reference/01-bundle_configuration.md @@ -97,6 +97,7 @@ Container parameters | Parameter | Default | Description | |-----------------------------------------|-------------------------------------------|-------------| | `cleverage_ui_process.upload_directory` | `%kernel.project_dir%/var/storage/uploads` | Directory where files uploaded from the launch form or the [HTTP API](06-http_api.md) are stored before being passed as process input. | +| `cleverage_ui_process.delete_uploaded_files` | `true` | Delete the uploaded file once the process has been executed (successfully or not), by the Messenger handler or by the HTTP API when the process is not queued. Only the files of `cleverage_ui_process.upload_directory` are deleted. Set it to `false` to keep them, e.g. when a process stores the path of its input. | | `upload_directory` | `%cleverage_ui_process.upload_directory%` | Deprecated since 3.3, removed in 4.0: use `cleverage_ui_process.upload_directory`. When your application still sets it, its value is used (unless `cleverage_ui_process.upload_directory` is also set), with a deprecation. | It can be overridden in your application: diff --git a/docs/reference/02-process_ui_options.md b/docs/reference/02-process_ui_options.md index 59eb99f..d4b7b7c 100644 --- a/docs/reference/02-process_ui_options.md +++ b/docs/reference/02-process_ui_options.md @@ -85,7 +85,8 @@ The launch form (`/process/launch?process=`) contains: When `entrypoint_type` is `file`, the uploaded file is saved to the `cleverage_ui_process.upload_directory` directory (see [bundle configuration](01-bundle_configuration.md#container-parameters)) with a random name keeping the original -extension, and its absolute path is passed as process input. The file is not removed after the execution. +extension, and its absolute path is passed as process input. The file is deleted once the process has been executed, +unless `cleverage_ui_process.delete_uploaded_files` is `false`. Default values -------------- diff --git a/docs/reference/06-http_api.md b/docs/reference/06-http_api.md index ddcfd5e..c52185c 100644 --- a/docs/reference/06-http_api.md +++ b/docs/reference/06-http_api.md @@ -31,7 +31,7 @@ Parameters can be sent either as a JSON body, or as form data (`application/x-ww | Parameter | Type | Required | Default | Description | |-----------|---------------------------|:--------:|---------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `code` | `string` | **X** | | Code of the process. It must exist and be public. | -| `input` | `string` or file | | `null` | Process input. With form data, it can be an uploaded file: the file is saved in the `cleverage_ui_process.upload_directory` directory (see [bundle configuration](01-bundle_configuration.md#container-parameters)) and its path is passed as input. | +| `input` | `string` or file | | `null` | Process input. With form data, it can be an uploaded file: the file is saved in the `cleverage_ui_process.upload_directory` directory (see [bundle configuration](01-bundle_configuration.md#container-parameters)) and its path is passed as input; the file is deleted once the process has been executed, unless `cleverage_ui_process.delete_uploaded_files` is `false`. | | `context` | `object` or JSON `string` | | `{}` | Process context, as key/value pairs. A JSON string must decode to an object or an array (`422` otherwise). With form data, send one field per value: `context[key]=value`. | | `queue` | `bool` | | `true` | `true`: the process is queued to the `execute_process` transport (see [messenger](08-messenger.md)). `false`: the process is executed during the HTTP request. | diff --git a/src/Controller/ProcessExecuteController.php b/src/Controller/ProcessExecuteController.php index 4af83ff..ecbd71c 100644 --- a/src/Controller/ProcessExecuteController.php +++ b/src/Controller/ProcessExecuteController.php @@ -17,6 +17,7 @@ use CleverAge\UiProcessBundle\Entity\ProcessExecution; use CleverAge\UiProcessBundle\Http\Model\HttpProcessExecution; use CleverAge\UiProcessBundle\Manager\ProcessExecutionManager; +use CleverAge\UiProcessBundle\Manager\UploadedFileManager; use CleverAge\UiProcessBundle\Message\ProcessExecuteMessage; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; @@ -40,6 +41,7 @@ public function __construct( private readonly MessageBusInterface $bus, private readonly ProcessManagerInterface $processManager, private readonly ProcessExecutionManager $processExecutionManager, + private readonly ?UploadedFileManager $uploadedFileManager = null, ) { } @@ -84,6 +86,8 @@ public function __invoke( : $e->getMessage(); return new JsonResponse($message, Response::HTTP_INTERNAL_SERVER_ERROR); + } finally { + $this->uploadedFileManager?->deleteInput($httpProcessExecution->input); } return new JsonResponse('Process has been proceed well.'); diff --git a/src/Manager/UploadedFileManager.php b/src/Manager/UploadedFileManager.php new file mode 100644 index 0000000..598483f --- /dev/null +++ b/src/Manager/UploadedFileManager.php @@ -0,0 +1,48 @@ +deleteUploadedFiles || !\is_string($input) || !is_file($input)) { + return; + } + $uploadDirectory = realpath($this->uploadDirectory); + // Only the files written by the bundle, never a file given by its path + if (false === $uploadDirectory || realpath(\dirname($input)) !== $uploadDirectory) { + return; + } + + (new Filesystem())->remove($input); + } +} diff --git a/src/Message/ProcessExecuteHandler.php b/src/Message/ProcessExecuteHandler.php index a1896d3..3fed142 100644 --- a/src/Message/ProcessExecuteHandler.php +++ b/src/Message/ProcessExecuteHandler.php @@ -14,6 +14,7 @@ namespace CleverAge\UiProcessBundle\Message; use CleverAge\ProcessBundle\Manager\ProcessManagerInterface; +use CleverAge\UiProcessBundle\Manager\UploadedFileManager; use CleverAge\UiProcessBundle\Monolog\Handler\ProcessHandler; use Symfony\Component\Messenger\Attribute\AsMessageHandler; @@ -23,13 +24,20 @@ #[AsMessageHandler] readonly class ProcessExecuteHandler { - public function __construct(private ProcessManagerInterface $manager, private ProcessHandler $processHandler) - { + public function __construct( + private ProcessManagerInterface $manager, + private ProcessHandler $processHandler, + private ?UploadedFileManager $uploadedFileManager = null, + ) { } public function __invoke(ProcessExecuteMessage $message): void { $this->processHandler->close(); - $this->manager->execute($message->code, $message->input, $message->context); + try { + $this->manager->execute($message->code, $message->input, $message->context); + } finally { + $this->uploadedFileManager?->deleteInput($message->input); + } } } diff --git a/tests/Functional/HttpProcessExecuteTest.php b/tests/Functional/HttpProcessExecuteTest.php index 419caa7..ac96f9b 100644 --- a/tests/Functional/HttpProcessExecuteTest.php +++ b/tests/Functional/HttpProcessExecuteTest.php @@ -32,6 +32,7 @@ use CleverAge\UiProcessBundle\Http\ValueResolver\HttpProcessExecuteValueResolver; use CleverAge\UiProcessBundle\Manager\ProcessConfigurationsManager; use CleverAge\UiProcessBundle\Manager\ProcessExecutionManager; +use CleverAge\UiProcessBundle\Manager\UploadedFileManager; use CleverAge\UiProcessBundle\Message\ProcessExecuteMessage; use CleverAge\UiProcessBundle\Monolog\Handler\DoctrineProcessHandler; use CleverAge\UiProcessBundle\Monolog\Handler\ProcessHandler; @@ -55,6 +56,7 @@ * HTTP API: POST /http/process/execute, authenticated with a Bearer token. */ #[CoversClass(ProcessExecuteController::class)] +#[UsesClass(UploadedFileManager::class)] #[CoversClass(HttpProcessExecutionAuthenticator::class)] #[UsesClass(CleverAgeUiProcessBundle::class)] #[UsesClass(CheckDoctrineLazyGhostObjectsPass::class)] diff --git a/tests/Manager/UploadedFileManagerTest.php b/tests/Manager/UploadedFileManagerTest.php new file mode 100644 index 0000000..2eaff92 --- /dev/null +++ b/tests/Manager/UploadedFileManagerTest.php @@ -0,0 +1,76 @@ +directory = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('uploaded_file_test_', true); + $filesystem = new Filesystem(); + $filesystem->dumpFile($this->directory.'/uploads/input.csv', 'data'); + $filesystem->dumpFile($this->directory.'/uploads/sub/input.csv', 'data'); + $filesystem->dumpFile($this->directory.'/other/input.csv', 'data'); + } + + protected function tearDown(): void + { + (new Filesystem())->remove($this->directory); + } + + public function testTheUploadedFileIsDeleted(): void + { + (new UploadedFileManager($this->directory.'/uploads'))->deleteInput($this->directory.'/uploads/input.csv'); + + self::assertFileDoesNotExist($this->directory.'/uploads/input.csv'); + } + + public function testFilesOutsideOfTheUploadDirectoryAreKept(): void + { + $manager = new UploadedFileManager($this->directory.'/uploads'); + $manager->deleteInput($this->directory.'/other/input.csv'); + $manager->deleteInput($this->directory.'/uploads/sub/input.csv'); + $manager->deleteInput($this->directory.'/uploads/../other/input.csv'); + + self::assertFileExists($this->directory.'/other/input.csv'); + self::assertFileExists($this->directory.'/uploads/sub/input.csv'); + } + + public function testNothingIsDeletedWhenDisabled(): void + { + (new UploadedFileManager($this->directory.'/uploads', false))->deleteInput($this->directory.'/uploads/input.csv'); + + self::assertFileExists($this->directory.'/uploads/input.csv'); + } + + public function testOtherInputsAreIgnored(): void + { + $manager = new UploadedFileManager($this->directory.'/uploads'); + $manager->deleteInput(null); + $manager->deleteInput(['input.csv']); + $manager->deleteInput($this->directory.'/uploads/missing.csv'); + $manager->deleteInput($this->directory.'/uploads'); + (new UploadedFileManager($this->directory.'/missing'))->deleteInput($this->directory.'/uploads/input.csv'); + + self::assertFileExists($this->directory.'/uploads/input.csv'); + } +} diff --git a/tests/Message/ProcessExecuteHandlerTest.php b/tests/Message/ProcessExecuteHandlerTest.php index cfebd6e..cd4937c 100644 --- a/tests/Message/ProcessExecuteHandlerTest.php +++ b/tests/Message/ProcessExecuteHandlerTest.php @@ -15,16 +15,19 @@ use CleverAge\ProcessBundle\Manager\ProcessManagerInterface; use CleverAge\UiProcessBundle\Manager\ProcessExecutionManager; +use CleverAge\UiProcessBundle\Manager\UploadedFileManager; use CleverAge\UiProcessBundle\Message\ProcessExecuteHandler; use CleverAge\UiProcessBundle\Message\ProcessExecuteMessage; use CleverAge\UiProcessBundle\Monolog\Handler\ProcessHandler; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; +use Symfony\Component\Filesystem\Filesystem; #[CoversClass(ProcessExecuteHandler::class)] #[UsesClass(ProcessExecuteMessage::class)] #[UsesClass(ProcessHandler::class)] +#[UsesClass(UploadedFileManager::class)] class ProcessExecuteHandlerTest extends TestCase { public function testExecuteTheProcessWithANewLogFile(): void @@ -48,4 +51,30 @@ public function testExecuteTheProcessWithANewLogFile(): void new ProcessExecuteMessage('test.process', 'data.csv', ['foo' => 'bar']) ); } + + public function testTheUploadedInputIsDeletedEvenWhenTheProcessFails(): void + { + $directory = sys_get_temp_dir().\DIRECTORY_SEPARATOR.uniqid('process_execute_handler_test_', true); + $input = $directory.'/input.csv'; + (new Filesystem())->dumpFile($input, 'data'); + + $manager = $this->createStub(ProcessManagerInterface::class); + $manager->method('execute')->willThrowException(new \RuntimeException('Process failed')); + $handler = new ProcessExecuteHandler( + $manager, + new ProcessHandler(sys_get_temp_dir(), $this->createStub(ProcessExecutionManager::class)), + new UploadedFileManager($directory), + ); + + try { + $handler(new ProcessExecuteMessage('test.process', $input)); + self::fail('The process should have failed'); + } catch (\RuntimeException) { + } finally { + $exists = file_exists($input); + (new Filesystem())->remove($directory); + } + + self::assertFalse($exists); + } }