From 00cc3b78060183a0b69007e735357c5370c9addb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Wed, 28 Aug 2024 18:00:20 +0200 Subject: [PATCH] Refactor backend for getting signed url --- .../TempUrlOpenstackGenerator.php | 5 +- .../AsyncUpload/TempUrlGeneratorInterface.php | 3 +- .../Controller/AsyncUploadController.php | 118 +++++++----- .../Entity/StoredObject.php | 7 +- .../Resources/public/types.ts | 38 ++-- .../Controller/AsyncUploadControllerTest.php | 179 +++++++++++++++--- .../ChillDocStoreBundle/chill.api.specs.yaml | 130 ++++++++++--- 7 files changed, 345 insertions(+), 135 deletions(-) diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/TempUrlOpenstackGenerator.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/TempUrlOpenstackGenerator.php index ae74f9d0e..3ba1f0dea 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/TempUrlOpenstackGenerator.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/Driver/OpenstackObjectStore/TempUrlOpenstackGenerator.php @@ -58,6 +58,7 @@ final readonly class TempUrlOpenstackGenerator implements TempUrlGeneratorInterf ?int $expire_delay = null, ?int $submit_delay = null, int $max_file_count = 1, + ?string $object_name = null, ): SignedUrlPost { $delay = $expire_delay ?? $this->max_expire_delay; $submit_delay ??= $this->max_submit_delay; @@ -84,7 +85,9 @@ final readonly class TempUrlOpenstackGenerator implements TempUrlGeneratorInterf $expires = $this->clock->now()->add(new \DateInterval('PT'.(string) $delay.'S')); - $object_name = $this->generateObjectName(); + if (null === $object_name) { + $object_name = $this->generateObjectName(); + } $g = new SignedUrlPost( $url = $this->generateUrl($object_name), diff --git a/src/Bundle/ChillDocStoreBundle/AsyncUpload/TempUrlGeneratorInterface.php b/src/Bundle/ChillDocStoreBundle/AsyncUpload/TempUrlGeneratorInterface.php index 08ac9e062..e6589b3c7 100644 --- a/src/Bundle/ChillDocStoreBundle/AsyncUpload/TempUrlGeneratorInterface.php +++ b/src/Bundle/ChillDocStoreBundle/AsyncUpload/TempUrlGeneratorInterface.php @@ -16,7 +16,8 @@ interface TempUrlGeneratorInterface public function generatePost( ?int $expire_delay = null, ?int $submit_delay = null, - int $max_file_count = 1 + int $max_file_count = 1, + ?string $object_name = null, ): SignedUrlPost; public function generate(string $method, string $object_name, ?int $expire_delay = null): SignedUrl; diff --git a/src/Bundle/ChillDocStoreBundle/Controller/AsyncUploadController.php b/src/Bundle/ChillDocStoreBundle/Controller/AsyncUploadController.php index 75be20d34..879230cd9 100644 --- a/src/Bundle/ChillDocStoreBundle/Controller/AsyncUploadController.php +++ b/src/Bundle/ChillDocStoreBundle/Controller/AsyncUploadController.php @@ -11,9 +11,10 @@ declare(strict_types=1); namespace Chill\DocStoreBundle\Controller; -use Chill\DocStoreBundle\AsyncUpload\Exception\TempUrlGeneratorException; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; -use Chill\DocStoreBundle\Security\Authorization\AsyncUploadVoter; +use Chill\DocStoreBundle\Entity\StoredObject; +use Chill\DocStoreBundle\Entity\StoredObjectVersion; +use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; @@ -30,62 +31,77 @@ final readonly class AsyncUploadController private TempUrlGeneratorInterface $tempUrlGenerator, private SerializerInterface $serializer, private Security $security, - private LoggerInterface $logger, + private LoggerInterface $chillLogger, ) {} - #[Route(path: '/asyncupload/temp_url/generate/{method}', name: 'async_upload.generate_url')] - public function getSignedUrl(string $method, Request $request): JsonResponse + #[Route(path: '/api/1.0/doc-store/async-upload/temp_url/{uuid}/generate/post', name: 'chill_docstore_asyncupload_getsignedurlpost')] + public function getSignedUrlPost(Request $request, StoredObject $storedObject): JsonResponse { - try { - switch (strtolower($method)) { - case 'post': - $p = $this->tempUrlGenerator - ->generatePost( - $request->query->has('expires_delay') ? $request->query->getInt('expires_delay') : null, - $request->query->has('submit_delay') ? $request->query->getInt('submit_delay') : null - ) - ; - break; - case 'get': - case 'head': - $object_name = $request->query->get('object_name', null); + if (!$this->security->isGranted(StoredObjectRoleEnum::EDIT->value, $storedObject)) { + throw new AccessDeniedHttpException('not able to edit the given stored object'); + } - if (null === $object_name) { - return (new JsonResponse((object) [ - 'message' => 'the object_name is null', - ])) - ->setStatusCode(JsonResponse::HTTP_BAD_REQUEST); - } - $p = $this->tempUrlGenerator->generate( - $method, - $object_name, - $request->query->has('expires_delay') ? $request->query->getInt('expires_delay') : null - ); - break; - default: - return (new JsonResponse((object) ['message' => 'the method ' - ."{$method} is not valid"])) - ->setStatusCode(JsonResponse::HTTP_BAD_REQUEST); + // we create a dummy version, to generate a filename + $version = $storedObject->registerVersion(); + + $p = $this->tempUrlGenerator + ->generatePost( + $request->query->has('expires_delay') ? $request->query->getInt('expires_delay') : null, + $request->query->has('submit_delay') ? $request->query->getInt('submit_delay') : null, + object_name: $version->getFilename() + ); + + $this->chillLogger->notice('[Privacy Event] a request to upload a document has been generated', [ + 'doc_uuid' => $storedObject->getUuid(), + ]); + + return new JsonResponse( + $this->serializer->serialize($p, 'json', [AbstractNormalizer::GROUPS => ['read']]), + Response::HTTP_OK, + [], + true + ); + } + + #[Route(path: '/api/1.0/doc-store/async-upload/temp_url/{uuid}/generate/{method}', name: 'chill_docstore_asyncupload_getsignedurlget', requirements: ['method' => 'get|head'])] + public function getSignedUrlGet(Request $request, StoredObject $storedObject, string $method): JsonResponse + { + if (!$this->security->isGranted(StoredObjectRoleEnum::SEE->value, $storedObject)) { + throw new AccessDeniedHttpException('not able to read the given stored object'); + } + + // we really want to be sure that there are no other method than get or head: + if (!in_array($method, ['get', 'head'], true)) { + throw new AccessDeniedHttpException('Only methods get and head are allowed'); + } + + if ($request->query->has('version')) { + $filename = $request->query->get('version'); + + $storedObjectVersion = $storedObject->getVersions()->findFirst(fn(int $index, StoredObjectVersion $version): bool => $version->getFilename() === $filename); + + if (null === $storedObjectVersion) { + // we are here in the case where the version is not stored into the database + // as the version is prefixed by the stored object prefix, we just have to check that this prefix + // is the same. It means that the user had previously the permission to "SEE_AND_EDIT" this stored + // object with same prefix that we checked before + if (!str_starts_with($filename, $storedObject->getPrefix())) { + throw new AccessDeniedHttpException('not able to match the version with the same filename'); + } } - } catch (TempUrlGeneratorException $e) { - $this->logger->warning('The client requested a temp url' - .' which sparkle an error.', [ - 'message' => $e->getMessage(), - 'expire_delay' => $request->query->getInt('expire_delay', 0), - 'file_count' => $request->query->getInt('file_count', 1), - 'method' => $method, - ]); - - $p = new \stdClass(); - $p->message = $e->getMessage(); - $p->status = JsonResponse::HTTP_BAD_REQUEST; - - return new JsonResponse($p, JsonResponse::HTTP_BAD_REQUEST); + } else { + $filename = $storedObject->getCurrentVersion()->getFilename(); } - if (!$this->security->isGranted(AsyncUploadVoter::GENERATE_SIGNATURE, $p)) { - throw new AccessDeniedHttpException('not allowed to generate this signature'); - } + $p = $this->tempUrlGenerator->generate( + $method, + $filename, + $request->query->has('expires_delay') ? $request->query->getInt('expires_delay') : null + ); + + $this->chillLogger->notice('[Privacy Event] a request to see a document has been granted', [ + 'doc_uuid' => $storedObject->getUuid(), + ]); return new JsonResponse( $this->serializer->serialize($p, 'json', [AbstractNormalizer::GROUPS => ['read']]), diff --git a/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php b/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php index 87ddd5785..6ed52716d 100644 --- a/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php +++ b/src/Bundle/ChillDocStoreBundle/Entity/StoredObject.php @@ -41,6 +41,7 @@ use Symfony\Component\Serializer\Annotation as Serializer; class StoredObject implements Document, TrackCreationInterface { use TrackCreationTrait; + final public const STATUS_EMPTY = 'empty'; final public const STATUS_READY = 'ready'; final public const STATUS_PENDING = 'pending'; final public const STATUS_FAILURE = 'failure'; @@ -98,7 +99,7 @@ class StoredObject implements Document, TrackCreationInterface */ public function __construct( #[ORM\Column(type: \Doctrine\DBAL\Types\Types::TEXT, options: ['default' => 'ready'])] - private string $status = 'ready' + private string $status = 'empty' ) { $this->uuid = Uuid::uuid4(); $this->versions = new ArrayCollection(); @@ -330,6 +331,10 @@ class StoredObject implements Document, TrackCreationInterface $this->versions->add($version); + if ('empty' === $this->status) { + $this->status = self::STATUS_READY; + } + return $version; } diff --git a/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts b/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts index 1fc8b1cdd..1d21feacd 100644 --- a/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts +++ b/src/Bundle/ChillDocStoreBundle/Resources/public/types.ts @@ -1,27 +1,27 @@ import {DateTime} from "../../../ChillMainBundle/Resources/public/types"; -export type StoredObjectStatus = "ready"|"failure"|"pending"; +export type StoredObjectStatus = "empty"|"ready"|"failure"|"pending"; export interface StoredObject { - id: number, + id: number, - /** - * filename of the object in the object storage - */ - filename: string, - creationDate: DateTime, - datas: object, - iv: number[], - keyInfos: object, - title: string, - type: string, - uuid: string, - status: StoredObjectStatus, + /** + * filename of the object in the object storage + */ + filename: string, + creationDate: DateTime, + datas: object, + iv: number[], + keyInfos: object, + title: string, + type: string, + uuid: string, + status: StoredObjectStatus, _links?: { - dav_link?: { - href: string - expiration: number - }, + dav_link?: { + href: string + expiration: number + }, } } @@ -82,4 +82,4 @@ export interface Signature { zones: SignatureZone[], } -export type SignedState = 'pending' | 'signed' | 'rejected' | 'canceled' | 'error'; \ No newline at end of file +export type SignedState = 'pending' | 'signed' | 'rejected' | 'canceled' | 'error'; diff --git a/src/Bundle/ChillDocStoreBundle/Tests/Controller/AsyncUploadControllerTest.php b/src/Bundle/ChillDocStoreBundle/Tests/Controller/AsyncUploadControllerTest.php index 6b49b2919..489bf1185 100644 --- a/src/Bundle/ChillDocStoreBundle/Tests/Controller/AsyncUploadControllerTest.php +++ b/src/Bundle/ChillDocStoreBundle/Tests/Controller/AsyncUploadControllerTest.php @@ -15,7 +15,7 @@ use Chill\DocStoreBundle\AsyncUpload\SignedUrl; use Chill\DocStoreBundle\AsyncUpload\SignedUrlPost; use Chill\DocStoreBundle\AsyncUpload\TempUrlGeneratorInterface; use Chill\DocStoreBundle\Controller\AsyncUploadController; -use Chill\DocStoreBundle\Security\Authorization\AsyncUploadVoter; +use Chill\DocStoreBundle\Entity\StoredObject; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; @@ -34,46 +34,165 @@ class AsyncUploadControllerTest extends TestCase { use ProphecyTrait; - public function testGenerateWhenUserIsNotGranted(): void + public function testGetSignedUrlPost(): void { - $this->expectException(AccessDeniedHttpException::class); - $controller = $this->buildAsyncUploadController(false); + $storedObject = new StoredObject(); + $security = $this->prophesize(Security::class); + $security->isGranted('SEE_AND_EDIT', $storedObject)->willReturn(true)->shouldBeCalled(); - $controller->getSignedUrl('POST', new Request()); - } + $controller = new AsyncUploadController( + $this->buildTempUrlGenerator(), + $this->buildSerializer(), + $security->reveal(), + new NullLogger(), + ); - public function testGeneratePost(): void - { - $controller = $this->buildAsyncUploadController(true); + $actual = $controller->getSignedUrlPost(new Request(query: ['expires_delay' => 10, 'submit_delay' => 1800]), $storedObject); - $actual = $controller->getSignedUrl('POST', new Request()); $decodedActual = json_decode($actual->getContent(), true, JSON_THROW_ON_ERROR, JSON_THROW_ON_ERROR); self::assertArrayHasKey('method', $decodedActual); self::assertEquals('POST', $decodedActual['method']); } - public function testGenerateGet(): void + public function testGetSignedUrlGetSimpleScenarioHappy(): void { - $controller = $this->buildAsyncUploadController(true); + $storedObject = new StoredObject(); + $storedObject->registerVersion(); + $security = $this->prophesize(Security::class); + $security->isGranted('SEE', $storedObject)->willReturn(true)->shouldBeCalled(); + + $controller = new AsyncUploadController( + $this->buildTempUrlGenerator(), + $this->buildSerializer(), + $security->reveal(), + new NullLogger(), + ); + + $actual = $controller->getSignedUrlGet(new Request(query: ['expires_delay' => 10, 'submit_delay' => 1800]), $storedObject, 'get'); - $actual = $controller->getSignedUrl('GET', new Request(['object_name' => 'abc'])); $decodedActual = json_decode($actual->getContent(), true, JSON_THROW_ON_ERROR, JSON_THROW_ON_ERROR); self::assertArrayHasKey('method', $decodedActual); self::assertEquals('GET', $decodedActual['method']); } - private function buildAsyncUploadController( - bool $isGranted, - ): AsyncUploadController { - $tempUrlGenerator = new class () implements TempUrlGeneratorInterface { - public function generatePost(?int $expire_delay = null, ?int $submit_delay = null, int $max_file_count = 1): SignedUrlPost + public function testGetSignedUrlGetSimpleScenarioNotAuthorized(): void + { + $this->expectException(AccessDeniedHttpException::class); + + $storedObject = new StoredObject(); + $storedObject->registerVersion(); + $security = $this->prophesize(Security::class); + $security->isGranted('SEE', $storedObject)->willReturn(false)->shouldBeCalled(); + + $controller = new AsyncUploadController( + $this->buildTempUrlGenerator(), + $this->buildSerializer(), + $security->reveal(), + new NullLogger(), + ); + + $controller->getSignedUrlGet(new Request(query: ['expires_delay' => 10, 'submit_delay' => 1800]), $storedObject, 'get'); + } + + public function testGetSignedUrlGetForSpecificVersionOfTheStoredObjectStoredInDatabase(): void + { + $storedObject = new StoredObject(); + $version = $storedObject->registerVersion(); + // we add a version to be sure that the we do not get the last one + $storedObject->registerVersion(); + + $security = $this->prophesize(Security::class); + $security->isGranted('SEE', $storedObject)->willReturn(true)->shouldBeCalled(); + + $controller = new AsyncUploadController( + $this->buildTempUrlGenerator(), + $this->buildSerializer(), + $security->reveal(), + new NullLogger(), + ); + + $actual = $controller->getSignedUrlGet( + new Request(query: ['expires_delay' => 10, 'submit_delay' => 1800, 'version' => $version->getFilename()]), + $storedObject, + 'get' + ); + + $decodedActual = json_decode($actual->getContent(), true, JSON_THROW_ON_ERROR, JSON_THROW_ON_ERROR); + + self::assertArrayHasKey('method', $decodedActual); + self::assertEquals('GET', $decodedActual['method']); + self::assertArrayHasKey('object_name', $decodedActual); + self::assertEquals($version->getFilename(), $decodedActual['object_name']); + } + + public function testGetSignedUrlGetForSpecificVersionOfTheStoredObjectNotYetStoredInDatabase(): void + { + $storedObject = new StoredObject(); + $storedObject->registerVersion(); + // we generate a valid name + $version = $storedObject->getPrefix().'/some-version'; + + $security = $this->prophesize(Security::class); + $security->isGranted('SEE', $storedObject)->willReturn(true)->shouldBeCalled(); + + $controller = new AsyncUploadController( + $this->buildTempUrlGenerator(), + $this->buildSerializer(), + $security->reveal(), + new NullLogger(), + ); + + $actual = $controller->getSignedUrlGet( + new Request(query: ['expires_delay' => 10, 'submit_delay' => 1800, 'version' => $version]), + $storedObject, + 'get' + ); + + $decodedActual = json_decode($actual->getContent(), true, JSON_THROW_ON_ERROR, JSON_THROW_ON_ERROR); + + self::assertArrayHasKey('method', $decodedActual); + self::assertEquals('GET', $decodedActual['method']); + self::assertArrayHasKey('object_name', $decodedActual); + self::assertEquals($version, $decodedActual['object_name']); + } + + public function testGetSignedUrlGetForSpecificVersionNotBelongingToTheStoreObject(): void + { + $this->expectException(AccessDeniedHttpException::class); + + $storedObject = new StoredObject(); + $storedObject->registerVersion(); + // we generate a random version + $version = 'something/else'; + + $security = $this->prophesize(Security::class); + $security->isGranted('SEE', $storedObject)->willReturn(true)->shouldBeCalled(); + + $controller = new AsyncUploadController( + $this->buildTempUrlGenerator(), + $this->buildSerializer(), + $security->reveal(), + new NullLogger(), + ); + + $controller->getSignedUrlGet( + new Request(query: ['expires_delay' => 10, 'submit_delay' => 1800, 'version' => $version]), + $storedObject, + 'get' + ); + } + + public function buildTempUrlGenerator(): TempUrlGeneratorInterface + { + return new class () implements TempUrlGeneratorInterface { + public function generatePost(?int $expire_delay = null, ?int $submit_delay = null, int $max_file_count = 1, ?string $object_name = null): SignedUrlPost { return new SignedUrlPost( 'https://object.store.example', new \DateTimeImmutable('1 hour'), - 'abc', + $object_name ?? 'abc', 150, 1, 1800, @@ -86,27 +205,25 @@ class AsyncUploadControllerTest extends TestCase public function generate(string $method, string $object_name, ?int $expire_delay = null): SignedUrl { return new SignedUrl( - $method, + strtoupper($method), 'https://object.store.example', new \DateTimeImmutable('1 hour'), $object_name ); } }; + } + private function buildSerializer(): SerializerInterface + { $serializer = $this->prophesize(SerializerInterface::class); $serializer->serialize(Argument::type(SignedUrl::class), 'json', Argument::type('array')) - ->will(fn (array $args): string => json_encode(['method' => $args[0]->method], JSON_THROW_ON_ERROR, 3)); + ->will(fn (array $args): string => json_encode( + ['method' => $args[0]->method, 'object_name' => $args[0]->object_name], + JSON_THROW_ON_ERROR, + 3 + )); - $security = $this->prophesize(Security::class); - $security->isGranted(AsyncUploadVoter::GENERATE_SIGNATURE, Argument::type(SignedUrl::class)) - ->willReturn($isGranted); - - return new AsyncUploadController( - $tempUrlGenerator, - $serializer->reveal(), - $security->reveal(), - new NullLogger() - ); + return $serializer->reveal(); } } diff --git a/src/Bundle/ChillDocStoreBundle/chill.api.specs.yaml b/src/Bundle/ChillDocStoreBundle/chill.api.specs.yaml index 7864a18af..28345e463 100644 --- a/src/Bundle/ChillDocStoreBundle/chill.api.specs.yaml +++ b/src/Bundle/ChillDocStoreBundle/chill.api.specs.yaml @@ -1,39 +1,107 @@ --- openapi: "3.0.0" info: - version: "1.0.0" - title: "Chill api" - description: "Api documentation for chill. Currently, work in progress" + version: "1.0.0" + title: "Chill api" + description: "Api documentation for chill. Currently, work in progress" servers: - - url: "/api" - description: "Your current dev server" + - url: "/api" + description: "Your current dev server" components: - schemas: - StoredObject: - type: object - properties: - id: - type: integer - filename: - type: string - type: - type: string + schemas: + StoredObject: + type: object + properties: + id: + type: integer + filename: + type: string + type: + type: string paths: - /1.0/doc-store/stored-object/create: - post: - tags: - - storedobject - summary: Create a stored object - responses: - 200: - description: "OK" - content: - application/json: - schema: - $ref: "#/components/schemas/StoredObject" - 403: - description: "Unauthorized" - 422: - description: "Invalid data" + /1.0/doc-store/stored-object/create: + post: + tags: + - storedobject + summary: Create a stored object + responses: + 200: + description: "OK" + content: + application/json: + schema: + $ref: "#/components/schemas/StoredObject" + 403: + description: "Unauthorized" + 422: + description: "Invalid data" + + /1.0/doc-store/async-upload/temp_url/{uuid}/generate/post: + get: + tags: + - storedobject + summary: Get a signed route to post stored object + parameters: + - in: path + name: uuid + required: true + allowEmptyValue: false + description: The UUID of the storedObject + schema: + type: string + format: uuid + responses: + 200: + description: "OK" + content: + application/json: + schema: + type: object + 403: + description: "Unauthorized" + 404: + description: "Not found" + /1.0/doc-store/async-upload/temp_url/{uuid}/generate/{method}: + get: + tags: + - storedobject + summary: Get a signed route to get a stored object + parameters: + - in: path + name: uuid + required: true + allowEmptyValue: false + description: The UUID of the storedObjeect + schema: + type: string + format: uuid + - in: path + name: method + required: true + allowEmptyValue: false + description: the method of the signed url (get or head) + schema: + type: string + enum: [get, head] + - in: query + name: version + required: false + allowEmptyValue: false + description: the version's filename of the stored object + schema: + type: string + minLength: 2 + responses: + 200: + description: "OK" + content: + application/json: + schema: + type: object + 403: + description: "Unauthorized" + 404: + description: "Not found" +