From 57f15cf4f38278315c5f31d3949416c9455ba0d0 Mon Sep 17 00:00:00 2001 From: Antoine Bluchet Date: Mon, 16 Dec 2024 14:31:11 +0100 Subject: [PATCH] feat(state): strict query parameters (#6399) * feat(state): strict query parameters * fixes --- src/Metadata/ApiResource.php | 2 + src/Metadata/Delete.php | 2 + .../Extractor/XmlResourceExtractor.php | 1 + .../Extractor/YamlResourceExtractor.php | 1 + src/Metadata/Extractor/schema/resources.xsd | 1 + src/Metadata/Get.php | 2 + src/Metadata/GetCollection.php | 2 + src/Metadata/HttpOperation.php | 2 + src/Metadata/Metadata.php | 14 ++++++ src/Metadata/Operation.php | 2 + src/Metadata/Patch.php | 2 + src/Metadata/Post.php | 2 + src/Metadata/Put.php | 2 + .../Extractor/Adapter/XmlResourceAdapter.php | 1 + .../Tests/Extractor/Adapter/resources.xml | 2 +- .../Tests/Extractor/Adapter/resources.yaml | 2 + .../ResourceMetadataCompatibilityTest.php | 3 ++ .../Tests/Extractor/XmlExtractorTest.php | 4 ++ .../Tests/Extractor/YamlExtractorTest.php | 2 + .../ParameterNotSupportedException.php | 50 +++++++++++++++++++ src/State/Provider/ParameterProvider.php | 16 ++++++ .../ApiResource/StrictParameters.php | 35 +++++++++++++ .../Parameters/StrictParametersTest.php | 46 +++++++++++++++++ 23 files changed, 195 insertions(+), 1 deletion(-) create mode 100644 src/State/Exception/ParameterNotSupportedException.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/StrictParameters.php create mode 100644 tests/Functional/Parameters/StrictParametersTest.php diff --git a/src/Metadata/ApiResource.php b/src/Metadata/ApiResource.php index fbdc61648ae..154dca50e32 100644 --- a/src/Metadata/ApiResource.php +++ b/src/Metadata/ApiResource.php @@ -963,6 +963,7 @@ public function __construct( ?string $policy = null, array|string|null $middleware = null, array|Parameters|null $parameters = null, + protected ?bool $strictQueryParameterValidation = null, protected array $extraProperties = [], ) { parent::__construct( @@ -1007,6 +1008,7 @@ class: $class, rules: $rules, policy: $policy, middleware: $middleware, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties ); diff --git a/src/Metadata/Delete.php b/src/Metadata/Delete.php index 1656e928cf0..12c9ccb3337 100644 --- a/src/Metadata/Delete.php +++ b/src/Metadata/Delete.php @@ -98,6 +98,7 @@ public function __construct( mixed $rules = null, ?string $policy = null, array|string|null $middleware = null, + ?bool $strictQueryParameterValidation = null, array $extraProperties = [], ) { parent::__construct( @@ -178,6 +179,7 @@ class: $class, extraProperties: $extraProperties, collectDenormalizationErrors: $collectDenormalizationErrors, parameters: $parameters, + strictQueryParameterValidation: $strictQueryParameterValidation, stateOptions: $stateOptions, ); } diff --git a/src/Metadata/Extractor/XmlResourceExtractor.php b/src/Metadata/Extractor/XmlResourceExtractor.php index 9ec28bb382c..86c9f4fad07 100644 --- a/src/Metadata/Extractor/XmlResourceExtractor.php +++ b/src/Metadata/Extractor/XmlResourceExtractor.php @@ -94,6 +94,7 @@ private function buildExtendedBase(\SimpleXMLElement $resource): array 'paginationViaCursor' => $this->buildPaginationViaCursor($resource), 'exceptionToStatus' => $this->buildExceptionToStatus($resource), 'queryParameterValidationEnabled' => $this->phpize($resource, 'queryParameterValidationEnabled', 'bool'), + 'strictQueryParameterValidation' => $this->phpize($resource, 'strictQueryParameterValidation', 'bool'), 'stateOptions' => $this->buildStateOptions($resource), 'links' => $this->buildLinks($resource), 'headers' => $this->buildHeaders($resource), diff --git a/src/Metadata/Extractor/YamlResourceExtractor.php b/src/Metadata/Extractor/YamlResourceExtractor.php index 64a097e8904..16dcbd7ff13 100644 --- a/src/Metadata/Extractor/YamlResourceExtractor.php +++ b/src/Metadata/Extractor/YamlResourceExtractor.php @@ -338,6 +338,7 @@ private function buildOperations(array $resource, array $root): ?array 'write' => $this->phpize($operation, 'write', 'bool'), 'serialize' => $this->phpize($operation, 'serialize', 'bool'), 'queryParameterValidate' => $this->phpize($operation, 'queryParameterValidate', 'bool'), + 'strictQueryParameterValidation' => $this->phpize($operation, 'strictQueryParameterValidation', 'bool'), 'priority' => $this->phpize($operation, 'priority', 'integer'), 'name' => $this->phpize($operation, 'name', 'string'), 'class' => (string) $class, diff --git a/src/Metadata/Extractor/schema/resources.xsd b/src/Metadata/Extractor/schema/resources.xsd index 5c90713d806..28aeda24ffb 100644 --- a/src/Metadata/Extractor/schema/resources.xsd +++ b/src/Metadata/Extractor/schema/resources.xsd @@ -515,6 +515,7 @@ + diff --git a/src/Metadata/Get.php b/src/Metadata/Get.php index f636ca3d1a0..aecab2d4a0f 100644 --- a/src/Metadata/Get.php +++ b/src/Metadata/Get.php @@ -98,6 +98,7 @@ public function __construct( mixed $rules = null, ?string $policy = null, array|string|null $middleware = null, + ?bool $strictQueryParameterValidation = null, array $extraProperties = [], ) { parent::__construct( @@ -177,6 +178,7 @@ class: $class, rules: $rules, policy: $policy, middleware: $middleware, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties, ); } diff --git a/src/Metadata/GetCollection.php b/src/Metadata/GetCollection.php index a5fd3f29e6a..f71a0f844c8 100644 --- a/src/Metadata/GetCollection.php +++ b/src/Metadata/GetCollection.php @@ -98,6 +98,7 @@ public function __construct( array|string|null $rules = null, ?string $policy = null, array|string|null $middleware = null, + ?bool $strictQueryParameterValidation = null, array $extraProperties = [], private ?string $itemUriTemplate = null, ) { @@ -178,6 +179,7 @@ class: $class, rules: $rules, policy: $policy, middleware: $middleware, + strictQueryParameterValidation: $strictQueryParameterValidation, stateOptions: $stateOptions, ); } diff --git a/src/Metadata/HttpOperation.php b/src/Metadata/HttpOperation.php index eadd07482b0..7537fbd27b1 100644 --- a/src/Metadata/HttpOperation.php +++ b/src/Metadata/HttpOperation.php @@ -155,6 +155,7 @@ public function __construct( protected ?array $exceptionToStatus = null, protected ?array $links = null, protected ?array $errors = null, + protected ?bool $strictQueryParameterValidation = null, ?string $shortName = null, ?string $class = null, @@ -257,6 +258,7 @@ class: $class, policy: $policy, middleware: $middleware, queryParameterValidationEnabled: $queryParameterValidationEnabled, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties ); } diff --git a/src/Metadata/Metadata.php b/src/Metadata/Metadata.php index 75712b0ff6e..b0edc60b598 100644 --- a/src/Metadata/Metadata.php +++ b/src/Metadata/Metadata.php @@ -81,6 +81,7 @@ public function __construct( protected ?string $policy = null, protected array|string|null $middleware = null, protected ?bool $queryParameterValidationEnabled = null, + protected ?bool $strictQueryParameterValidation = null, protected array $extraProperties = [], ) { if (\is_array($parameters) && $parameters) { @@ -666,4 +667,17 @@ public function withExtraProperties(array $extraProperties = []): static return $self; } + + public function getStrictQueryParameterValidation(): ?bool + { + return $this->strictQueryParameterValidation; + } + + public function withStrictQueryParameterValidation(bool $strictQueryParameterValidation): static + { + $self = clone $this; + $self->strictQueryParameterValidation = $strictQueryParameterValidation; + + return $self; + } } diff --git a/src/Metadata/Operation.php b/src/Metadata/Operation.php index b48ff9827f2..072954d286c 100644 --- a/src/Metadata/Operation.php +++ b/src/Metadata/Operation.php @@ -811,6 +811,7 @@ public function __construct( ?string $policy = null, array|string|null $middleware = null, ?bool $queryParameterValidationEnabled = null, + protected ?bool $strictQueryParameterValidation = null, protected array $extraProperties = [], ) { parent::__construct( @@ -856,6 +857,7 @@ class: $class, policy: $policy, middleware: $middleware, queryParameterValidationEnabled: $queryParameterValidationEnabled, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties, ); } diff --git a/src/Metadata/Patch.php b/src/Metadata/Patch.php index 17788a95acc..6a8351518ca 100644 --- a/src/Metadata/Patch.php +++ b/src/Metadata/Patch.php @@ -98,6 +98,7 @@ public function __construct( mixed $rules = null, ?string $policy = null, array|string|null $middleware = null, + ?bool $strictQueryParameterValidation = null, array $extraProperties = [], ) { parent::__construct( @@ -178,6 +179,7 @@ class: $class, rules: $rules, policy: $policy, middleware: $middleware, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties ); } diff --git a/src/Metadata/Post.php b/src/Metadata/Post.php index 571a55c1056..6508da1767c 100644 --- a/src/Metadata/Post.php +++ b/src/Metadata/Post.php @@ -100,6 +100,7 @@ public function __construct( array|string|null $middleware = null, array $extraProperties = [], private ?string $itemUriTemplate = null, + ?bool $strictQueryParameterValidation = null, ) { parent::__construct( method: 'POST', @@ -179,6 +180,7 @@ class: $class, rules: $rules, policy: $policy, middleware: $middleware, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties ); } diff --git a/src/Metadata/Put.php b/src/Metadata/Put.php index a424da02fb9..5820e680a8e 100644 --- a/src/Metadata/Put.php +++ b/src/Metadata/Put.php @@ -99,6 +99,7 @@ public function __construct( ?string $policy = null, array|string|null $middleware = null, array $extraProperties = [], + ?bool $strictQueryParameterValidation = null, private ?bool $allowCreate = null, ) { parent::__construct( @@ -179,6 +180,7 @@ class: $class, rules: $rules, policy: $policy, middleware: $middleware, + strictQueryParameterValidation: $strictQueryParameterValidation, extraProperties: $extraProperties ); } diff --git a/src/Metadata/Tests/Extractor/Adapter/XmlResourceAdapter.php b/src/Metadata/Tests/Extractor/Adapter/XmlResourceAdapter.php index 03d9561fb35..23980115ed5 100644 --- a/src/Metadata/Tests/Extractor/Adapter/XmlResourceAdapter.php +++ b/src/Metadata/Tests/Extractor/Adapter/XmlResourceAdapter.php @@ -62,6 +62,7 @@ final class XmlResourceAdapter implements ResourceAdapterInterface 'securityPostValidation', 'securityPostValidationMessage', 'queryParameterValidationEnabled', + 'strictQueryParameterValidation', 'stateOptions', 'collectDenormalizationErrors', 'links', diff --git a/src/Metadata/Tests/Extractor/Adapter/resources.xml b/src/Metadata/Tests/Extractor/Adapter/resources.xml index 7f3ff308d27..627958a2d4e 100644 --- a/src/Metadata/Tests/Extractor/Adapter/resources.xml +++ b/src/Metadata/Tests/Extractor/Adapter/resources.xml @@ -1,3 +1,3 @@ -someirischemaanotheririschemaCommentapplication/vnd.openxmlformats-officedocument.spreadsheetml.sheetapplication/merge-patch+json+ldapplication/merge-patch+json+ld_foo\d+bazhttps
60120AuthorizationAccept-LanguageAcceptcomment:read_collectioncomment:writebazbazbarcomment.another_custom_filteruserIdLorem ipsum dolor sit ametDolor sit ametbarstringapplication/vnd.ms-excelapplication/merge-patch+jsonapplication/merge-patch+jsonpouet\d+barhttphttps60120AuthorizationAccept-Languagecomment:readcomment:writecomment:custombazbazbarcomment.custom_filterfoobarcustombazcustomquxcomment:read_collectioncomment:writebarcomment.another_custom_filteruserIdLorem ipsum dolor sit ametDolor sit ametbar/v1/v1Lorem ipsum dolor sit ametDolor sit amet/v1Lorem ipsum dolor sit ametDolor sit amet/v1Lorem ipsum dolor sit ametDolor sit ametLorem ipsum dolor sit ametDolor sit amet +someirischemaanotheririschemaCommentapplication/vnd.openxmlformats-officedocument.spreadsheetml.sheetapplication/merge-patch+json+ldapplication/merge-patch+json+ld_foo\d+bazhttps
60120AuthorizationAccept-LanguageAcceptcomment:read_collectioncomment:writebazbazbarcomment.another_custom_filteruserIdLorem ipsum dolor sit ametDolor sit ametbarstringapplication/vnd.ms-excelapplication/merge-patch+jsonapplication/merge-patch+jsonpouet\d+barhttphttps60120AuthorizationAccept-Languagecomment:readcomment:writecomment:custombazbazbarcomment.custom_filterfoobarcustombazcustomquxcomment:read_collectioncomment:writebarcomment.another_custom_filteruserIdLorem ipsum dolor sit ametDolor sit ametbar/v1/v1Lorem ipsum dolor sit ametDolor sit amet/v1Lorem ipsum dolor sit ametDolor sit amet/v1Lorem ipsum dolor sit ametDolor sit ametLorem ipsum dolor sit ametDolor sit amet diff --git a/src/Metadata/Tests/Extractor/Adapter/resources.yaml b/src/Metadata/Tests/Extractor/Adapter/resources.yaml index 3316a0798cc..6e9f10d9404 100644 --- a/src/Metadata/Tests/Extractor/Adapter/resources.yaml +++ b/src/Metadata/Tests/Extractor/Adapter/resources.yaml @@ -102,6 +102,7 @@ resources: exceptionToStatus: Symfony\Component\Serializer\Exception\ExceptionInterface: 404 queryParameterValidationEnabled: false + strictQueryParameterValidation: false read: true deserialize: false validate: false @@ -339,6 +340,7 @@ resources: policy: null middleware: null parameters: null + strictQueryParameterValidation: false extraProperties: custom_property: 'Lorem ipsum dolor sit amet' another_custom_property: diff --git a/src/Metadata/Tests/Extractor/ResourceMetadataCompatibilityTest.php b/src/Metadata/Tests/Extractor/ResourceMetadataCompatibilityTest.php index d8dfc53f0f9..20d476b83e4 100644 --- a/src/Metadata/Tests/Extractor/ResourceMetadataCompatibilityTest.php +++ b/src/Metadata/Tests/Extractor/ResourceMetadataCompatibilityTest.php @@ -96,6 +96,7 @@ final class ResourceMetadataCompatibilityTest extends TestCase 'securityPostValidation' => 'is_granted(\'ROLE_OWNER\')', 'securityPostValidationMessage' => 'Sorry, you must the owner of this resource to access it.', 'queryParameterValidationEnabled' => true, + 'strictQueryParameterValidation' => false, 'types' => ['someirischema', 'anotheririschema'], 'formats' => [ 'json' => null, @@ -399,6 +400,7 @@ final class ResourceMetadataCompatibilityTest extends TestCase 'Symfony\Component\Serializer\Exception\ExceptionInterface' => 404, ], 'queryParameterValidationEnabled' => false, + 'strictQueryParameterValidation' => false, 'read' => true, 'deserialize' => false, 'validate' => false, @@ -486,6 +488,7 @@ final class ResourceMetadataCompatibilityTest extends TestCase 'condition', 'controller', 'queryParameterValidationEnabled', + 'strictQueryParameterValidation', 'exceptionToStatus', 'types', 'formats', diff --git a/src/Metadata/Tests/Extractor/XmlExtractorTest.php b/src/Metadata/Tests/Extractor/XmlExtractorTest.php index 3f050a2963b..8f7890c0336 100644 --- a/src/Metadata/Tests/Extractor/XmlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/XmlExtractorTest.php @@ -66,6 +66,7 @@ public function testValidXML(): void 'securityPostValidation' => null, 'securityPostValidationMessage' => null, 'queryParameterValidationEnabled' => null, + 'strictQueryParameterValidation' => null, 'input' => null, 'output' => null, 'types' => null, @@ -136,6 +137,7 @@ public function testValidXML(): void 'securityPostValidation' => null, 'securityPostValidationMessage' => null, 'queryParameterValidationEnabled' => null, + 'strictQueryParameterValidation' => null, 'input' => null, 'output' => null, 'types' => ['someirischema', 'anotheririschema'], @@ -264,6 +266,7 @@ public function testValidXML(): void 'write' => null, 'serialize' => null, 'queryParameterValidate' => null, + 'strictQueryParameterValidation' => null, 'collection' => null, 'method' => null, 'priority' => null, @@ -367,6 +370,7 @@ public function testValidXML(): void 'write' => null, 'serialize' => null, 'queryParameterValidate' => null, + 'strictQueryParameterValidation' => null, 'collection' => null, 'method' => null, 'priority' => null, diff --git a/src/Metadata/Tests/Extractor/YamlExtractorTest.php b/src/Metadata/Tests/Extractor/YamlExtractorTest.php index 477d0bb5375..d4690da9fd0 100644 --- a/src/Metadata/Tests/Extractor/YamlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/YamlExtractorTest.php @@ -274,6 +274,7 @@ public function testValidYaml(): void 'securityPostValidation' => null, 'securityPostValidationMessage' => null, 'queryParameterValidationEnabled' => null, + 'strictQueryParameterValidation' => null, 'input' => null, 'output' => null, 'types' => ['someirischema'], @@ -353,6 +354,7 @@ public function testValidYaml(): void 'securityPostValidation' => null, 'securityPostValidationMessage' => null, 'queryParameterValidationEnabled' => null, + 'strictQueryParameterValidation' => null, 'input' => null, 'output' => null, 'types' => ['anotheririschema'], diff --git a/src/State/Exception/ParameterNotSupportedException.php b/src/State/Exception/ParameterNotSupportedException.php new file mode 100644 index 00000000000..1f0dcd5d58f --- /dev/null +++ b/src/State/Exception/ParameterNotSupportedException.php @@ -0,0 +1,50 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\State\Exception; + +use ApiPlatform\Metadata\Exception\ProblemExceptionInterface; +use ApiPlatform\Metadata\Exception\RuntimeException; + +final class ParameterNotSupportedException extends RuntimeException implements ProblemExceptionInterface +{ + public function __construct(private readonly string $parameter, string $message = 'Parameter not supported', int $code = 0, ?\Throwable $previous = null) + { + parent::__construct($message, $code, $previous); + } + + public function getType(): string + { + return '/error/400'; + } + + public function getTitle(): ?string + { + return $this->message; + } + + public function getStatus(): ?int + { + return 400; + } + + public function getDetail(): ?string + { + return \sprintf('Parameter "%s" not supported', $this->parameter); + } + + public function getInstance(): ?string + { + return $this->parameter; + } +} diff --git a/src/State/Provider/ParameterProvider.php b/src/State/Provider/ParameterProvider.php index 548d364b7e7..35521f50dcc 100644 --- a/src/State/Provider/ParameterProvider.php +++ b/src/State/Provider/ParameterProvider.php @@ -13,7 +13,9 @@ namespace ApiPlatform\State\Provider; +use ApiPlatform\Metadata\HttpOperation; use ApiPlatform\Metadata\Operation; +use ApiPlatform\State\Exception\ParameterNotSupportedException; use ApiPlatform\State\Exception\ProviderNotFoundException; use ApiPlatform\State\ParameterNotFound; use ApiPlatform\State\ParameterProviderInterface; @@ -50,6 +52,20 @@ public function provide(Operation $operation, array $uriVariables = [], array $c } $parameters = $operation->getParameters(); + + if ($operation instanceof HttpOperation && true === $operation->getStrictQueryParameterValidation()) { + $keys = []; + foreach ($parameters as $parameter) { + $keys[] = $parameter->getKey(); + } + + foreach (array_keys($request->attributes->get('_api_query_parameters')) as $key) { + if (!\in_array($key, $keys, true)) { + throw new ParameterNotSupportedException($key); + } + } + } + foreach ($parameters ?? [] as $parameter) { $extraProperties = $parameter->getExtraProperties(); unset($extraProperties['_api_values']); diff --git a/tests/Fixtures/TestBundle/ApiResource/StrictParameters.php b/tests/Fixtures/TestBundle/ApiResource/StrictParameters.php new file mode 100644 index 00000000000..c51a079ea00 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/StrictParameters.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource; + +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\QueryParameter; + +#[Get( + uriTemplate: 'strict_query_parameters', + strictQueryParameterValidation: true, + parameters: [ + 'foo' => new QueryParameter(), + ], + provider: [self::class, 'provider'] +)] +class StrictParameters +{ + public string $id; + + public static function provider() + { + return new self(); + } +} diff --git a/tests/Functional/Parameters/StrictParametersTest.php b/tests/Functional/Parameters/StrictParametersTest.php new file mode 100644 index 00000000000..fc038ab6735 --- /dev/null +++ b/tests/Functional/Parameters/StrictParametersTest.php @@ -0,0 +1,46 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Functional\Parameters; + +use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\StrictParameters; +use ApiPlatform\Tests\SetupClassResourcesTrait; + +final class StrictParametersTest extends ApiTestCase +{ + use SetupClassResourcesTrait; + + /** + * @return class-string[] + */ + public static function getResources(): array + { + return [StrictParameters::class]; + } + + public function testErrorAsParameterIsNotAllowed(): void + { + self::createClient()->request('GET', 'strict_query_parameters?bar=test'); + $this->assertJsonContains(['detail' => 'Parameter not supported']); + $this->assertResponseStatusCodeSame(400); + } + + public function testCorrectParameters(): void + { + self::createClient()->request('GET', 'strict_query_parameters'); + $this->assertResponseStatusCodeSame(200); + self::createClient()->request('GET', 'strict_query_parameters?foo=test'); + $this->assertResponseStatusCodeSame(200); + } +}