From e25c1e1816fda8515639c4929e281635b5dfefa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Sun, 15 Dec 2024 22:37:45 +0100 Subject: [PATCH 01/14] Refactor object storage to separate local storage and openstack storage --- .../Driver/OpenstackObjectStore}/StoredObjectManager.php | 3 ++- .../OpenstackObjectStore}/StoredObjectManagerTest.php | 6 +++--- .../ChillWopiBundle/src/Controller/ConvertController.php | 4 ---- 3 files changed, 5 insertions(+), 8 deletions(-) rename src/Bundle/ChillDocStoreBundle/{Service => AsyncUpload/Driver/OpenstackObjectStore}/StoredObjectManager.php (98%) rename src/Bundle/ChillDocStoreBundle/Tests/{Service => AsyncUpload/Driver/OpenstackObjectStore}/StoredObjectManagerTest.php (98%) diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php similarity index 98% rename from src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php rename to src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php index 098746efa..875a6f4fb 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManager.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php @@ -9,13 +9,14 @@ declare(strict_types=1); * the LICENSE file that was distributed with this source code. */ -namespace Chill\DocStoreBundle\Service; +namespace Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore; use Base64Url\Base64Url; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Entity\StoredObjectVersion; use Chill\DocStoreBundle\Exception\StoredObjectManagerException; +use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManagerTest.php similarity index 98% rename from src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php rename to src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManagerTest.php index 39412b9f7..d0cb6cb02 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Service/StoredObjectManagerTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManagerTest.php @@ -9,13 +9,13 @@ declare(strict_types=1); * the LICENSE file that was distributed with this source code. */ -namespace Chill\DocStoreBundle\Tests\Service; +namespace Chill\DocStoreBundle\Tests\AsyncUpload\Driver\OpenstackObjectStore; +use Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\StoredObjectManager; use Chill\DocStoreBundle\AsyncUpload\SignedUrl; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Exception\StoredObjectManagerException; -use Chill\DocStoreBundle\Service\StoredObjectManager; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpClient\Exception\TransportException; @@ -27,7 +27,7 @@ use Symfony\Contracts\HttpClient\HttpClientInterface; /** * @internal * - * @covers \Chill\DocStoreBundle\Service\StoredObjectManager + * @covers \Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\StoredObjectManager */ final class StoredObjectManagerTest extends TestCase { diff --git a/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php b/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php index 3b97dc6fe..247abfa13 100644 --- a/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php +++ b/src/Bundle/ChillWopiBundle/src/Controller/ConvertController.php @@ -13,7 +13,6 @@ namespace Chill\WopiBundle\Controller; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum; -use Chill\DocStoreBundle\Service\StoredObjectManager; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Chill\WopiBundle\Service\WopiConverter; use Psr\Log\LoggerInterface; @@ -26,9 +25,6 @@ class ConvertController { private const LOG_PREFIX = '[convert] '; - /** - * @param StoredObjectManager $storedObjectManager - */ public function __construct( private readonly Security $security, private readonly StoredObjectManagerInterface $storedObjectManager, From 67b5bc6dba8fbd08aa684e5c9a2ad0cfcf5d5e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Sun, 15 Dec 2024 22:37:59 +0100 Subject: [PATCH 02/14] Implements required interface to store documents on disk --- .../LocalStorage/StoredObjectManager.php | 59 +++++++++++++++++++ .../TempUrlLocalStorageGenerator.php | 29 +++++++++ 2 files changed, 88 insertions(+) create mode 100644 src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php create mode 100644 src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php new file mode 100644 index 000000000..7d5b39db8 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php @@ -0,0 +1,59 @@ + Date: Sun, 15 Dec 2024 22:38:06 +0100 Subject: [PATCH 03/14] Define new configuration for local storage --- .../DependencyInjection/Configuration.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php index 7c6a08f80..4d17c7ebd 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php @@ -30,10 +30,18 @@ class Configuration implements ConfigurationInterface /* @phpstan-ignore-next-line As there are inconsistencies in return types, but the code works... */ $rootNode->children() + ->arrayNode('local_storage') + ->info('where the stored object should be stored') + ->children() + ->scalarNode('storage_path') + ->info('the folder where the stored object should be stored') + ->isRequired()->cannotBeEmpty() + ->end() // end of storage_path + ->end() // end of children + ->end() // end of local_storage // openstack node ->arrayNode('openstack') ->info('parameters to authenticate and generate temp url against the openstack object storage service') - ->addDefaultsIfNotSet() ->children() // openstack.temp_url ->arrayNode('temp_url') From 3a2548ed8910d719f806b5d6c09725b23de0c2d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Sun, 15 Dec 2024 22:38:11 +0100 Subject: [PATCH 04/14] Select storage depending on configuration --- .../ChillDocStoreBundle.php | 3 + .../ChillDocStoreExtension.php | 6 +- .../StorageConfigurationCompilerPass.php | 60 +++++++++++++++++++ .../ChillDocStoreBundle/config/services.yaml | 4 -- 4 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php diff --git a/src/Bundle/ChillDocStoreBundle/ChillDocStoreBundle.php b/src/Bundle/ChillDocStoreBundle/ChillDocStoreBundle.php index 8dcbe72c3..1cd203c35 100644 --- a/src/Bundle/ChillDocStoreBundle/ChillDocStoreBundle.php +++ b/src/Bundle/ChillDocStoreBundle/ChillDocStoreBundle.php @@ -11,6 +11,7 @@ declare(strict_types=1); namespace Chill\DocStoreBundle; +use Chill\DocStoreBundle\DependencyInjection\Compiler\StorageConfigurationCompilerPass; use Chill\DocStoreBundle\GenericDoc\GenericDocForAccompanyingPeriodProviderInterface; use Chill\DocStoreBundle\GenericDoc\GenericDocForPersonProviderInterface; use Chill\DocStoreBundle\GenericDoc\Twig\GenericDocRendererInterface; @@ -27,5 +28,7 @@ class ChillDocStoreBundle extends Bundle ->addTag('chill_doc_store.generic_doc_person_provider'); $container->registerForAutoconfiguration(GenericDocRendererInterface::class) ->addTag('chill_doc_store.generic_doc_renderer'); + + $container->addCompilerPass(new StorageConfigurationCompilerPass()); } } diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php index efeb6362d..6851441e0 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/ChillDocStoreExtension.php @@ -53,7 +53,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf $this->prependTwig($container); } - protected function prependAuthorization(ContainerBuilder $container) + private function prependAuthorization(ContainerBuilder $container) { $container->prependExtensionConfig('security', [ 'role_hierarchy' => [ @@ -69,7 +69,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf ]); } - protected function prependRoute(ContainerBuilder $container) + private function prependRoute(ContainerBuilder $container) { // declare routes for task bundle $container->prependExtensionConfig('chill_main', [ @@ -81,7 +81,7 @@ class ChillDocStoreExtension extends Extension implements PrependExtensionInterf ]); } - protected function prependTwig(ContainerBuilder $container) + private function prependTwig(ContainerBuilder $container) { $twigConfig = [ 'form_themes' => ['@ChillDocStore/Form/fields.html.twig'], diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php new file mode 100644 index 000000000..f36cba2f4 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php @@ -0,0 +1,60 @@ +getParameterBag() + ->resolveValue($container->getParameter('chill_doc_store')); + + if (array_key_exists('openstack', $config) && array_key_exists('local_storage', $config)) { + throw new InvalidConfigurationException('chill_doc_store: you can either configure a local storage (key local_storage) or openstack storage (key openstack). Choose between them.'); + } + + if (array_key_exists('local_storage', $config)) { + foreach (self::SERVICES_OPENSTACK as $service) { + $container->removeDefinition($service); + } + + $container->setAlias(StoredObjectManagerInterface::class, \Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\StoredObjectManager::class); + $container->setAlias(TempUrlGeneratorInterface::class, TempUrlLocalStorageGenerator::class); + } else { + foreach (self::SERVICES_LOCAL_STORAGE as $service) { + $container->removeDefinition($service); + } + + $container->setAlias(StoredObjectManagerInterface::class, \Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\StoredObjectManager::class); + $container->setAlias(TempUrlGeneratorInterface::class, TempUrlOpenstackGenerator::class); + } + } +} diff --git a/src/Bundle/ChillDocStoreBundle/config/services.yaml b/src/Bundle/ChillDocStoreBundle/config/services.yaml index ec8a51832..1ca7fe4b9 100644 --- a/src/Bundle/ChillDocStoreBundle/config/services.yaml +++ b/src/Bundle/ChillDocStoreBundle/config/services.yaml @@ -8,7 +8,6 @@ services: tags: - { name: doctrine.repository_service } - Chill\DocStoreBundle\Security\Authorization\: resource: "./../Security/Authorization" @@ -56,6 +55,3 @@ services: Chill\DocStoreBundle\AsyncUpload\Command\: resource: '../AsyncUpload/Command/' - - Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface: - alias: Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\TempUrlOpenstackGenerator From 1f6de3cb1130f2f6ac0df4b376d2be60c488cd0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 16 Dec 2024 23:20:08 +0100 Subject: [PATCH 05/14] Implement TempUrlLocalStorageGenerator and its tests Added the full implementation for TempUrlLocalStorageGenerator, including methods to generate signed URLs and POST requests with expiration and signature logic. Introduced corresponding unit tests to validate functionality using mocked dependencies. --- .../TempUrlLocalStorageGenerator.php | 50 +++++++++++- .../TempUrlLocalStorageGeneratorTest.php | 78 +++++++++++++++++++ 2 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php index 81f69927c..957413b3a 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php @@ -14,16 +14,62 @@ namespace Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage; use Chill\DocStoreBundle\AsyncUpload\SignedUrl; use Chill\DocStoreBundle\AsyncUpload\SignedUrlPost; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; +use Chill\DocStoreBundle\Entity\StoredObject; +use Symfony\Component\Clock\ClockInterface; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; class TempUrlLocalStorageGenerator implements TempUrlGeneratorInterface { + private const SIGNATURE_DURATION = 180; + + public function __construct( + private readonly string $secret, + private readonly ClockInterface $clock, + private readonly UrlGeneratorInterface $urlGenerator, + ) {} + public function generate(string $method, string $object_name, ?int $expire_delay = null): SignedUrl { - // TODO: Implement generate() method. + $expiration = $this->clock->now()->getTimestamp() + min($expire_delay ?? self::SIGNATURE_DURATION, self::SIGNATURE_DURATION); + + return new SignedUrl( + $method, + $this->urlGenerator->generate('chill_docstore_stored_object_operate', [ + 'object_name' => $object_name, + 'exp' => $expiration, + 'sig' => $this->sign($method, $object_name, $expiration), + ], UrlGeneratorInterface::ABSOLUTE_URL), + \DateTimeImmutable::createFromFormat('U', (string) $expiration), + $object_name, + ); } public function generatePost(?int $expire_delay = null, ?int $submit_delay = null, int $max_file_count = 1, ?string $object_name = null): SignedUrlPost { - // TODO: Implement generatePost() method. + $submitDelayComputed = min($submit_delay ?? self::SIGNATURE_DURATION, self::SIGNATURE_DURATION); + $expireDelayComputed = min($expire_delay ?? self::SIGNATURE_DURATION, self::SIGNATURE_DURATION); + $objectNameComputed = $object_name ?? StoredObject::generatePrefix(); + $expiration = $this->clock->now()->getTimestamp() + $expireDelayComputed + $submitDelayComputed; + + return new SignedUrlPost( + $this->urlGenerator->generate( + 'chill_docstore_storedobject_post', + ['prefix' => $objectNameComputed], + UrlGeneratorInterface::ABSOLUTE_URL + ), + \DateTimeImmutable::createFromFormat('U', (string) $expiration), + $objectNameComputed, + 15_000_000, + 1, + $submitDelayComputed, + '', + $objectNameComputed, + $this->sign('POST', $object_name, $expiration), + ); + } + + private function sign(string $method, string $object_name, int $expiration): string + { + return hash_hmac('sha512', $method, sprintf('%s.%s.%s.%d', $method, $this->secret, $object_name, $expiration)); } } diff --git a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php new file mode 100644 index 000000000..f7170abcd --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php @@ -0,0 +1,78 @@ +prophesize(UrlGeneratorInterface::class); + $urlGenerator->generate('chill_docstore_stored_object_operate', [ + 'object_name' => 'testABC', + 'exp' => $exp = 1734307200 + 180, + 'sig' => '528a426aebea86ada86035e9f3299788a074b77d3f926c6c15492457f81e88cfa98b270ba489d5d7588612eadaeeb7717a57887b33932d6d449b5f441252e600', + ], UrlGeneratorInterface::ABSOLUTE_URL) + ->shouldBeCalled() + ->willReturn($url = 'http://example.com/public/doc-store/stored-object/operate/testABC'); + + $generator = $this->buildGenerator($urlGenerator->reveal()); + + $signedUrl = $generator->generate('GET', 'testABC'); + + self::assertEquals($url, $signedUrl->url); + self::assertEquals('testABC', $signedUrl->object_name); + self::assertEquals($exp, $signedUrl->expires->getTimestamp()); + self::assertEquals('GET', $signedUrl->method); + } + + public function testGeneratePost(): void + { + $urlGenerator = $this->prophesize(UrlGeneratorInterface::class); + $urlGenerator->generate('chill_docstore_storedobject_post', [ + 'prefix' => 'prefixABC', + ], UrlGeneratorInterface::ABSOLUTE_URL) + ->shouldBeCalled() + ->willReturn($url = 'http://example.com/public/doc-store/stored-object/prefixABC'); + + $generator = $this->buildGenerator($urlGenerator->reveal()); + + $signedUrl = $generator->generatePost(object_name: 'prefixABC'); + + self::assertEquals($url, $signedUrl->url); + self::assertEquals('prefixABC', $signedUrl->object_name); + self::assertEquals(1734307200 + 180 + 180, $signedUrl->expires->getTimestamp()); + self::assertEquals('POST', $signedUrl->method); + self::assertEquals('47e9271917c6c9149a47248d88eeadcbc1f804138e445f3406d39f6aa51b2d976c2ee6e5b1196d2bf88852b627b330596c2fbf6f31b06837e9f41cf56103a4e4', $signedUrl->signature); + } + + private function buildGenerator(UrlGeneratorInterface $urlGenerator): TempUrlLocalStorageGenerator + { + return new TempUrlLocalStorageGenerator( + 'abc', + new MockClock('2024-12-16T00:00:00+00:00'), + $urlGenerator, + ); + } +} From c1e449f48e1887a6f9c01c952f53ed155979c567 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 19 Dec 2024 17:09:16 +0100 Subject: [PATCH 06/14] Implements StoredObjectManager for local storage --- .../LocalStorage/StoredObjectManager.php | 167 +++++++++++++++++- .../StoredObjectManager.php | 4 +- .../Entity/StoredObject.php | 8 + .../Entity/StoredObjectVersion.php | 5 + .../StoredObjectManagerException.php | 25 +++ .../Service/Cryptography/KeyGenerator.php | 59 +++++++ .../Service/StoredObjectManagerInterface.php | 2 + .../LocalStorage/StoredObjectManagerTest.php | 160 +++++++++++++++++ .../Service/Cryptography/KeyGeneratorTest.php | 47 +++++ 9 files changed, 466 insertions(+), 11 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Service/Cryptography/KeyGeneratorTest.php diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php index 7d5b39db8..537a46a56 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php @@ -11,49 +11,200 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage; +use Base64Url\Base64Url; use Chill\DocStoreBundle\Entity\StoredObject; use Chill\DocStoreBundle\Entity\StoredObjectVersion; +use Chill\DocStoreBundle\Exception\StoredObjectManagerException; +use Chill\DocStoreBundle\Service\Cryptography\KeyGenerator; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; +use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; +use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Filesystem\Path; class StoredObjectManager implements StoredObjectManagerInterface { + private readonly string $baseDir; + + private readonly Filesystem $filesystem; + + public function __construct( + ParameterBagInterface $parameterBag, + private readonly KeyGenerator $keyGenerator, + ) { + $this->baseDir = $parameterBag->get('chill_doc_store')['local_storage']['base_dir']; + $this->filesystem = new Filesystem(); + } + public function getLastModified(StoredObject|StoredObjectVersion $document): \DateTimeInterface { - // TODO: Implement getLastModified() method. + $version = $document instanceof StoredObject ? $document->getCurrentVersion() : $document; + + if (null === $version) { + throw StoredObjectManagerException::storedObjectDoesNotContainsVersion(); + } + + $path = $this->buildPath($version->getFilename()); + + if (false === $ts = filemtime($path)) { + throw StoredObjectManagerException::unableToReadDocumentOnDisk($path); + } + + return \DateTimeImmutable::createFromFormat('U', (string) $ts); } public function getContentLength(StoredObject|StoredObjectVersion $document): int { - // TODO: Implement getContentLength() method. + return strlen($this->read($document)); } public function exists(StoredObject|StoredObjectVersion $document): bool { - // TODO: Implement exists() method. + $version = $document instanceof StoredObject ? $document->getCurrentVersion() : $document; + + if (null === $version) { + return false; + } + + $path = $this->buildPath($version->getFilename()); + + return $this->filesystem->exists($path); } public function read(StoredObject|StoredObjectVersion $document): string { - // TODO: Implement read() method. + $version = $document instanceof StoredObject ? $document->getCurrentVersion() : $document; + + if (null === $version) { + throw StoredObjectManagerException::storedObjectDoesNotContainsVersion(); + } + + $path = $this->buildPath($version->getFilename()); + + if (!file_exists($path)) { + throw StoredObjectManagerException::unableToFindDocumentOnDisk($path); + } + + if (false === $content = file_get_contents($path)) { + throw StoredObjectManagerException::unableToReadDocumentOnDisk($path); + } + + if (!$this->isVersionEncrypted($version)) { + return $content; + } + + $clearData = openssl_decrypt( + $content, + self::ALGORITHM, + // TODO: Why using this library and not use base64_decode() ? + Base64Url::decode($version->getKeyInfos()['k']), + \OPENSSL_RAW_DATA, + pack('C*', ...$version->getIv()) + ); + + if (false === $clearData) { + throw StoredObjectManagerException::unableToDecrypt(openssl_error_string()); + } + + return $clearData; } public function write(StoredObject $document, string $clearContent, ?string $contentType = null): StoredObjectVersion { - // TODO: Implement write() method. + $newIv = $document->isEncrypted() ? $document->getIv() : $this->keyGenerator->generateIv(); + $newKey = $document->isEncrypted() ? $document->getKeyInfos() : $this->keyGenerator->generateKey(self::ALGORITHM); + $newType = $contentType ?? $document->getType(); + $version = $document->registerVersion( + $newIv, + $newKey, + $newType + ); + + $encryptedContent = $this->isVersionEncrypted($version) + ? openssl_encrypt( + $clearContent, + self::ALGORITHM, + // TODO: Why using this library and not use base64_decode() ? + Base64Url::decode($version->getKeyInfos()['k']), + \OPENSSL_RAW_DATA, + pack('C*', ...$version->getIv()) + ) + : $clearContent; + + if (false === $encryptedContent) { + throw StoredObjectManagerException::unableToEncryptDocument((string) openssl_error_string()); + } + + $fullPath = $this->buildPath($version->getFilename()); + $dir = Path::getDirectory($fullPath); + + if (!$this->filesystem->exists($dir)) { + $this->filesystem->mkdir($dir); + } + + $result = file_put_contents($fullPath, $encryptedContent); + + if (false === $result) { + throw StoredObjectManagerException::unableToStoreDocumentOnDisk(); + } + + return $version; + } + + private function buildPath(string $filename): string + { + $dirs = [$this->baseDir]; + + for ($i = 0; $i < min(strlen($filename), 8); ++$i) { + $dirs[] = $filename[$i]; + } + + $dirs[] = $filename; + + return Path::canonicalize(implode(DIRECTORY_SEPARATOR, $dirs)); } public function delete(StoredObjectVersion $storedObjectVersion): void { - // TODO: Implement delete() method. + if (!$this->exists($storedObjectVersion)) { + return; + } + + $path = $this->buildPath($storedObjectVersion->getFilename()); + + $this->filesystem->remove($path); + $this->removeDirectoriesRecursively(Path::getDirectory($path)); } + private function removeDirectoriesRecursively(string $path): void + { + if ($path === $this->baseDir) { + return; + } + + $files = scandir($path); + + // if it does contains only "." and "..", we can remove the directory + if (2 === count($files) && in_array('.', $files, true) && in_array('..', $files, true)) { + $this->filesystem->remove($path); + $this->removeDirectoriesRecursively(Path::getDirectory($path)); + } + } + + /** + * @throws StoredObjectManagerException + */ public function etag(StoredObject|StoredObjectVersion $document): string { - // TODO: Implement etag() method. + return md5($this->read($document)); } public function clearCache(): void { - // TODO: Implement clearCache() method. + // there is no cache: nothing to do here ! + } + + private function isVersionEncrypted(StoredObjectVersion $storedObjectVersion): bool + { + return $storedObjectVersion->isEncrypted(); } } diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php index 875a6f4fb..e74985f69 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/StoredObjectManager.php @@ -25,8 +25,6 @@ use Symfony\Contracts\HttpClient\ResponseInterface; final class StoredObjectManager implements StoredObjectManagerInterface { - private const ALGORITHM = 'AES-256-CBC'; - private array $inMemory = []; public function __construct( @@ -362,6 +360,6 @@ final class StoredObjectManager implements StoredObjectManagerInterface private function isVersionEncrypted(StoredObjectVersion $storedObjectVersion): bool { - return ([] !== $storedObjectVersion->getKeyInfos()) && ([] !== $storedObjectVersion->getIv()); + return $storedObjectVersion->isEncrypted(); } } diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php index d13452f76..c849abacc 100644 --- a/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php @@ -448,4 +448,12 @@ class StoredObject implements Document, TrackCreationInterface { return $storedObject->getDeleteAt() < $now && $storedObject->getVersions()->isEmpty(); } + + /** + * Return true if it has a current version, and if the current version is encrypted. + */ + public function isEncrypted(): bool + { + return $this->hasCurrentVersion() && $this->getCurrentVersion()->isEncrypted(); + } } diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php index 72532def7..18c78563a 100644 --- a/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObjectVersion.php @@ -226,4 +226,9 @@ class StoredObjectVersion implements TrackCreationInterface return $this; } + + public function isEncrypted(): bool + { + return ([] !== $this->getKeyInfos()) && ([] !== $this->getIv()); + } } diff --git a/src/Bundle/ChillDocStoreBundle/Exception/StoredObjectManagerException.php b/src/Bundle/ChillDocStoreBundle/Exception/StoredObjectManagerException.php index 68a978e80..4c6fe2675 100644 --- a/src/Bundle/ChillDocStoreBundle/Exception/StoredObjectManagerException.php +++ b/src/Bundle/ChillDocStoreBundle/Exception/StoredObjectManagerException.php @@ -34,4 +34,29 @@ final class StoredObjectManagerException extends \Exception { return new self('Unable to get content from response.', 500, $exception); } + + public static function unableToStoreDocumentOnDisk(?\Throwable $exception = null): self + { + return new self('Unable to store document on disk.', previous: $exception); + } + + public static function unableToFindDocumentOnDisk(string $path): self + { + return new self('Unable to find document on disk at path "'.$path.'".'); + } + + public static function unableToReadDocumentOnDisk(string $path): self + { + return new self('Unable to read document on disk at path "'.$path.'".'); + } + + public static function unableToEncryptDocument(string $errors): self + { + return new self('Unable to encrypt document: '.$errors); + } + + public static function storedObjectDoesNotContainsVersion(): self + { + return new self('Stored object does not contains any version'); + } } diff --git a/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php b/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php new file mode 100644 index 000000000..58520b352 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php @@ -0,0 +1,59 @@ +randomizer = new Randomizer(); + } + + /** + * @return array{alg: string, ext: bool, k: string, key_ops: list, kty: string} + */ + public function generateKey(string $algo = StoredObjectManagerInterface::ALGORITHM): array + { + if (StoredObjectManagerInterface::ALGORITHM !== $algo) { + throw new \LogicException(sprintf("Algorithm '%s' is not supported.", $algo)); + } + + $key = $this->randomizer->getBytes(128); + + return [ + 'alg' => 'A256CBC', + 'ext' => true, + 'k' => Base64Url::encode($key), + 'key_ops' => ['encrypt', 'decrypt'], + 'kty' => 'oct', + ]; + } + + /** + * @return list> + */ + public function generateIv(): array + { + $iv = []; + for ($i = 0; $i < 16; ++$i) { + $iv[] = unpack('C', $this->randomizer->getBytes(8))[1]; + } + + return $iv; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php index b4c7a063e..53a0d7d07 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php +++ b/src/Bundle/ChillDocStoreBundle/Service/StoredObjectManagerInterface.php @@ -18,6 +18,8 @@ use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface; interface StoredObjectManagerInterface { + public const ALGORITHM = 'AES-256-CBC'; + /** * @param StoredObject|StoredObjectVersion $document if a StoredObject is given, the last version will be used */ diff --git a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php new file mode 100644 index 000000000..695e0caed --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php @@ -0,0 +1,160 @@ +buildStoredObjectManager(); + $version = $manager->write($storedObject, self::CONTENT); + + self::assertSame($storedObject, $version->getStoredObject()); + + return $version; + } + + /** + * @depends testWrite + */ + public function testRead(StoredObjectVersion $version): StoredObjectVersion + { + $manager = $this->buildStoredObjectManager(); + $content = $manager->read($version); + + self::assertEquals(self::CONTENT, $content); + + return $version; + } + + /** + * @depends testRead + */ + public function testExists(StoredObjectVersion $version): StoredObjectVersion + { + $manager = $this->buildStoredObjectManager(); + $notExisting = new StoredObject(); + $versionNotPersisted = $notExisting->registerVersion(); + + self::assertTrue($manager->exists($version)); + self::assertFalse($manager->exists($versionNotPersisted)); + self::assertFalse($manager->exists(new StoredObject())); + + return $version; + } + + /** + * @throws \Chill\DocStoreBundle\Exception\StoredObjectManagerException + * + * @depends testExists + */ + public function testEtag(StoredObjectVersion $version): StoredObjectVersion + { + $manager = $this->buildStoredObjectManager(); + $actual = $manager->etag($version); + + self::assertEquals(md5(self::CONTENT), $actual); + + return $version; + } + + /** + * @depends testEtag + */ + public function testGetContentLength(StoredObjectVersion $version): StoredObjectVersion + { + $manager = $this->buildStoredObjectManager(); + + $actual = $manager->getContentLength($version); + + self::assertSame(5, $actual); + + return $version; + } + + /** + * @throws \Chill\DocStoreBundle\Exception\StoredObjectManagerException + * + * @depends testGetContentLength + */ + public function testGetLastModified(StoredObjectVersion $version): StoredObjectVersion + { + $manager = $this->buildStoredObjectManager(); + $actual = $manager->getLastModified($version); + + self::assertInstanceOf(\DateTimeImmutable::class, $actual); + self::assertGreaterThan((new \DateTimeImmutable('now'))->getTimestamp() - 10, $actual->getTimestamp()); + + return $version; + } + + /** + * @depends testGetLastModified + */ + public function testDelete(StoredObjectVersion $version): void + { + $manager = $this->buildStoredObjectManager(); + $manager->delete($version); + + self::assertFalse($manager->exists($version)); + } + + public function testDeleteDoesNotRemoveOlderVersion(): void + { + $storedObject = new StoredObject(); + $manager = $this->buildStoredObjectManager(); + $version1 = $manager->write($storedObject, 'version1'); + $version2 = $manager->write($storedObject, 'version2'); + $version3 = $manager->write($storedObject, 'version3'); + + self::assertTrue($manager->exists($version1)); + self::assertEquals('version1', $manager->read($version1)); + self::assertTrue($manager->exists($version2)); + self::assertEquals('version2', $manager->read($version2)); + self::assertTrue($manager->exists($version3)); + self::assertEquals('version3', $manager->read($version3)); + + // we delete the intermediate version + $manager->delete($version2); + + self::assertFalse($manager->exists($version2)); + // we check that we are still able to download the other versions + self::assertTrue($manager->exists($version1)); + self::assertEquals('version1', $manager->read($version1)); + self::assertTrue($manager->exists($version3)); + self::assertEquals('version3', $manager->read($version3)); + } + + private function buildStoredObjectManager(): StoredObjectManager + { + return new StoredObjectManager( + new ParameterBag(['chill_doc_store' => ['local_storage' => ['base_dir' => '/tmp/chill-local-storage-test']]]), + new KeyGenerator(), + ); + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Service/Cryptography/KeyGeneratorTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Service/Cryptography/KeyGeneratorTest.php new file mode 100644 index 000000000..07fb09a8f --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Service/Cryptography/KeyGeneratorTest.php @@ -0,0 +1,47 @@ +generateKey(); + + self::assertNotEmpty($key['k']); + self::assertEquals('A256CBC', $key['alg']); + } + + public function testGenerateIv(): void + { + $keyGenerator = new KeyGenerator(); + + $actual = $keyGenerator->generateIv(); + + self::assertCount(16, $actual); + foreach ($actual as $value) { + self::assertIsInt($value); + self::assertGreaterThanOrEqual(0, $value); + self::assertLessThan(256, $value); + } + } +} From 83f7086bb09c785fe1fbc453e1b2764f06aa1eb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 19 Dec 2024 21:32:37 +0100 Subject: [PATCH 07/14] Configure DI for providing kernel secret for TempUrlLocalStorageGenerator --- src/Bundle/ChillDocStoreBundle/config/services.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Bundle/ChillDocStoreBundle/config/services.yaml b/src/Bundle/ChillDocStoreBundle/config/services.yaml index 1ca7fe4b9..d8b4b0b61 100644 --- a/src/Bundle/ChillDocStoreBundle/config/services.yaml +++ b/src/Bundle/ChillDocStoreBundle/config/services.yaml @@ -50,6 +50,10 @@ services: Chill\DocStoreBundle\AsyncUpload\Driver\: resource: '../AsyncUpload/Driver/' + Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\TempUrlLocalStorageGenerator: + arguments: + $secret: '%kernel.secret%' + Chill\DocStoreBundle\AsyncUpload\Templating\: resource: '../AsyncUpload/Templating/' From c65f1d495df751cbca858ecd316769f130574811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 19 Dec 2024 21:33:55 +0100 Subject: [PATCH 08/14] Refactor ConfigureOpenstackObjectStorageCommand - change namespace for more obvious handling; - remove command of local storage is configured --- .../ConfigureOpenstackObjectStorageCommand.php | 2 +- .../Compiler/StorageConfigurationCompilerPass.php | 2 ++ .../ConfigureOpenstackObjectStorageCommandTest.php | 4 ++-- src/Bundle/ChillDocStoreBundle/config/services.yaml | 3 --- 4 files changed, 5 insertions(+), 6 deletions(-) rename src/Bundle/ChillDocStoreBundle/AsyncUpload/{Command => Driver/OpenstackObjectStore}/ConfigureOpenstackObjectStorageCommand.php (97%) rename src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/{Command => Driver/OpenstackObjectStore}/ConfigureOpenstackObjectStorageCommandTest.php (91%) diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Command/ConfigureOpenstackObjectStorageCommand.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/ConfigureOpenstackObjectStorageCommand.php similarity index 97% rename from src/Bundle/ChillDocStoreBundle/AsyncUpload/Command/ConfigureOpenstackObjectStorageCommand.php rename to src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/ConfigureOpenstackObjectStorageCommand.php index fd1b41481..13e0a6bbe 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Command/ConfigureOpenstackObjectStorageCommand.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/ConfigureOpenstackObjectStorageCommand.php @@ -9,7 +9,7 @@ declare(strict_types=1); * the LICENSE file that was distributed with this source code. */ -namespace Chill\DocStoreBundle\AsyncUpload\Command; +namespace Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php index f36cba2f4..6b6d5573b 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php @@ -12,6 +12,7 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\DependencyInjection\Compiler; use Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\TempUrlLocalStorageGenerator; +use Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\ConfigureOpenstackObjectStorageCommand; use Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\TempUrlOpenstackGenerator; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; @@ -24,6 +25,7 @@ class StorageConfigurationCompilerPass implements CompilerPassInterface private const SERVICES_OPENSTACK = [ \Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\StoredObjectManager::class, TempUrlOpenstackGenerator::class, + ConfigureOpenstackObjectStorageCommand::class, ]; private const SERVICES_LOCAL_STORAGE = [ diff --git a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Command/ConfigureOpenstackObjectStorageCommandTest.php b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/OpenstackObjectStore/ConfigureOpenstackObjectStorageCommandTest.php similarity index 91% rename from src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Command/ConfigureOpenstackObjectStorageCommandTest.php rename to src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/OpenstackObjectStore/ConfigureOpenstackObjectStorageCommandTest.php index e0318313c..ccb9920d4 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Command/ConfigureOpenstackObjectStorageCommandTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/OpenstackObjectStore/ConfigureOpenstackObjectStorageCommandTest.php @@ -9,9 +9,9 @@ declare(strict_types=1); * the LICENSE file that was distributed with this source code. */ -namespace AsyncUpload\Command; +namespace Chill\DocStoreBundle\Tests\AsyncUpload\Driver\OpenstackObjectStore; -use Chill\DocStoreBundle\AsyncUpload\Command\ConfigureOpenstackObjectStorageCommand; +use Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\ConfigureOpenstackObjectStorageCommand; use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Tester\CommandTester; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; diff --git a/src/Bundle/ChillDocStoreBundle/config/services.yaml b/src/Bundle/ChillDocStoreBundle/config/services.yaml index d8b4b0b61..070731b54 100644 --- a/src/Bundle/ChillDocStoreBundle/config/services.yaml +++ b/src/Bundle/ChillDocStoreBundle/config/services.yaml @@ -56,6 +56,3 @@ services: Chill\DocStoreBundle\AsyncUpload\Templating\: resource: '../AsyncUpload/Templating/' - - Chill\DocStoreBundle\AsyncUpload\Command\: - resource: '../AsyncUpload/Command/' From 0c628c39dbadec5b41be480848ebccdd8a7ba458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 20 Dec 2024 21:23:02 +0100 Subject: [PATCH 09/14] store encrypted content --- .../LocalStorage/StoredObjectManager.php | 47 +++++++++---- ...dObjectContentToLocalStorageController.php | 69 +++++++++++++++++++ .../StorageConfigurationCompilerPass.php | 2 + .../LocalStorage/StoredObjectManagerTest.php | 2 +- 4 files changed, 104 insertions(+), 16 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php index 537a46a56..09d277635 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/StoredObjectManager.php @@ -31,7 +31,7 @@ class StoredObjectManager implements StoredObjectManagerInterface ParameterBagInterface $parameterBag, private readonly KeyGenerator $keyGenerator, ) { - $this->baseDir = $parameterBag->get('chill_doc_store')['local_storage']['base_dir']; + $this->baseDir = $parameterBag->get('chill_doc_store')['local_storage']['storage_path']; $this->filesystem = new Filesystem(); } @@ -65,9 +65,7 @@ class StoredObjectManager implements StoredObjectManagerInterface return false; } - $path = $this->buildPath($version->getFilename()); - - return $this->filesystem->exists($path); + return $this->existsContent($version->getFilename()); } public function read(StoredObject|StoredObjectVersion $document): string @@ -78,15 +76,7 @@ class StoredObjectManager implements StoredObjectManagerInterface throw StoredObjectManagerException::storedObjectDoesNotContainsVersion(); } - $path = $this->buildPath($version->getFilename()); - - if (!file_exists($path)) { - throw StoredObjectManagerException::unableToFindDocumentOnDisk($path); - } - - if (false === $content = file_get_contents($path)) { - throw StoredObjectManagerException::unableToReadDocumentOnDisk($path); - } + $content = $this->readContent($version->getFilename()); if (!$this->isVersionEncrypted($version)) { return $content; @@ -134,7 +124,29 @@ class StoredObjectManager implements StoredObjectManagerInterface throw StoredObjectManagerException::unableToEncryptDocument((string) openssl_error_string()); } - $fullPath = $this->buildPath($version->getFilename()); + $this->writeContent($version->getFilename(), $encryptedContent); + + return $version; + } + + public function readContent(string $filename): string + { + $path = $this->buildPath($filename); + + if (!file_exists($path)) { + throw StoredObjectManagerException::unableToFindDocumentOnDisk($path); + } + + if (false === $content = file_get_contents($path)) { + throw StoredObjectManagerException::unableToReadDocumentOnDisk($path); + } + + return $content; + } + + public function writeContent(string $filename, string $encryptedContent): void + { + $fullPath = $this->buildPath($filename); $dir = Path::getDirectory($fullPath); if (!$this->filesystem->exists($dir)) { @@ -146,8 +158,13 @@ class StoredObjectManager implements StoredObjectManagerInterface if (false === $result) { throw StoredObjectManagerException::unableToStoreDocumentOnDisk(); } + } - return $version; + public function existsContent(string $filename): bool + { + $path = $this->buildPath($filename); + + return $this->filesystem->exists($path); } private function buildPath(string $filename): string diff --git a/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php b/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php new file mode 100644 index 000000000..b7a53514e --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php @@ -0,0 +1,69 @@ +query->get('prefix', ''); + + if ('' === $prefix) { + throw new BadRequestHttpException('prefix parameter is missing'); + } + + $keyFiles = $request->files->keys(); + + foreach ($keyFiles as $keyFile) { + /** @var UploadedFile $file */ + $file = $request->files->get($keyFile); + $this->storedObjectManager->writeContent($keyFile, $file->getContent()); + } + + return new Response(status: Response::HTTP_NO_CONTENT); + } + + #[Route('/public/stored-object/operate', name: 'chill_docstore_stored_object_operate', methods: ['GET', 'HEAD'])] + public function contentOperate(Request $request): Response + { + $objectName = $request->query->get('object_name', ''); + + if ('' === $objectName) { + throw new BadRequestHttpException('object name parameter is missing'); + } + + if (!$this->storedObjectManager->existsContent($objectName)) { + throw new NotFoundHttpException('object does not exists on disk'); + } + + return match ($request->getMethod()) { + 'GET' => new Response($this->storedObjectManager->readContent($objectName)), + 'HEAD' => new Response(''), + }; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php index 6b6d5573b..075103c59 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php @@ -15,6 +15,7 @@ use Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\TempUrlLocalStorageGene use Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\ConfigureOpenstackObjectStorageCommand; use Chill\DocStoreBundle\AsyncUpload\Driver\OpenstackObjectStore\TempUrlOpenstackGenerator; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; +use Chill\DocStoreBundle\Controller\StoredObjectContentToLocalStorageController; use Chill\DocStoreBundle\Service\StoredObjectManagerInterface; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; @@ -31,6 +32,7 @@ class StorageConfigurationCompilerPass implements CompilerPassInterface private const SERVICES_LOCAL_STORAGE = [ \Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\StoredObjectManager::class, TempUrlLocalStorageGenerator::class, + StoredObjectContentToLocalStorageController::class, ]; public function process(ContainerBuilder $container) diff --git a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php index 695e0caed..e91fed7f0 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/StoredObjectManagerTest.php @@ -153,7 +153,7 @@ class StoredObjectManagerTest extends TestCase private function buildStoredObjectManager(): StoredObjectManager { return new StoredObjectManager( - new ParameterBag(['chill_doc_store' => ['local_storage' => ['base_dir' => '/tmp/chill-local-storage-test']]]), + new ParameterBag(['chill_doc_store' => ['local_storage' => ['storage_path' => '/tmp/chill-local-storage-test']]]), new KeyGenerator(), ); } From 999ac3af2bc1965a960a15f2b3fd331aa49dc765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Mon, 6 Jan 2025 14:37:35 +0100 Subject: [PATCH 10/14] Add TempUrl signature validation to local storage Implemented local storage-based file handling with TempUrl signature validation for upload and retrieval. Added validation checks for parameters like max file size/count, expiration, and signature integrity. Included unit tests for TempUrl signature validation and adjusted configuration for local storage. --- config/packages/chill_doc_store.yaml | 12 +- .../TempUrlLocalStorageGenerator.php | 38 +- ...dObjectContentToLocalStorageController.php | 61 +++- .../TempUrlLocalStorageGeneratorTest.php | 184 +++++++++- ...ectContentToLocalStorageControllerTest.php | 338 ++++++++++++++++++ .../dummy_file | 1 + 6 files changed, 609 insertions(+), 25 deletions(-) create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Controller/StoredObjectContentToLocalStorageControllerTest.php create mode 100644 src/Bundle/ChillDocStoreBundle/Tests/Controller/data/StoredObjectContentToLocalStorageControllerTest/dummy_file diff --git a/config/packages/chill_doc_store.yaml b/config/packages/chill_doc_store.yaml index 994e19240..e6cfd10ec 100644 --- a/config/packages/chill_doc_store.yaml +++ b/config/packages/chill_doc_store.yaml @@ -1,6 +1,8 @@ chill_doc_store: - openstack: - temp_url: - temp_url_key: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_KEY)%' # Required - container: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_CONTAINER)%' # Required - temp_url_base_path: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_BASE_PATH)%' # Required + local_storage: + storage_path: '%kernel.project_dir%/var/storage' +# openstack: +# temp_url: +# temp_url_key: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_KEY)%' # Required +# container: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_CONTAINER)%' # Required +# temp_url_base_path: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_BASE_PATH)%' # Required diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php index 957413b3a..40ee009c5 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGenerator.php @@ -33,11 +33,11 @@ class TempUrlLocalStorageGenerator implements TempUrlGeneratorInterface $expiration = $this->clock->now()->getTimestamp() + min($expire_delay ?? self::SIGNATURE_DURATION, self::SIGNATURE_DURATION); return new SignedUrl( - $method, + strtoupper($method), $this->urlGenerator->generate('chill_docstore_stored_object_operate', [ 'object_name' => $object_name, 'exp' => $expiration, - 'sig' => $this->sign($method, $object_name, $expiration), + 'sig' => $this->sign(strtoupper($method), $object_name, $expiration), ], UrlGeneratorInterface::ABSOLUTE_URL), \DateTimeImmutable::createFromFormat('U', (string) $expiration), $object_name, @@ -70,6 +70,38 @@ class TempUrlLocalStorageGenerator implements TempUrlGeneratorInterface private function sign(string $method, string $object_name, int $expiration): string { - return hash_hmac('sha512', $method, sprintf('%s.%s.%s.%d', $method, $this->secret, $object_name, $expiration)); + return hash('sha512', sprintf('%s.%s.%s.%d', $method, $this->secret, $object_name, $expiration)); + } + + public function validateSignaturePost(string $signature, string $prefix, int $expiration, int $maxFileSize, int $maxFileCount): bool + { + if (15_000_000 !== $maxFileSize || 1 !== $maxFileCount) { + return false; + } + + return $this->internalValidateSignature($signature, 'POST', $prefix, $expiration); + } + + private function internalValidateSignature(string $signature, string $method, string $object_name, int $expiration): bool + { + if ($expiration < $this->clock->now()->format('U')) { + return false; + } + + if ('' === $object_name) { + return false; + } + + + return $this->sign($method, $object_name, $expiration) === $signature; + } + + public function validateSignature(string $signature, string $method, string $objectName, int $expiration): bool + { + if (!in_array($method, ['GET', 'HEAD'], true)) { + return false; + } + + return $this->internalValidateSignature($signature, $method, $objectName, $expiration); } } diff --git a/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php b/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php index b7a53514e..1d7fdf8b4 100644 --- a/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php +++ b/src/Bundle/ChillDocStoreBundle/Controller/StoredObjectContentToLocalStorageController.php @@ -12,9 +12,11 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\Controller; use Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\StoredObjectManager; +use Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\TempUrlLocalStorageGenerator; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Routing\Annotation\Route; @@ -26,6 +28,7 @@ final readonly class StoredObjectContentToLocalStorageController { public function __construct( private StoredObjectManager $storedObjectManager, + private TempUrlLocalStorageGenerator $tempUrlLocalStorageGenerator, ) {} #[Route('/public/stored-object/post', name: 'chill_docstore_storedobject_post', methods: ['POST'])] @@ -34,14 +37,51 @@ final readonly class StoredObjectContentToLocalStorageController $prefix = $request->query->get('prefix', ''); if ('' === $prefix) { - throw new BadRequestHttpException('prefix parameter is missing'); + throw new BadRequestHttpException('Prefix parameter is missing'); + } + + if (0 === $maxFileSize = $request->request->getInt('max_file_size', 0)) { + throw new BadRequestHttpException('Max file size is not set or equal to zero'); + } + + if (1 !== $maxFileCount = $request->request->getInt('max_file_count', 0)) { + throw new BadRequestHttpException('Max file count is not set or equal to zero'); + } + + if (0 === $expiration = $request->request->getInt('expires', 0)) { + throw new BadRequestHttpException('Expiration is not set or equal to zero'); + } + + if ('' === $signature = $request->request->get('signature', '')) { + throw new BadRequestHttpException('Signature is not set or is a blank string'); + } + + if (!$this->tempUrlLocalStorageGenerator->validateSignaturePost($signature, $prefix, $expiration, $maxFileSize, $maxFileCount)) { + throw new AccessDeniedHttpException('Invalid signature'); } $keyFiles = $request->files->keys(); + if ($maxFileCount < count($keyFiles)) { + throw new AccessDeniedHttpException('More files than max file count'); + } + + if (0 === count($keyFiles)) { + throw new BadRequestHttpException('Zero files given'); + } + foreach ($keyFiles as $keyFile) { /** @var UploadedFile $file */ $file = $request->files->get($keyFile); + + if ($maxFileSize < strlen($file->getContent())) { + throw new AccessDeniedHttpException('File is too big'); + } + + if (!str_starts_with((string) $keyFile, $prefix)) { + throw new AccessDeniedHttpException('Filename does not start with signed prefix'); + } + $this->storedObjectManager->writeContent($keyFile, $file->getContent()); } @@ -51,19 +91,30 @@ final readonly class StoredObjectContentToLocalStorageController #[Route('/public/stored-object/operate', name: 'chill_docstore_stored_object_operate', methods: ['GET', 'HEAD'])] public function contentOperate(Request $request): Response { - $objectName = $request->query->get('object_name', ''); + if ('' === $objectName = $request->query->get('object_name', '')) { + throw new BadRequestHttpException('Object name parameter is missing'); + } - if ('' === $objectName) { - throw new BadRequestHttpException('object name parameter is missing'); + if (0 === $expiration = $request->query->getInt('exp', 0)) { + throw new BadRequestHttpException('Expiration is not set or equal to zero'); + } + + if ('' === $signature = $request->query->get('sig', '')) { + throw new BadRequestHttpException('Signature is not set or is a blank string'); + } + + if (!$this->tempUrlLocalStorageGenerator->validateSignature($signature, strtoupper($request->getMethod()), $objectName, $expiration)) { + throw new AccessDeniedHttpException('Invalid signature'); } if (!$this->storedObjectManager->existsContent($objectName)) { - throw new NotFoundHttpException('object does not exists on disk'); + throw new NotFoundHttpException('Object does not exists on disk'); } return match ($request->getMethod()) { 'GET' => new Response($this->storedObjectManager->readContent($objectName)), 'HEAD' => new Response(''), + default => throw new BadRequestHttpException('method not supported'), }; } } diff --git a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php index f7170abcd..d85d3094a 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/AsyncUpload/Driver/LocalStorage/TempUrlLocalStorageGeneratorTest.php @@ -14,6 +14,7 @@ namespace Chill\DocStoreBundle\Tests\AsyncUpload\Driver\LocalStorage; use Chill\DocStoreBundle\AsyncUpload\Driver\LocalStorage\TempUrlLocalStorageGenerator; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; +use Symfony\Component\Clock\ClockInterface; use Symfony\Component\Clock\MockClock; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; @@ -26,24 +27,26 @@ class TempUrlLocalStorageGeneratorTest extends TestCase { use ProphecyTrait; + private const SECRET = 'abc'; + public function testGenerate(): void { $urlGenerator = $this->prophesize(UrlGeneratorInterface::class); $urlGenerator->generate('chill_docstore_stored_object_operate', [ - 'object_name' => 'testABC', - 'exp' => $exp = 1734307200 + 180, - 'sig' => '528a426aebea86ada86035e9f3299788a074b77d3f926c6c15492457f81e88cfa98b270ba489d5d7588612eadaeeb7717a57887b33932d6d449b5f441252e600', + 'object_name' => $object_name = 'testABC', + 'exp' => $expiration = 1734307200 + 180, + 'sig' => TempUrlLocalStorageGeneratorTest::expectedSignature('GET', $object_name, $expiration), ], UrlGeneratorInterface::ABSOLUTE_URL) ->shouldBeCalled() ->willReturn($url = 'http://example.com/public/doc-store/stored-object/operate/testABC'); $generator = $this->buildGenerator($urlGenerator->reveal()); - $signedUrl = $generator->generate('GET', 'testABC'); + $signedUrl = $generator->generate('GET', $object_name); self::assertEquals($url, $signedUrl->url); - self::assertEquals('testABC', $signedUrl->object_name); - self::assertEquals($exp, $signedUrl->expires->getTimestamp()); + self::assertEquals($object_name, $signedUrl->object_name); + self::assertEquals($expiration, $signedUrl->expires->getTimestamp()); self::assertEquals('GET', $signedUrl->method); } @@ -62,17 +65,174 @@ class TempUrlLocalStorageGeneratorTest extends TestCase self::assertEquals($url, $signedUrl->url); self::assertEquals('prefixABC', $signedUrl->object_name); - self::assertEquals(1734307200 + 180 + 180, $signedUrl->expires->getTimestamp()); + self::assertEquals($expiration = 1734307200 + 180 + 180, $signedUrl->expires->getTimestamp()); self::assertEquals('POST', $signedUrl->method); - self::assertEquals('47e9271917c6c9149a47248d88eeadcbc1f804138e445f3406d39f6aa51b2d976c2ee6e5b1196d2bf88852b627b330596c2fbf6f31b06837e9f41cf56103a4e4', $signedUrl->signature); + self::assertEquals(TempUrlLocalStorageGeneratorTest::expectedSignature('POST', 'prefixABC', $expiration), $signedUrl->signature); } - private function buildGenerator(UrlGeneratorInterface $urlGenerator): TempUrlLocalStorageGenerator + private static function expectedSignature(string $method, $objectName, int $expiration): string + { + return hash('sha512', sprintf('%s.%s.%s.%d', $method, self::SECRET, $objectName, $expiration)); + } + + /** + * @dataProvider generateValidateSignatureData + */ + public function testValidateSignature(string $signature, string $method, string $objectName, int $expiration, \DateTimeImmutable $now, bool $expected, string $message): void + { + $urlGenerator = $this->buildGenerator(clock: new MockClock($now)); + + self::assertEquals($expected, $urlGenerator->validateSignature($signature, $method, $objectName, $expiration), $message); + } + + /** + * @dataProvider generateValidateSignaturePostData + */ + public function testValidateSignaturePost(string $signature, int $expiration, string $objectName, int $maxFileSize, int $maxFileCount, \DateTimeImmutable $now, bool $expected, string $message): void + { + $urlGenerator = $this->buildGenerator(clock: new MockClock($now)); + + self::assertEquals($expected, $urlGenerator->validateSignaturePost($signature, $objectName, $expiration, $maxFileSize, $maxFileCount), $message); + } + + public static function generateValidateSignaturePostData(): iterable + { + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180), + $expiration, + $object_name, + 15_000_000, + 1, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + true, + 'Valid signature', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180), + $expiration, + $object_name, + 15_000_001, + 1, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Wrong max file size', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180), + $expiration, + $object_name, + 15_000_000, + 2, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Wrong max file count', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180), + $expiration, + $object_name.'AAA', + 15_000_000, + 1, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Invalid object name', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180).'A', + $expiration, + $object_name, + 15_000_000, + 1, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Invalid signature', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180), + $expiration, + $object_name, + 15_000_000, + 1, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration + 1)), + false, + 'Expired signature', + ]; + } + + public static function generateValidateSignatureData(): iterable + { + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('GET', $object_name = 'testABC', $expiration = 1734307200 + 180), + 'GET', + $object_name, + $expiration, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + true, + 'Valid signature, not expired', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('HEAD', $object_name = 'testABC', $expiration = 1734307200 + 180), + 'HEAD', + $object_name, + $expiration, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + true, + 'Valid signature, not expired', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('GET', $object_name = 'testABC', $expiration = 1734307200 + 180).'A', + 'GET', + $object_name, + $expiration, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Invalid signature', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('GET', $object_name = 'testABC', $expiration = 1734307200 + 180), + 'GET', + $object_name, + $expiration, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration + 1)), + false, + 'Signature expired', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('GET', $object_name = 'testABC', $expiration = 1734307200 + 180), + 'GET', + $object_name.'____', + $expiration, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Invalid object name', + ]; + + yield [ + TempUrlLocalStorageGeneratorTest::expectedSignature('POST', $object_name = 'testABC', $expiration = 1734307200 + 180), + 'POST', + $object_name, + $expiration, + \DateTimeImmutable::createFromFormat('U', (string) ($expiration - 10)), + false, + 'Wrong method', + ]; + } + + private function buildGenerator(?UrlGeneratorInterface $urlGenerator = null, ?ClockInterface $clock = null): TempUrlLocalStorageGenerator { return new TempUrlLocalStorageGenerator( - 'abc', - new MockClock('2024-12-16T00:00:00+00:00'), - $urlGenerator, + self::SECRET, + $clock ?? new MockClock('2024-12-16T00:00:00+00:00'), + $urlGenerator ?? $this->prophesize(UrlGeneratorInterface::class)->reveal(), ); } } diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Controller/StoredObjectContentToLocalStorageControllerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Controller/StoredObjectContentToLocalStorageControllerTest.php new file mode 100644 index 000000000..c680bd0b3 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Controller/StoredObjectContentToLocalStorageControllerTest.php @@ -0,0 +1,338 @@ +prophesize(StoredObjectManager::class); + $storedObjectManager->existsContent(Argument::any())->willReturn($existContent); + $storedObjectManager->readContent(Argument::any())->willReturn($readContent); + + $tempUrlLocalStorageGenerator = $this->prophesize(TempUrlLocalStorageGenerator::class); + $tempUrlLocalStorageGenerator->validateSignature( + $request->query->get('sig', ''), + $request->getMethod(), + $request->query->get('object_name', ''), + $request->query->getInt('exp', 0) + ) + ->willReturn($signatureValidity); + + $this->expectException($expectedException); + $this->expectExceptionMessage($expectedExceptionMessage); + + $controller = new StoredObjectContentToLocalStorageController( + $storedObjectManager->reveal(), + $tempUrlLocalStorageGenerator->reveal() + ); + + $controller->contentOperate($request); + } + + public function testOperateContentGetHappyScenario(): void + { + $objectName = 'testABC'; + $expiration = new \DateTimeImmutable(); + $storedObjectManager = $this->prophesize(StoredObjectManager::class); + $storedObjectManager->existsContent($objectName)->willReturn(true); + $storedObjectManager->readContent($objectName)->willReturn('123456789'); + + $tempUrlLocalStorageGenerator = $this->prophesize(TempUrlLocalStorageGenerator::class); + $tempUrlLocalStorageGenerator->validateSignature('signature', 'GET', $objectName, $expiration->getTimestamp()) + ->shouldBeCalled() + ->willReturn(true); + + $controller = new StoredObjectContentToLocalStorageController( + $storedObjectManager->reveal(), + $tempUrlLocalStorageGenerator->reveal() + ); + + $response = $controller->contentOperate(new Request(['object_name' => $objectName, 'sig' => 'signature', 'exp' => $expiration->getTimestamp()])); + + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals('123456789', $response->getContent()); + } + + public function testOperateContentHeadHappyScenario(): void + { + $objectName = 'testABC'; + $expiration = new \DateTimeImmutable(); + $storedObjectManager = $this->prophesize(StoredObjectManager::class); + $storedObjectManager->existsContent($objectName)->willReturn(true); + $storedObjectManager->readContent($objectName)->willReturn('123456789'); + + $tempUrlLocalStorageGenerator = $this->prophesize(TempUrlLocalStorageGenerator::class); + $tempUrlLocalStorageGenerator->validateSignature('signature', 'HEAD', $objectName, $expiration->getTimestamp()) + ->shouldBeCalled() + ->willReturn(true); + + $controller = new StoredObjectContentToLocalStorageController( + $storedObjectManager->reveal(), + $tempUrlLocalStorageGenerator->reveal() + ); + + $request = new Request(['object_name' => $objectName, 'sig' => 'signature', 'exp' => $expiration->getTimestamp()]); + $request->setMethod('HEAD'); + $response = $controller->contentOperate($request); + + self::assertEquals(200, $response->getStatusCode()); + self::assertEquals('', $response->getContent()); + } + + public function testPostContentHappyScenario(): void + { + $expiration = 171899000; + $storedObjectManager = $this->prophesize(StoredObjectManager::class); + $storedObjectManager->writeContent('filePrefix/abcSUFFIX', Argument::containingString('fake_encrypted_content')) + ->shouldBeCalled(); + + $tempUrlGenerator = $this->prophesize(TempUrlLocalStorageGenerator::class); + $tempUrlGenerator->validateSignaturePost('signature', 'filePrefix/abc', $expiration, 15_000_000, 1) + ->shouldBeCalled() + ->willReturn(true); + + $controller = new StoredObjectContentToLocalStorageController($storedObjectManager->reveal(), $tempUrlGenerator->reveal()); + + $request = new Request( + ['prefix' => 'filePrefix/abc'], + ['signature' => 'signature', 'expires' => $expiration, 'max_file_size' => 15_000_000, 'max_file_count' => 1], + files: [ + 'filePrefix/abcSUFFIX' => new UploadedFile( + __DIR__.'/data/StoredObjectContentToLocalStorageControllerTest/dummy_file', + 'Document.odt', + test: true + ), + ] + ); + + $response = $controller->postContent($request); + + self::assertEquals(204, $response->getStatusCode()); + } + + /** + * @dataProvider generatePostContentWithExceptionDataProvider + */ + public function testPostContentWithException(Request $request, bool $isSignatureValid, string $expectedException, string $expectedExceptionMessage): void + { + $storedObjectManager = $this->prophesize(StoredObjectManager::class); + $storedObjectManager->writeContent(Argument::any(), Argument::any())->shouldNotBeCalled(); + + $tempUrlGenerator = $this->prophesize(TempUrlLocalStorageGenerator::class); + $tempUrlGenerator->validateSignaturePost('signature', Argument::any(), Argument::any(), Argument::any(), Argument::any()) + ->willReturn($isSignatureValid); + + $controller = new StoredObjectContentToLocalStorageController( + $storedObjectManager->reveal(), + $tempUrlGenerator->reveal() + ); + + $this->expectException($expectedException); + $this->expectExceptionMessage($expectedExceptionMessage); + + $controller->postContent($request); + } + + public static function generatePostContentWithExceptionDataProvider(): iterable + { + $query = ['prefix' => 'filePrefix/abc']; + $attributes = ['signature' => 'signature', 'expires' => 15088556855, 'max_file_size' => 15_000_000, 'max_file_count' => 1]; + + $request = new Request([]); + $request->setMethod('POST'); + + yield [ + $request, + true, + BadRequestHttpException::class, + 'Prefix parameter is missing', + ]; + + + $attrCloned = [...$attributes]; + unset($attrCloned['max_file_size']); + $request = new Request($query, $attrCloned); + $request->setMethod('POST'); + + yield [ + $request, + true, + BadRequestHttpException::class, + 'Max file size is not set or equal to zero', + ]; + + $attrCloned = [...$attributes]; + unset($attrCloned['max_file_count']); + $request = new Request($query, $attrCloned); + $request->setMethod('POST'); + + yield [ + $request, + true, + BadRequestHttpException::class, + 'Max file count is not set or equal to zero', + ]; + + $attrCloned = [...$attributes]; + unset($attrCloned['expires']); + $request = new Request($query, $attrCloned); + $request->setMethod('POST'); + + yield [ + $request, + true, + BadRequestHttpException::class, + 'Expiration is not set or equal to zero', + ]; + + $attrCloned = [...$attributes]; + unset($attrCloned['signature']); + $request = new Request($query, $attrCloned); + $request->setMethod('POST'); + + yield [ + $request, + true, + BadRequestHttpException::class, + 'Signature is not set or is a blank string', + ]; + + $request = new Request($query, $attributes); + $request->setMethod('POST'); + + yield [ + $request, + false, + AccessDeniedHttpException::class, + 'Invalid signature', + ]; + + $request = new Request($query, $attributes, files: []); + $request->setMethod('POST'); + + yield [ + $request, + true, + BadRequestHttpException::class, + 'Zero files given', + ]; + + $request = new Request($query, $attributes, files: [ + 'filePrefix/abcSUFFIX_1' => new UploadedFile(__DIR__.'/data/StoredObjectContentToLocalStorageControllerTest/dummy_file', 'Some content', test: true), + 'filePrefix/abcSUFFIX_2' => new UploadedFile(__DIR__.'/data/StoredObjectContentToLocalStorageControllerTest/dummy_file', 'Some content2', test: true), + ]); + $request->setMethod('POST'); + + yield [ + $request, + true, + AccessDeniedHttpException::class, + 'More files than max file count', + ]; + + $request = new Request($query, [...$attributes, 'max_file_size' => 3], files: [ + 'filePrefix/abcSUFFIX_1' => new UploadedFile(__DIR__.'/data/StoredObjectContentToLocalStorageControllerTest/dummy_file', 'Some content', test: true), + ]); + $request->setMethod('POST'); + + yield [ + $request, + true, + AccessDeniedHttpException::class, + 'File is too big', + ]; + + $request = new Request($query, [...$attributes], files: [ + 'some/other/prefix_SUFFIX' => new UploadedFile(__DIR__.'/data/StoredObjectContentToLocalStorageControllerTest/dummy_file', 'Some content', test: true), + ]); + $request->setMethod('POST'); + + yield [ + $request, + true, + AccessDeniedHttpException::class, + 'Filename does not start with signed prefix', + ]; + } + + public static function generateOperateContentWithExceptionDataProvider(): iterable + { + yield [ + new Request(['object_name' => '', 'sig' => '', 'exp' => 0]), + BadRequestHttpException::class, + 'Object name parameter is missing', + false, + '', + false, + ]; + + yield [ + new Request(['object_name' => 'testABC', 'sig' => '', 'exp' => 0]), + BadRequestHttpException::class, + 'Expiration is not set or equal to zero', + false, + '', + false, + ]; + + yield [ + new Request(['object_name' => 'testABC', 'sig' => '', 'exp' => (new \DateTimeImmutable())->getTimestamp()]), + BadRequestHttpException::class, + 'Signature is not set or is a blank string', + false, + '', + false, + ]; + + yield [ + new Request(['object_name' => 'testABC', 'sig' => '1234', 'exp' => (new \DateTimeImmutable())->getTimestamp()]), + AccessDeniedHttpException::class, + 'Invalid signature', + false, + '', + false, + ]; + + + yield [ + new Request(['object_name' => 'testABC', 'sig' => '1234', 'exp' => (new \DateTimeImmutable())->getTimestamp()]), + NotFoundHttpException::class, + 'Object does not exists on disk', + false, + '', + true, + ]; + } +} diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Controller/data/StoredObjectContentToLocalStorageControllerTest/dummy_file b/src/Bundle/ChillDocStoreBundle/Tests/Controller/data/StoredObjectContentToLocalStorageControllerTest/dummy_file new file mode 100644 index 000000000..819563de7 --- /dev/null +++ b/src/Bundle/ChillDocStoreBundle/Tests/Controller/data/StoredObjectContentToLocalStorageControllerTest/dummy_file @@ -0,0 +1 @@ +fake_encrypted_content From 812e4047d04801263f4f9341fcc1e17a2c0f18fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 7 Jan 2025 09:25:11 +0100 Subject: [PATCH 11/14] Adjust key size in KeyGenerator to 32 bytes. Changed the key size from 128 bytes to 32 bytes in the KeyGenerator service. This aligns with the expected algorithm requirements and ensures proper cryptographic behavior. --- .../ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php b/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php index 58520b352..9956cbfa7 100644 --- a/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php +++ b/src/Bundle/ChillDocStoreBundle/Service/Cryptography/KeyGenerator.php @@ -33,7 +33,7 @@ class KeyGenerator throw new \LogicException(sprintf("Algorithm '%s' is not supported.", $algo)); } - $key = $this->randomizer->getBytes(128); + $key = $this->randomizer->getBytes(32); return [ 'alg' => 'A256CBC', From 73bcfb82b7d4b6c37236c0926e9b57072cd465a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Tue, 7 Jan 2025 10:36:10 +0100 Subject: [PATCH 12/14] Add configuration option to select storage driver Introduces a new `use_driver` configuration option to specify the desired storage driver (`local_storage` or `openstack`). Ensures proper validation to handle multiple drivers and throws appropriate errors when configurations are inconsistent or missing. Refactors related logic to improve clarity and maintainability. --- .../StorageConfigurationCompilerPass.php | 28 +++++++++++++++++-- .../DependencyInjection/Configuration.php | 4 +++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php index 075103c59..f135b54a4 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Compiler/StorageConfigurationCompilerPass.php @@ -41,11 +41,22 @@ class StorageConfigurationCompilerPass implements CompilerPassInterface ->getParameterBag() ->resolveValue($container->getParameter('chill_doc_store')); - if (array_key_exists('openstack', $config) && array_key_exists('local_storage', $config)) { - throw new InvalidConfigurationException('chill_doc_store: you can either configure a local storage (key local_storage) or openstack storage (key openstack). Choose between them.'); + if (array_key_exists('local_storage', $config) && !array_key_exists('openstack', $config)) { + $driver = 'local_storage'; + $this->checkUseDriverConfiguration($config['use_driver'] ?? null, $driver); + } elseif (!array_key_exists('local_storage', $config) && array_key_exists('openstack', $config)) { + $driver = 'openstack'; + $this->checkUseDriverConfiguration($config['use_driver'] ?? null, $driver); + } elseif (array_key_exists('openstack', $config) && array_key_exists('local_storage', $config)) { + $driver = $config['use_driver'] ?? null; + if (null === $driver) { + throw new InvalidConfigurationException('There are multiple drivers configured for chill_doc_store, set the one you want to use with the variable use_driver'); + } + } else { + throw new InvalidConfigurationException('No driver defined for storing document. Define one in chill_doc_store configuration'); } - if (array_key_exists('local_storage', $config)) { + if ('local_storage' === $driver) { foreach (self::SERVICES_OPENSTACK as $service) { $container->removeDefinition($service); } @@ -61,4 +72,15 @@ class StorageConfigurationCompilerPass implements CompilerPassInterface $container->setAlias(TempUrlGeneratorInterface::class, TempUrlOpenstackGenerator::class); } } + + private function checkUseDriverConfiguration(?string $useDriver, string $driver): void + { + if (null === $useDriver) { + return; + } + + if ($useDriver !== $driver) { + throw new InvalidConfigurationException(sprintf('The "use_driver" configuration require a driver (%s) which is not configured. Configure this driver in order to use it.', $useDriver)); + } + } } diff --git a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php index 4d17c7ebd..2e046a810 100644 --- a/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php +++ b/src/Bundle/ChillDocStoreBundle/DependencyInjection/Configuration.php @@ -30,6 +30,10 @@ class Configuration implements ConfigurationInterface /* @phpstan-ignore-next-line As there are inconsistencies in return types, but the code works... */ $rootNode->children() + ->enumNode('use_driver') + ->values(['local_storage', 'openstack']) + ->info('Driver to use. Default to the single one if multiple driver are defined. Configuration will raise an error if there are multiple drivers defined, and if this key is not set') + ->end() ->arrayNode('local_storage') ->info('where the stored object should be stored') ->children() From 0787e61c223b5b85876ffeaf01e7b7fb7017d8aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 9 Jan 2025 12:06:31 +0100 Subject: [PATCH 13/14] Set default configuration file for chill_doc_store --- config/packages/chill_doc_store.yaml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/config/packages/chill_doc_store.yaml b/config/packages/chill_doc_store.yaml index e6cfd10ec..eb7f6be01 100644 --- a/config/packages/chill_doc_store.yaml +++ b/config/packages/chill_doc_store.yaml @@ -1,8 +1,9 @@ chill_doc_store: + use_driver: openstack local_storage: storage_path: '%kernel.project_dir%/var/storage' -# openstack: -# temp_url: -# temp_url_key: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_KEY)%' # Required -# container: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_CONTAINER)%' # Required -# temp_url_base_path: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_BASE_PATH)%' # Required + openstack: + temp_url: + temp_url_key: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_KEY)%' # Required + container: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_CONTAINER)%' # Required + temp_url_base_path: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_BASE_PATH)%' # Required From 6c97654e5e7a62a49ce5a6b9c7da9d86e09231ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 9 Jan 2025 16:05:58 +0100 Subject: [PATCH 14/14] Add documentation for document storage configuration Introduce a new guide detailing document storage options, including on-disk storage and cloud-based OpenStack integration. This document explains configuration steps, benefits, and limitations for both methods, and is now linked in the production installation index. --- docs/source/installation/document-storage.rst | 84 +++++++++++++++++++ .../installation/installation-production.rst | 1 + 2 files changed, 85 insertions(+) create mode 100644 docs/source/installation/document-storage.rst diff --git a/docs/source/installation/document-storage.rst b/docs/source/installation/document-storage.rst new file mode 100644 index 000000000..0b87d9eb2 --- /dev/null +++ b/docs/source/installation/document-storage.rst @@ -0,0 +1,84 @@ +Document storage +################ + +You can store document on two different ways: + +- on disk +- in the cloud, using object storage: currently only `openstack swift `_ is supported. + +Comparison +========== + +Storing documents within the cloud is particularily suitable for "portable" deployments, like in kubernetes, or within container +without having to manage volumes to store documents. But you'll have to subscribe on a commercial offer. + +Storing documents on disk is more easy to configure, but more difficult to manage: if you use container, you will have to +manager volumes to attach documents on disk. You'll have to do some backup of the directory. If chill is load-balanced (and +multiple instances of chill are run), you will have to find a way to share the directories in read-write mode for every instance. + +On Disk +======= + +Configure Chill like this: + +.. code-block:: yaml + + # file config/packages/chill_doc_store.yaml + chill_doc_store: + use_driver: local_storage + local_storage: + storage_path: '%kernel.project_dir%/var/storage' + +In this configuration, documents will be stored in :code:`var/storage` within your app directory. But this path can be +elsewhere on the disk. Be aware that the directory must be writable by the user executing the chill app (php-fpm or www-data). + +Documents will be stored in subpathes within that directory. The files will be encrypted, the key is stored in the database. + +In the cloud, using openstack object store +########################################## + +You must subscribe to a commercial offer for object store. + +Chill use some features to allow documents to be stored in the cloud without being uploaded first to the chill server: + +- `Form POST Middelware `_; +- `Temporary URL Middelware `_. + +A secret key must be generated and configured, and CORS must be configured depending on the domain you will use to serve Chill. + +At first, create a container and get the base path to the container. For instance, on OVH, if you create a container named "mychill", +you will be able to retrieve the base path of the container within the OVH interface, like this: + +- base_path: :code:`https://storage.gra.cloud.ovh.net/v1/AUTH_123456789/mychill/` => will be variable :code:`ASYNC_UPLOAD_TEMP_URL_BASE_PATH` +- container: :code:`mychill` => will be variable :code:`ASYNC_UPLOAD_TEMP_URL_CONTAINER` + +You can also generate a key, which should have at least 20 characters. This key will go in the variable :code:`ASYNC_UPLOAD_TEMP_URL_KEY`. + +.. note:: + + See the `documentation of symfony `_ on how to store variables, and how to encrypt them if needed. + +Configure the storage like this: + +.. code-block:: yaml + + # file config/packages/chill_doc_store.yaml + chill_doc_store: + use_driver: openstack + openstack: + temp_url: + temp_url_key: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_KEY)%' # Required + container: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_CONTAINER)%' # Required + temp_url_base_path: '%env(resolve:ASYNC_UPLOAD_TEMP_URL_BASE_PATH)%' # Required + +Chill is able to configure the container in order to store document. Grab an Openstack Token (for instance, using :code:`openstack token issue` or +the web interface of your openstack provider), and run this command: + +.. code-block:: bash + + symfony console async-upload:configure --os_token=OPENSTACK_TOKEN -d https://mychill.mydomain.example + + # or, without symfony-cli + bin/console async-upload:configure --os_token=OPENSTACK_TOKEN -d https://mychill.mydomain.example + + diff --git a/docs/source/installation/installation-production.rst b/docs/source/installation/installation-production.rst index c02390b5a..30e8c91ce 100644 --- a/docs/source/installation/installation-production.rst +++ b/docs/source/installation/installation-production.rst @@ -323,6 +323,7 @@ Going further :maxdepth: 2 prod.rst + document-storage.rst load-addresses.rst prod-calendar-sms-sending.rst msgraph-configure.rst