diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 094894df..4b309518 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -449,6 +449,7 @@ jobs: composer config extra.symfony.require ${{ matrix.symfony }} composer config minimum-stability dev composer config prefer-stable false + composer require --dev --no-update --ansi 'phpunit/phpunit:^13.3@stable' 'brianium/paratest:^7.26@stable' composer update --no-progress --ansi - name: Flag held back Symfony packages env: diff --git a/CHANGELOG.md b/CHANGELOG.md index d0d36618..51644832 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,15 @@ Every change that reaches `main` adds a line under **Unreleased**. When a versio ## Unreleased +### Breaking +- An uploadable resource's item upload (`POST {iri}/upload`) takes its `security`, `securityPostDenormalize` and `securityPostValidation` (with their messages) from the resource's `Patch`, else its `Put`, instead of its `Get`, and a resource with neither has no item upload route; the download still follows `Get` ([#584](https://github.com/components-web-app/api-components-bundle/pull/584)) + ### Fixed - `GET /_/contexts/` (the JSON-LD context) no longer fails with a TypeError in the publishable response listener ([#583](https://github.com/components-web-app/api-components-bundle/pull/583)) +### Tooling +- The Symfony NEXT CI leg keeps PHPUnit and ParaTest on their stable releases, so it tests the next Symfony rather than unreleased test tools ([#584](https://github.com/components-web-app/api-components-bundle/pull/584)) + ## [2.0.0-alpha.40](https://github.com/components-web-app/api-components-bundle/compare/2.0.0-alpha.39...2.0.0-alpha.40) - 2026-10-31 ### Breaking diff --git a/CLAUDE.md b/CLAUDE.md index 42b703ce..994d1824 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -330,6 +330,7 @@ Resources are fetched and cached **individually**. Never embed related data outs - **A variant's field reaches `FlysystemCacheResolver::store()` by its source path.** `MediaObjectFactory` and `UploadableFileManager::storeFilesMetadata()` call `UploadableCacheControl::registerVariantSource($path, $field)` before generating, and the resolver reads `forVariantOf($path)`, which falls back to the bundle value (as for liip's own filter route). Anything new that generates variants must register first. The map is cleared on `kernel.reset`. `ImagineCompilerPass` injects `$cacheControl` into every tagged `FlysystemCacheResolver` unless the application set it. - **A copy passes no `Cache-Control`; the stores keep the source's.** GCS copies metadata when the request sets none, the AWS SDK adapter copies with `MetadataDirective: COPY`, and AsyncAws's `copy()` ignores its config entirely. A clone of a file stored before #414 therefore has none until backfilled. - **`#[UploadedImage]` (#455) is `Assert\Image` for an upload field**, a subclass, so every `Image` option still applies and the orphaned file report's `StoredFileConstraintValidatorFactory` (routing on `instanceof File`) reaches it. Its validator extends `ImageValidator`. Both validators resolve it through `UploadedImage::forFile()`, the one place that decides it: an SVG has every dimension, pixel, ratio, shape and `detectCorrupted` option dropped (only the file checks run), a raster image takes `maxPixels` from `uploadable.image_max_pixels` when the constraint sets none, and `{{ max_width }}`/`{{ max_height }}` (a 3:2 approximation) are written into the message text before validating. `allowSvg: false` refuses an SVG before any other check. The configured limit reaches the upload validator service and `StoredFileConstraintChecker`'s `$defaultMaxPixels`; the scan builds its own validators, so anything else that resolves the constraint must be given it too. There is no `allowNull: false`: `file` is null on every write that does not upload, and `requiredOnPublish` is how a file is required. +- **An item upload (`POST {iri}/upload`) is a write, so it carries the resource's `Patch` security, else its `Put`'s** (#581): `UploadableResourceMetadataCollectionFactory` builds it from the `Get` (provider, contexts, controller) and copies `security`, `securityPostDenormalize`, `securityPostValidation` and their messages from the write operation, all six, including nulls. A resource with neither gets no item upload route rather than one guarded by a read's security; the collection upload copies the `Post`, and the download (a read) keeps the `Get`'s. #569's content write security is applied on top for CWA content, because its factory decorates further out. `Uploads\UploadWriteSecurityTest` pins it on a resource that is not CWA content. - Each `UploadableField` needs a **distinct `property:`**; `UploadableAttributeReader` throws when two share one (#199). Imagine runs only on raster images. - `requiredOnPublish` adds a class-level `RequiresUploadedFile` in the groups publishing validates: `{ShortName}:published`, or the class's custom `#[Publishable(validationGroups:)]`, which replace it (#193, #252). **Those groups are decided only by `PublishableAttributeReader::getPublishedValidationGroups()`**, which `PublishableValidator` and the orphaned files scan both call. - `urlGenerator: 'public'` asks the Filesystem (honouring the tag's `public_url` config) and falls back to `api` only on `UnableToGeneratePublicUrl` (#257). diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6e64c9b9..4a020448 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3,7 +3,7 @@ parameters: - message: '#^Attribute class JetBrains\\PhpStorm\\Pure does not exist\.$#' identifier: attribute.notFound - count: 5 + count: 4 path: src/ApiPlatform/Metadata/Resource/UploadableResourceMetadataCollectionFactory.php - diff --git a/src/ApiPlatform/Metadata/Resource/UploadableResourceMetadataCollectionFactory.php b/src/ApiPlatform/Metadata/Resource/UploadableResourceMetadataCollectionFactory.php index bc737ca5..f3a47910 100644 --- a/src/ApiPlatform/Metadata/Resource/UploadableResourceMetadataCollectionFactory.php +++ b/src/ApiPlatform/Metadata/Resource/UploadableResourceMetadataCollectionFactory.php @@ -15,8 +15,12 @@ use ApiPlatform\Metadata\CollectionOperationInterface; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\HttpOperation; +use ApiPlatform\Metadata\Metadata; +use ApiPlatform\Metadata\Operation; use ApiPlatform\Metadata\Operation\PathSegmentNameGeneratorInterface; +use ApiPlatform\Metadata\Patch; use ApiPlatform\Metadata\Post; +use ApiPlatform\Metadata\Put; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; use JetBrains\PhpStorm\Pure; @@ -29,6 +33,15 @@ */ class UploadableResourceMetadataCollectionFactory implements ResourceMetadataCollectionFactoryInterface { + private const array WRITE_SECURITY_PROPERTIES = [ + 'security', + 'securityMessage', + 'securityPostDenormalize', + 'securityPostDenormalizeMessage', + 'securityPostValidation', + 'securityPostValidationMessage', + ]; + private ResourceMetadataCollectionFactoryInterface $decorated; private UploadableAttributeReaderInterface $uploadableFileManager; private PathSegmentNameGeneratorInterface $pathSegmentNameGenerator; @@ -64,15 +77,18 @@ public function create(string $resourceClass): ResourceMetadataCollection if (!$operations) { continue; } + $itemWriteOperation = self::findItemWriteOperation($operations); foreach ($operations as $operation) { if ($operation instanceof Post) { $postUploadOperation = static::generatePostOperation($operation, $openApiRequestMultipartProperties, $pathSegmentName); $operations->add(self::generateOperationName($postUploadOperation), $postUploadOperation); } if ($operation instanceof Get) { - $uploadItemOperation = self::generateUploadItemOperation($operation, $openApiRequestMultipartProperties, $pathSegmentName); - $uploadName = self::generateOperationName($uploadItemOperation); - $operations->add($uploadName, $uploadItemOperation->withName($uploadName)); + if (null !== $itemWriteOperation) { + $uploadItemOperation = self::generateUploadItemOperation($operation, $itemWriteOperation, $openApiRequestMultipartProperties, $pathSegmentName); + $uploadName = self::generateOperationName($uploadItemOperation); + $operations->add($uploadName, $uploadItemOperation->withName($uploadName)); + } $downloadItemOperation = self::generateDownloadItemOperation($operation, $pathSegmentName); $downloadName = self::generateOperationName($downloadItemOperation); @@ -84,6 +100,35 @@ public function create(string $resourceClass): ResourceMetadataCollection return $resourceMetadata; } + /** + * @param iterable $operations + */ + private static function findItemWriteOperation(iterable $operations): ?HttpOperation + { + $put = null; + foreach ($operations as $operation) { + if ($operation instanceof Patch) { + return $operation; + } + if (null === $put && $operation instanceof Put) { + $put = $operation; + } + } + + return $put; + } + + private static function withWriteSecurity(HttpOperation $operation, HttpOperation $writeOperation): HttpOperation + { + $secured = clone $operation; + foreach (self::WRITE_SECURITY_PROPERTIES as $property) { + $reflectionProperty = new \ReflectionProperty(Metadata::class, $property); + $reflectionProperty->setValue($secured, $reflectionProperty->getValue($writeOperation)); + } + + return $secured; + } + #[Pure] private static function generateOperationName(HttpOperation $operation): string { @@ -116,11 +161,10 @@ private static function generatePostOperation(Post $defaultOperation, array $ope return self::configurePostOperation($newPost, $openApiRequestMultipartProperties); } - #[Pure] - private static function generateUploadItemOperation(Get $getOperation, array $openApiRequestMultipartProperties, string $pathSegmentName): HttpOperation + private static function generateUploadItemOperation(Get $getOperation, HttpOperation $writeOperation, array $openApiRequestMultipartProperties, string $pathSegmentName): HttpOperation { $path = \sprintf('/%s/{id}/upload', $pathSegmentName); - $newUploadPost = $getOperation + $newUploadPost = self::withWriteSecurity($getOperation, $writeOperation) ->withUriTemplate($path) ->withMethod(HttpOperation::METHOD_POST) ->withShortName($getOperation->getShortName()) diff --git a/tests/ApiPlatform/Metadata/Resource/StateOperationMetadataTest.php b/tests/ApiPlatform/Metadata/Resource/StateOperationMetadataTest.php index 4587ffc7..57c5c201 100644 --- a/tests/ApiPlatform/Metadata/Resource/StateOperationMetadataTest.php +++ b/tests/ApiPlatform/Metadata/Resource/StateOperationMetadataTest.php @@ -14,8 +14,10 @@ use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Get; use ApiPlatform\Metadata\HttpOperation; +use ApiPlatform\Metadata\Operation; use ApiPlatform\Metadata\Operation\PathSegmentNameGeneratorInterface; use ApiPlatform\Metadata\Operations; +use ApiPlatform\Metadata\Patch; use ApiPlatform\Metadata\Put; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; @@ -129,7 +131,7 @@ public function test_the_upload_and_download_operations_are_marked_for_their_pro $reader->method('getConfiguredProperties')->willReturn([]); $segments = $this->createStub(PathSegmentNameGeneratorInterface::class); $segments->method('getSegmentName')->willReturn('dummy_uploadables'); - $factory = new UploadableResourceMetadataCollectionFactory($this->decorated(DummyUploadable::class, 'DummyUploadable', ['kept' => 1]), $reader, $segments); + $factory = new UploadableResourceMetadataCollectionFactory($this->decorated(DummyUploadable::class, 'DummyUploadable', ['kept' => 1], [new Patch(uriTemplate: '/items/{id}', shortName: 'DummyUploadable', class: DummyUploadable::class, name: 'item_patch')]), $reader, $segments); $metadata = $factory->create(DummyUploadable::class); $upload = $metadata->getOperation('_api_/dummy_uploadables/{id}/upload_post'); @@ -143,15 +145,98 @@ public function test_the_upload_and_download_operations_are_marked_for_their_pro self::assertFalse($download->canSerialize()); } + public function test_the_upload_operation_takes_every_security_of_the_patch_operation_and_the_download_keeps_the_gets(): void + { + $metadata = $this->uploadableFactory([ + self::securedGet(), + new Put(uriTemplate: '/items/{id}', shortName: 'DummyUploadable', class: DummyUploadable::class, name: 'item_put', security: 'put', securityMessage: 'put message'), + new Patch(uriTemplate: '/items/{id}', shortName: 'DummyUploadable', class: DummyUploadable::class, name: 'item_patch', security: 'patch', securityMessage: 'patch message', securityPostDenormalize: 'patch denormalize', securityPostDenormalizeMessage: 'patch denormalize message', securityPostValidation: 'patch validation', securityPostValidationMessage: 'patch validation message'), + ])->create(DummyUploadable::class); + + self::assertSame(['patch', 'patch message', 'patch denormalize', 'patch denormalize message', 'patch validation', 'patch validation message'], self::securityOf($metadata->getOperation(self::UPLOAD))); + self::assertSame(['get', 'get message', 'get denormalize', 'get denormalize message', 'get validation', 'get validation message'], self::securityOf($metadata->getOperation(self::DOWNLOAD))); + self::assertSame('/items/{id}/upload', $metadata->getOperation(self::UPLOAD)->getUriTemplate()); + self::assertSame('POST', $metadata->getOperation(self::UPLOAD)->getMethod()); + } + + public function test_the_upload_operation_takes_the_put_operations_security_when_there_is_no_patch(): void + { + $metadata = $this->uploadableFactory([ + self::securedGet(), + new Put(uriTemplate: '/items/{id}', shortName: 'DummyUploadable', class: DummyUploadable::class, name: 'item_put', security: 'put', securityMessage: 'put message'), + new Put(uriTemplate: '/other/{id}', shortName: 'DummyUploadable', class: DummyUploadable::class, name: 'other_put', security: 'other put'), + ])->create(DummyUploadable::class); + + self::assertSame(['put', 'put message', null, null, null, null], self::securityOf($metadata->getOperation(self::UPLOAD))); + } + + public function test_a_resource_with_no_patch_or_put_gets_no_item_upload_operation_but_keeps_its_download(): void + { + $metadata = $this->uploadableFactory([self::securedGet()])->create(DummyUploadable::class); + + self::assertSame(['item_get', self::DOWNLOAD], array_keys(iterator_to_array($metadata[0]->getOperations() ?? []))); + } + + private const string UPLOAD = '_api_/items/{id}/upload_post'; + + private const string DOWNLOAD = '_api_/items/{id}/download/{property}_get'; + + private static function securedGet(): Get + { + return new Get(uriTemplate: '/items/{id}', uriVariables: ['id'], shortName: 'DummyUploadable', class: DummyUploadable::class, name: 'item_get', security: 'get', securityMessage: 'get message', securityPostDenormalize: 'get denormalize', securityPostDenormalizeMessage: 'get denormalize message', securityPostValidation: 'get validation', securityPostValidationMessage: 'get validation message'); + } + + /** + * @return list + */ + private static function securityOf(Operation $operation): array + { + return [ + $operation->getSecurity(), + $operation->getSecurityMessage(), + $operation->getSecurityPostDenormalize(), + $operation->getSecurityPostDenormalizeMessage(), + $operation->getSecurityPostValidation(), + $operation->getSecurityPostValidationMessage(), + ]; + } + + /** + * @param list $operations + */ + private function uploadableFactory(array $operations): UploadableResourceMetadataCollectionFactory + { + $reader = $this->createStub(UploadableAttributeReaderInterface::class); + $reader->method('isConfigured')->willReturn(true); + $reader->method('getConfiguredProperties')->willReturn([]); + $segments = $this->createStub(PathSegmentNameGeneratorInterface::class); + $segments->method('getSegmentName')->willReturn('items'); + $decorated = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); + $named = []; + foreach ($operations as $operation) { + $named[(string) $operation->getName()] = $operation; + } + $decorated->method('create')->willReturn(new ResourceMetadataCollection(DummyUploadable::class, [ + (new ApiResource(shortName: 'DummyUploadable', class: DummyUploadable::class))->withOperations(new Operations($named)), + ])); + + return new UploadableResourceMetadataCollectionFactory($decorated, $reader, $segments); + } + /** * @param array $extraProperties + * @param list $otherOperations */ - private function decorated(string $class, string $shortName, array $extraProperties = []): ResourceMetadataCollectionFactoryInterface + private function decorated(string $class, string $shortName, array $extraProperties = [], array $otherOperations = []): ResourceMetadataCollectionFactoryInterface { $get = new Get(uriTemplate: '/items/{id}', uriVariables: ['id'], shortName: $shortName, class: $class, name: 'item_get', controller: 'main_controller', extraProperties: $extraProperties); + $operations = ['item_get' => $get]; + foreach ($otherOperations as $operation) { + $operations[(string) $operation->getName()] = $operation; + } $decorated = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); $decorated->method('create')->willReturn(new ResourceMetadataCollection($class, [ - (new ApiResource(shortName: $shortName, class: $class))->withOperations(new Operations(['item_get' => $get])), + (new ApiResource(shortName: $shortName, class: $class))->withOperations(new Operations($operations)), ])); return $decorated; diff --git a/tests/Functional/TestBundle/Entity/DummyUploadableWithAdminWrite.php b/tests/Functional/TestBundle/Entity/DummyUploadableWithAdminWrite.php new file mode 100644 index 00000000..a40efb7c --- /dev/null +++ b/tests/Functional/TestBundle/Entity/DummyUploadableWithAdminWrite.php @@ -0,0 +1,36 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Patch; +use Doctrine\ORM\Mapping as ORM; +use Silverback\ApiComponentsBundle\Annotation as Silverback; +use Silverback\ApiComponentsBundle\Entity\Utility\IdTrait; +use Silverback\ApiComponentsBundle\Entity\Utility\UploadableTrait; +use Symfony\Component\HttpFoundation\File\File; + +#[Silverback\Uploadable] +#[ApiResource(operations: [ + new Get(), + new Patch(security: "is_granted('ROLE_ADMIN')", securityMessage: 'Only an admin may write this resource.'), +])] +#[ORM\Entity] +class DummyUploadableWithAdminWrite +{ + use IdTrait; + use UploadableTrait; + + #[Silverback\UploadableField(adapter: 'local')] + public ?File $file = null; +} diff --git a/tests/Functional/Uploads/UploadWriteSecurityTest.php b/tests/Functional/Uploads/UploadWriteSecurityTest.php new file mode 100644 index 00000000..67617693 --- /dev/null +++ b/tests/Functional/Uploads/UploadWriteSecurityTest.php @@ -0,0 +1,81 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\Functional\Uploads; + +use Silverback\ApiComponentsBundle\Tests\Functional\FunctionalTestCase; +use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyImageComponent; +use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyUploadableWithAdminWrite; +use Silverback\ApiComponentsBundle\Tests\Functional\Trait\AuthenticationTrait; + +final class UploadWriteSecurityTest extends FunctionalTestCase +{ + use AuthenticationTrait; + use UploadsAreaTrait; + + public function test_an_anonymous_upload_to_a_resource_whose_patch_is_admin_only_is_refused(): void + { + $client = static::createClient(); + $resource = self::createUploadable(new DummyUploadableWithAdminWrite()); + + $this->uploadMultipart($client, self::iriOf($resource) . '/upload', ['file' => 'image.svg']); + + self::assertResponseStatusCodeSame(401); + } + + public function test_a_users_upload_to_a_resource_whose_patch_is_admin_only_is_refused_with_the_patch_message(): void + { + $client = static::createClient(); + self::loginAsUser($client); + $resource = self::createUploadable(new DummyUploadableWithAdminWrite()); + $filename = self::findByIri(self::iriOf($resource))?->getFilename(); + + $this->uploadMultipart($client, self::iriOf($resource) . '/upload', ['file' => 'image.svg']); + + self::assertResponseStatusCodeSame(403); + self::assertJsonNodeSame('detail', 'Only an admin may write this resource.'); + self::assertSame($filename, self::findByIri(self::iriOf($resource))?->getFilename()); + } + + public function test_an_admins_upload_to_a_resource_whose_patch_is_admin_only_is_stored(): void + { + $client = static::createClient(); + self::loginAsAdmin($client); + $resource = self::createUploadable(new DummyUploadableWithAdminWrite()); + + $this->uploadMultipart($client, self::iriOf($resource) . '/upload', ['file' => 'image.svg']); + + self::assertResponseStatusCodeSame(201); + self::assertMatchesRegularExpression('#^image-[0-9a-f]{8}\.svg$#', (string) self::findByIri(self::iriOf($resource))?->getFilename()); + } + + public function test_a_users_upload_to_a_component_is_refused_by_the_publishable_permission(): void + { + $client = static::createClient(); + self::loginAsUser($client); + $resource = self::createUploadable(new DummyImageComponent()); + + $this->uploadMultipart($client, self::iriOf($resource) . '/upload', ['file' => 'image.svg']); + + self::assertResponseStatusCodeSame(403); + } + + public function test_the_download_still_follows_the_public_get(): void + { + $client = static::createClient(); + $resource = self::createUploadable(new DummyUploadableWithAdminWrite()); + + $client->request('GET', self::iriOf($resource) . '/download/file'); + + self::assertResponseStatusCodeSame(200); + self::assertResponseHeaderSame('content-type', 'image/png'); + } +}