From 49a249d6b7d6dee19a8e13cc6d945450eeef5b33 Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 10 Jun 2026 14:32:22 +0200 Subject: [PATCH] feat(metadata): resolve %param% in yaml/xml and attribute resource config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symfony container parameters (%param%) are now resolved against the container in resource configuration regardless of the source — YAML, XML or PHP attributes. Previously only the resource class key was resolved; scalar fields went through verbatim and attribute values, being raw PHP literals, reached ExpressionLanguage as "%param%" and threw 'Unexpected token "operator" of value "%"'. A DI-free ContainerParameterResolver (PSR ContainerInterface) holds the substitution logic, reusing Symfony's router regex (/%%|%([^%\s]++)%/, %% escapes a literal %, env() forbidden, arrays resolved recursively). The metadata component gains no symfony/dependency-injection dependency. Two resolution rules apply everywhere: - scalar string fields resolve %param% anywhere in the value; - ExpressionLanguage fields (security, securityPostDenormalize, securityPostValidation, condition) resolve only when the whole trimmed value is a single %param% reference (^%[^%\s]+%$). Such a value is invalid ExpressionLanguage on its own, so resolving it cannot break a working expression; partial uses and real modulo (object.value % 2) reach the expression engine untouched. YAML/XML: AbstractResourceExtractor now delegates to the shared resolver (removing the duplicated router logic) and phpize() resolves scalar strings, while expression fields opt out and route through resolveExpressionPlaceholder(). Attributes: a Symfony-bridge decorator (ContainerParameterResourceMetadataCollectionFactory) walks each resource and operation and substitutes parameters via the immutable with*() setters. Resolved fields: shortName, description, uriTemplate, routePrefix, routeName, host, controller, securityMessage, securityPostDenormalizeMessage, securityPostValidationMessage, provider, processor (scalar) and security, securityPostDenormalize, securityPostValidation, condition (whole-string expression). Purely additive: %param% in these positions previously threw or was ignored, so no working configuration changes behavior. Functional ApiTestCases cover both a YAML-config and an attribute-defined resource declaring security: '%param%' (200/403/401). Closes #8104 --- .../Extractor/AbstractResourceExtractor.php | 75 ++------- .../Extractor/ResourceExtractorTrait.php | 13 +- .../Extractor/XmlResourceExtractor.php | 8 +- .../Extractor/YamlResourceExtractor.php | 8 +- .../Tests/Extractor/XmlExtractorTest.php | 27 ++++ .../Tests/Extractor/YamlExtractorTest.php | 27 ++++ .../Tests/Extractor/xml/parameters.xml | 12 ++ .../Tests/Extractor/yaml/parameters.yaml | 7 + .../Util/ContainerParameterResolver.php | 110 +++++++++++++ .../Resources/config/metadata/resource.php | 8 + ...meterResourceMetadataCollectionFactory.php | 144 ++++++++++++++++++ ...rResourceMetadataCollectionFactoryTest.php | 102 +++++++++++++ .../SecurityFromContainerParameter.php | 44 ++++++ .../Model/SecurityFromParameter.php | 36 +++++ .../config/api_resources/resources.yaml | 9 ++ tests/Fixtures/app/AppKernel.php | 1 + ...ityFromContainerParameterAttributeTest.php | 67 ++++++++ .../SecurityFromContainerParameterTest.php | 67 ++++++++ 18 files changed, 693 insertions(+), 72 deletions(-) create mode 100644 src/Metadata/Tests/Extractor/xml/parameters.xml create mode 100644 src/Metadata/Tests/Extractor/yaml/parameters.yaml create mode 100644 src/Metadata/Util/ContainerParameterResolver.php create mode 100644 src/Symfony/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactory.php create mode 100644 src/Symfony/Tests/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactoryTest.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/SecurityFromContainerParameter.php create mode 100644 tests/Fixtures/TestBundle/Model/SecurityFromParameter.php create mode 100644 tests/Functional/Security/SecurityFromContainerParameterAttributeTest.php create mode 100644 tests/Functional/Security/SecurityFromContainerParameterTest.php diff --git a/src/Metadata/Extractor/AbstractResourceExtractor.php b/src/Metadata/Extractor/AbstractResourceExtractor.php index d9bd3eac43..6fc03a4d0c 100644 --- a/src/Metadata/Extractor/AbstractResourceExtractor.php +++ b/src/Metadata/Extractor/AbstractResourceExtractor.php @@ -13,8 +13,8 @@ namespace ApiPlatform\Metadata\Extractor; +use ApiPlatform\Metadata\Util\ContainerParameterResolver; use Psr\Container\ContainerInterface; -use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface; /** * Base file extractor. @@ -24,13 +24,14 @@ abstract class AbstractResourceExtractor implements ResourceExtractorInterface { protected ?array $resources = null; - private array $collectedParameters = []; + private readonly ContainerParameterResolver $parameterResolver; /** * @param string[] $paths */ public function __construct(protected array $paths, private readonly ?ContainerInterface $container = null) { + $this->parameterResolver = new ContainerParameterResolver($container); } /** @@ -56,68 +57,20 @@ public function getResources(): array abstract protected function extractPath(string $path): void; /** - * Recursively replaces placeholders with the service container parameters. - * - * @see https://github.com/symfony/symfony/blob/6fec32c/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php - * - * @copyright (c) Fabien Potencier - * - * @param mixed $value The source which might contain "%placeholders%" - * - * @throws \RuntimeException When a container value is not a string or a numeric value - * - * @return mixed The source with the placeholders replaced by the container - * parameters. Arrays are resolved recursively. + * Recursively replaces %param% placeholders with the service container parameters. */ protected function resolve(mixed $value): mixed { - if (null === $this->container) { - return $value; - } - - if (\is_array($value)) { - foreach ($value as $key => $val) { - $value[$key] = $this->resolve($val); - } - - return $value; - } - - if (!\is_string($value)) { - return $value; - } - - $escapedValue = preg_replace_callback('/%%|%([^%\s]++)%/', function ($match) use ($value) { - $parameter = $match[1] ?? null; - - // skip %% - if (!isset($parameter)) { - return '%%'; - } - - if (preg_match('/^env\(\w+\)$/', $parameter)) { - throw new \RuntimeException(\sprintf('Using "%%%s%%" is not allowed in routing configuration.', $parameter)); - } - - if (\array_key_exists($parameter, $this->collectedParameters)) { - return $this->collectedParameters[$parameter]; - } - - if ($this->container instanceof SymfonyContainerInterface) { - $resolved = $this->container->getParameter($parameter); - } else { - $resolved = $this->container->get($parameter); - } - - if (\is_string($resolved) || is_numeric($resolved)) { - $this->collectedParameters[$parameter] = $resolved; - - return (string) $resolved; - } - - throw new \RuntimeException(\sprintf('The container parameter "%s", used in the resource configuration value "%s", must be a string or numeric, but it is of type %s.', $parameter, $value, \gettype($resolved))); - }, $value); + return $this->parameterResolver->resolve($value); + } - return str_replace('%%', '%', $escapedValue); + /** + * Resolves a container parameter in an ExpressionLanguage field (security, condition, …) only + * when the whole trimmed value is a single %param% reference, leaving real expressions (and + * their modulo "%") untouched. + */ + protected function resolveExpressionPlaceholder(mixed $value): mixed + { + return $this->parameterResolver->resolveExpressionPlaceholder($value); } } diff --git a/src/Metadata/Extractor/ResourceExtractorTrait.php b/src/Metadata/Extractor/ResourceExtractorTrait.php index d56abf3c63..871ea17dcc 100644 --- a/src/Metadata/Extractor/ResourceExtractorTrait.php +++ b/src/Metadata/Extractor/ResourceExtractorTrait.php @@ -46,8 +46,13 @@ private function buildArrayValue(\SimpleXMLElement|array|null $resource, string /** * Transforms an attribute's value in a PHP value. + * + * Container parameters (%param%) found in plain string values are resolved against the + * service container. ExpressionLanguage fields (security, conditions, …) must opt out via + * $resolve = false so their %param% tokens reach the expression engine untouched, then route + * the raw value through resolveExpressionPlaceholder() to recover whole-string %param% refs. */ - private function phpize(\SimpleXMLElement|array|null $resource, string $key, string $type, mixed $default = null): array|bool|int|string|null + private function phpize(\SimpleXMLElement|array|null $resource, string $key, string $type, mixed $default = null, bool $resolve = true): array|bool|int|string|null { if (!isset($resource[$key])) { return $default; @@ -55,9 +60,11 @@ private function phpize(\SimpleXMLElement|array|null $resource, string $key, str switch ($type) { case 'bool|string': - return \is_bool($resource[$key]) || \in_array((string) $resource[$key], ['1', '0', 'true', 'false'], true) ? $this->phpize($resource, $key, 'bool') : $this->phpize($resource, $key, 'string'); + return \is_bool($resource[$key]) || \in_array((string) $resource[$key], ['1', '0', 'true', 'false'], true) ? $this->phpize($resource, $key, 'bool') : $this->phpize($resource, $key, 'string', resolve: $resolve); case 'string': - return (string) $resource[$key]; + $value = (string) $resource[$key]; + + return $resolve ? $this->resolve($value) : $value; case 'integer': return (int) $resource[$key]; case 'bool': diff --git a/src/Metadata/Extractor/XmlResourceExtractor.php b/src/Metadata/Extractor/XmlResourceExtractor.php index d439c1e981..8a65f44e9e 100644 --- a/src/Metadata/Extractor/XmlResourceExtractor.php +++ b/src/Metadata/Extractor/XmlResourceExtractor.php @@ -80,7 +80,7 @@ private function buildExtendedBase(\SimpleXMLElement $resource): array 'acceptPatch' => $this->phpize($resource, 'acceptPatch', 'string'), 'status' => $this->phpize($resource, 'status', 'integer'), 'host' => $this->phpize($resource, 'host', 'string'), - 'condition' => $this->phpize($resource, 'condition', 'string'), + 'condition' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'condition', 'string', resolve: false)), 'controller' => $this->phpize($resource, 'controller', 'string'), 'types' => $this->buildArrayValue($resource, 'type'), 'formats' => $this->buildFormats($resource, 'formats'), @@ -132,11 +132,11 @@ private function buildBase(\SimpleXMLElement $resource): array 'paginationType' => $this->phpize($resource, 'paginationType', 'string'), 'processor' => $this->phpize($resource, 'processor', 'string'), 'provider' => $this->phpize($resource, 'provider', 'string'), - 'security' => $this->phpize($resource, 'security', 'string'), + 'security' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'security', 'string', resolve: false)), 'securityMessage' => $this->phpize($resource, 'securityMessage', 'string'), - 'securityPostDenormalize' => $this->phpize($resource, 'securityPostDenormalize', 'string'), + 'securityPostDenormalize' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'securityPostDenormalize', 'string', resolve: false)), 'securityPostDenormalizeMessage' => $this->phpize($resource, 'securityPostDenormalizeMessage', 'string'), - 'securityPostValidation' => $this->phpize($resource, 'securityPostValidation', 'string'), + 'securityPostValidation' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'securityPostValidation', 'string', resolve: false)), 'securityPostValidationMessage' => $this->phpize($resource, 'securityPostValidationMessage', 'string'), 'normalizationContext' => isset($resource->normalizationContext->values) ? $this->buildValues($resource->normalizationContext->values) : null, 'denormalizationContext' => isset($resource->denormalizationContext->values) ? $this->buildValues($resource->denormalizationContext->values) : null, diff --git a/src/Metadata/Extractor/YamlResourceExtractor.php b/src/Metadata/Extractor/YamlResourceExtractor.php index 38ac28e057..8f47aa2147 100644 --- a/src/Metadata/Extractor/YamlResourceExtractor.php +++ b/src/Metadata/Extractor/YamlResourceExtractor.php @@ -108,7 +108,7 @@ private function buildExtendedBase(array $resource): array 'sunset' => $this->phpize($resource, 'sunset', 'string'), 'acceptPatch' => $this->phpize($resource, 'acceptPatch', 'string'), 'host' => $this->phpize($resource, 'host', 'string'), - 'condition' => $this->phpize($resource, 'condition', 'string'), + 'condition' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'condition', 'string', resolve: false)), 'controller' => $this->phpize($resource, 'controller', 'string'), 'queryParameterValidationEnabled' => $this->phpize($resource, 'queryParameterValidationEnabled', 'bool'), 'types' => $this->buildArrayValue($resource, 'types'), @@ -155,11 +155,11 @@ private function buildBase(array $resource): array 'paginationType' => $this->phpize($resource, 'paginationType', 'string'), 'processor' => $this->phpize($resource, 'processor', 'string'), 'provider' => $this->phpize($resource, 'provider', 'string'), - 'security' => $this->phpize($resource, 'security', 'string'), + 'security' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'security', 'string', resolve: false)), 'securityMessage' => $this->phpize($resource, 'securityMessage', 'string'), - 'securityPostDenormalize' => $this->phpize($resource, 'securityPostDenormalize', 'string'), + 'securityPostDenormalize' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'securityPostDenormalize', 'string', resolve: false)), 'securityPostDenormalizeMessage' => $this->phpize($resource, 'securityPostDenormalizeMessage', 'string'), - 'securityPostValidation' => $this->phpize($resource, 'securityPostValidation', 'string'), + 'securityPostValidation' => $this->resolveExpressionPlaceholder($this->phpize($resource, 'securityPostValidation', 'string', resolve: false)), 'securityPostValidationMessage' => $this->phpize($resource, 'securityPostValidationMessage', 'string'), 'input' => $this->phpize($resource, 'input', 'bool|string'), 'output' => $this->phpize($resource, 'output', 'bool|string'), diff --git a/src/Metadata/Tests/Extractor/XmlExtractorTest.php b/src/Metadata/Tests/Extractor/XmlExtractorTest.php index 7b3f496d04..b64fce5964 100644 --- a/src/Metadata/Tests/Extractor/XmlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/XmlExtractorTest.php @@ -24,6 +24,7 @@ use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\User; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; use Symfony\Component\Serializer\Exception\ExceptionInterface; /** @@ -429,6 +430,32 @@ public function testValidXML(): void ], $extractor->getResources()); } + public function testContainerParametersAreResolved(): void + { + $parameters = [ + 'user.class' => User::class, + 'user.route_prefix' => '/admin', + 'user.security' => 'is_granted("ROLE_ADMIN")', + ]; + $container = $this->createStub(ContainerInterface::class); + $container->method('get')->willReturnCallback(static fn (string $id): string => $parameters[$id]); + + $extractor = new XmlResourceExtractor([__DIR__.'/xml/parameters.xml'], $container); + $resources = $extractor->getResources(); + + $this->assertArrayHasKey(User::class, $resources); + + // scalar string field: %param% resolved anywhere in the string + $this->assertSame('/admin', $resources[User::class][0]['routePrefix']); + + // expression field, whole-string %param%: resolved to the stored expression so it reaches + // ExpressionLanguage (the literal #8104 case, which throws if left as "%user.security%") + $this->assertSame('is_granted("ROLE_ADMIN")', $resources[User::class][0]['security']); + + // expression field with a real expression (no whole-string param): left untouched + $this->assertSame('is_granted("ROLE_USER")', $resources[User::class][0]['operations'][0]['security']); + } + #[DataProvider('getInvalidPaths')] public function testInvalidXML(string $path, string $error): void { diff --git a/src/Metadata/Tests/Extractor/YamlExtractorTest.php b/src/Metadata/Tests/Extractor/YamlExtractorTest.php index de25bcf7a9..581aeba726 100644 --- a/src/Metadata/Tests/Extractor/YamlExtractorTest.php +++ b/src/Metadata/Tests/Extractor/YamlExtractorTest.php @@ -24,6 +24,7 @@ use ApiPlatform\Metadata\Tests\Fixtures\ApiResource\User; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; /** * @author Vincent Chalamon @@ -593,6 +594,32 @@ public function testExtendingYamlResourcesByMultipleYamlFiles(): void $this->assertCount(1, $resources[User::class][1]['operations']); } + public function testContainerParametersAreResolved(): void + { + $parameters = [ + 'user.class' => User::class, + 'user.route_prefix' => '/admin', + 'user.security' => 'is_granted("ROLE_ADMIN")', + ]; + $container = $this->createStub(ContainerInterface::class); + $container->method('get')->willReturnCallback(static fn (string $id): string => $parameters[$id]); + + $extractor = new YamlResourceExtractor([__DIR__.'/yaml/parameters.yaml'], $container); + $resources = $extractor->getResources(); + + $this->assertArrayHasKey(User::class, $resources); + + // scalar string field: %param% resolved anywhere in the string + $this->assertSame('/admin', $resources[User::class][0]['routePrefix']); + + // expression field, whole-string %param%: resolved to the stored expression so it reaches + // ExpressionLanguage (the literal #8104 case, which throws if left as "%user.security%") + $this->assertSame('is_granted("ROLE_ADMIN")', $resources[User::class][0]['security']); + + // expression field with a real expression (no whole-string param): left untouched + $this->assertSame('is_granted("ROLE_USER")', $resources[User::class][0]['operations'][0]['security']); + } + #[DataProvider('getInvalidPaths')] public function testInvalidYaml(string $path, string $error): void { diff --git a/src/Metadata/Tests/Extractor/xml/parameters.xml b/src/Metadata/Tests/Extractor/xml/parameters.xml new file mode 100644 index 0000000000..9a09d31fcc --- /dev/null +++ b/src/Metadata/Tests/Extractor/xml/parameters.xml @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/src/Metadata/Tests/Extractor/yaml/parameters.yaml b/src/Metadata/Tests/Extractor/yaml/parameters.yaml new file mode 100644 index 0000000000..320c4eeab8 --- /dev/null +++ b/src/Metadata/Tests/Extractor/yaml/parameters.yaml @@ -0,0 +1,7 @@ +resources: + '%user.class%': + - routePrefix: '%user.route_prefix%' + security: '%user.security%' + operations: + ApiPlatform\Metadata\GetCollection: + security: 'is_granted("ROLE_USER")' diff --git a/src/Metadata/Util/ContainerParameterResolver.php b/src/Metadata/Util/ContainerParameterResolver.php new file mode 100644 index 0000000000..e076a3f598 --- /dev/null +++ b/src/Metadata/Util/ContainerParameterResolver.php @@ -0,0 +1,110 @@ + + * + * 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\Metadata\Util; + +use Psr\Container\ContainerInterface; +use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface; + +/** + * Replaces Symfony container parameters (%param%) found in resource configuration values. + * + * The substitution logic mirrors Symfony's router (and the YAML/XML resource extractors): + * %% escapes a literal %, env() parameters are forbidden, and a parameter must resolve to a + * scalar. It is intentionally kept free of any symfony/dependency-injection requirement so the + * standalone metadata component can rely on it through a PSR ContainerInterface. + * + * @see https://github.com/symfony/symfony/blob/6fec32c/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php + * + * @copyright (c) Fabien Potencier + * @author Antoine Bluchet + */ +final class ContainerParameterResolver +{ + private array $collectedParameters = []; + + public function __construct(private readonly ?ContainerInterface $container = null) + { + } + + /** + * Resolves every %param% reference found anywhere in $value (router-style substitution). + * + * @throws \RuntimeException When a container value is not a string or a numeric value + */ + public function resolve(mixed $value): mixed + { + if (null === $this->container) { + return $value; + } + + if (\is_array($value)) { + foreach ($value as $key => $val) { + $value[$key] = $this->resolve($val); + } + + return $value; + } + + if (!\is_string($value)) { + return $value; + } + + $escapedValue = preg_replace_callback('/%%|%([^%\s]++)%/', function ($match) use ($value) { + $parameter = $match[1] ?? null; + + // skip %% + if (!isset($parameter)) { + return '%%'; + } + + if (preg_match('/^env\(\w+\)$/', $parameter)) { + throw new \RuntimeException(\sprintf('Using "%%%s%%" is not allowed in resource configuration.', $parameter)); + } + + if (\array_key_exists($parameter, $this->collectedParameters)) { + return $this->collectedParameters[$parameter]; + } + + if ($this->container instanceof SymfonyContainerInterface) { + $resolved = $this->container->getParameter($parameter); + } else { + $resolved = $this->container->get($parameter); + } + + if (\is_string($resolved) || is_numeric($resolved)) { + return $this->collectedParameters[$parameter] = (string) $resolved; + } + + throw new \RuntimeException(\sprintf('The container parameter "%s", used in the resource configuration value "%s", must be a string or numeric, but it is of type %s.', $parameter, $value, get_debug_type($resolved))); + }, $value); + + return str_replace('%%', '%', $escapedValue); + } + + /** + * Resolves a container parameter in an ExpressionLanguage field (security, condition, …) only + * when the whole trimmed value is a single %param% reference. Such a value is invalid + * ExpressionLanguage on its own, so resolving it cannot break a working expression. Any other + * use of "%" — partial, or a real modulo like "object.value % 2" — is left untouched so it + * reaches the expression engine verbatim. + */ + public function resolveExpressionPlaceholder(mixed $value): mixed + { + if (!\is_string($value) || !preg_match('/^%[^%\s]+%$/', trim($value))) { + return $value; + } + + return $this->resolve(trim($value)); + } +} diff --git a/src/Symfony/Bundle/Resources/config/metadata/resource.php b/src/Symfony/Bundle/Resources/config/metadata/resource.php index ef3481bafc..4249e08768 100644 --- a/src/Symfony/Bundle/Resources/config/metadata/resource.php +++ b/src/Symfony/Bundle/Resources/config/metadata/resource.php @@ -31,6 +31,7 @@ use ApiPlatform\Metadata\Resource\Factory\PhpFileResourceMetadataCollectionFactory; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\Resource\Factory\UriTemplateResourceMetadataCollectionFactory; +use ApiPlatform\Symfony\Metadata\Resource\Factory\ContainerParameterResourceMetadataCollectionFactory; return function (ContainerConfigurator $container) { $services = $container->services(); @@ -83,6 +84,13 @@ '%api_platform.graphql.enabled%', ]); + $services->set('api_platform.metadata.resource.metadata_collection_factory.container_parameter', ContainerParameterResourceMetadataCollectionFactory::class) + ->decorate('api_platform.metadata.resource.metadata_collection_factory', null, 900) + ->args([ + service('service_container'), + service('api_platform.metadata.resource.metadata_collection_factory.container_parameter.inner'), + ]); + $services->set('api_platform.metadata.resource.metadata_collection_factory.not_exposed_operation', NotExposedOperationResourceMetadataCollectionFactory::class) ->decorate('api_platform.metadata.resource.metadata_collection_factory', null, 700) ->args([ diff --git a/src/Symfony/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactory.php b/src/Symfony/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactory.php new file mode 100644 index 0000000000..fcfdbb9b26 --- /dev/null +++ b/src/Symfony/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactory.php @@ -0,0 +1,144 @@ + + * + * 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\Symfony\Metadata\Resource\Factory; + +use ApiPlatform\Metadata\Metadata; +use ApiPlatform\Metadata\Operations; +use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; +use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; +use ApiPlatform\Metadata\Util\ContainerParameterResolver; +use Psr\Container\ContainerInterface; + +/** + * Resolves Symfony container parameters (%param%) declared in attribute-defined resource metadata. + * + * Attribute arguments are raw PHP literals, so #[ApiResource(security: '%app.security%')] reaches + * the metadata as the literal string "%app.security%". This decorator walks the produced collection + * and substitutes those parameters, matching the YAML/XML extractors. The container is only + * available in the Symfony bridge, which is why this factory lives here and not in the + * dependency-injection-free metadata component. + * + * Two resolution rules apply, mirroring the extractors: + * - scalar string fields (uriTemplate, routePrefix, host, controller, provider, processor, + * securityMessage, …) resolve %param% anywhere in the string; + * - ExpressionLanguage fields (security, securityPostDenormalize, securityPostValidation, + * condition) resolve only when the whole trimmed value is a single %param% reference, so partial + * uses and real modulo expressions reach the expression engine untouched. + * + * @author Antoine Bluchet + */ +final class ContainerParameterResourceMetadataCollectionFactory implements ResourceMetadataCollectionFactoryInterface +{ + /** + * Scalar string fields resolved anywhere in the value. + * + * @var array getter => setter + */ + private const SCALAR_FIELDS = [ + 'getShortName' => 'withShortName', + 'getDescription' => 'withDescription', + 'getUriTemplate' => 'withUriTemplate', + 'getRoutePrefix' => 'withRoutePrefix', + 'getRouteName' => 'withRouteName', + 'getHost' => 'withHost', + 'getController' => 'withController', + 'getSecurityMessage' => 'withSecurityMessage', + 'getSecurityPostDenormalizeMessage' => 'withSecurityPostDenormalizeMessage', + 'getSecurityPostValidationMessage' => 'withSecurityPostValidationMessage', + 'getProvider' => 'withProvider', + 'getProcessor' => 'withProcessor', + ]; + + /** + * ExpressionLanguage fields resolved only when the whole value is a single %param% reference. + * + * @var array getter => setter + */ + private const EXPRESSION_FIELDS = [ + 'getSecurity' => 'withSecurity', + 'getSecurityPostDenormalize' => 'withSecurityPostDenormalize', + 'getSecurityPostValidation' => 'withSecurityPostValidation', + 'getCondition' => 'withCondition', + ]; + + private readonly ContainerParameterResolver $resolver; + + public function __construct( + ContainerInterface $container, + private readonly ResourceMetadataCollectionFactoryInterface $decorated, + ) { + $this->resolver = new ContainerParameterResolver($container); + } + + public function create(string $resourceClass): ResourceMetadataCollection + { + $resourceMetadataCollection = $this->decorated->create($resourceClass); + + foreach ($resourceMetadataCollection as $i => $resource) { + $operations = $resource->getOperations(); + if (null !== $operations) { + $newOperations = new Operations(); + foreach ($operations as $operationName => $operation) { + $newOperations->add($operationName, $this->resolveMetadata($operation)); + } + + $resource = $resource->withOperations($newOperations); + } + + $resourceMetadataCollection[$i] = $this->resolveMetadata($resource); + } + + return $resourceMetadataCollection; + } + + /** + * @template T of Metadata + * + * @param T $metadata + * + * @return T + */ + private function resolveMetadata(Metadata $metadata): Metadata + { + foreach (self::SCALAR_FIELDS as $getter => $setter) { + if (!method_exists($metadata, $getter)) { + continue; + } + + $value = $metadata->{$getter}(); + if (!\is_string($value)) { + continue; + } + + $resolved = $this->resolver->resolve($value); + if ($resolved !== $value) { + $metadata = $metadata->{$setter}($resolved); + } + } + + foreach (self::EXPRESSION_FIELDS as $getter => $setter) { + if (!method_exists($metadata, $getter)) { + continue; + } + + $value = $metadata->{$getter}(); + $resolved = $this->resolver->resolveExpressionPlaceholder($value); + if ($resolved !== $value) { + $metadata = $metadata->{$setter}($resolved); + } + } + + return $metadata; + } +} diff --git a/src/Symfony/Tests/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactoryTest.php b/src/Symfony/Tests/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactoryTest.php new file mode 100644 index 0000000000..01a91ab0f7 --- /dev/null +++ b/src/Symfony/Tests/Metadata/Resource/Factory/ContainerParameterResourceMetadataCollectionFactoryTest.php @@ -0,0 +1,102 @@ + + * + * 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\Symfony\Tests\Metadata\Resource\Factory; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Operations; +use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; +use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; +use ApiPlatform\Symfony\Metadata\Resource\Factory\ContainerParameterResourceMetadataCollectionFactory; +use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; + +final class ContainerParameterResourceMetadataCollectionFactoryTest extends TestCase +{ + private function createContainer(array $parameters): ContainerInterface + { + $container = $this->createStub(ContainerInterface::class); + $container->method('has')->willReturnCallback(static fn (string $id): bool => \array_key_exists($id, $parameters)); + $container->method('get')->willReturnCallback(static fn (string $id): mixed => $parameters[$id] ?? null); + + return $container; + } + + private function createFactory(ResourceMetadataCollection $collection, array $parameters): ContainerParameterResourceMetadataCollectionFactory + { + $decorated = $this->createStub(ResourceMetadataCollectionFactoryInterface::class); + $decorated->method('create')->willReturn($collection); + + return new ContainerParameterResourceMetadataCollectionFactory($this->createContainer($parameters), $decorated); + } + + public function testItResolvesWholeStringSecurityExpressionParameter(): void + { + $collection = new ResourceMetadataCollection('Foo', [ + (new ApiResource())->withSecurity('%app.security%'), + ]); + + $factory = $this->createFactory($collection, ['app.security' => 'is_granted("ROLE_ADMIN")']); + + $this->assertSame('is_granted("ROLE_ADMIN")', $factory->create('Foo')[0]->getSecurity()); + } + + public function testItLeavesPlainExpressionUntouched(): void + { + $collection = new ResourceMetadataCollection('Foo', [ + (new ApiResource())->withSecurity('is_granted("ROLE_USER")'), + ]); + + $factory = $this->createFactory($collection, []); + + $this->assertSame('is_granted("ROLE_USER")', $factory->create('Foo')[0]->getSecurity()); + } + + public function testItLeavesModuloExpressionUntouched(): void + { + $collection = new ResourceMetadataCollection('Foo', [ + (new ApiResource())->withCondition('object.value % 2 === 0'), + ]); + + $factory = $this->createFactory($collection, []); + + $this->assertSame('object.value % 2 === 0', $factory->create('Foo')[0]->getCondition()); + } + + public function testItResolvesScalarFieldAnywhereInTheString(): void + { + $collection = new ResourceMetadataCollection('Foo', [ + (new ApiResource())->withOperations(new Operations([ + 'get' => (new Get())->withRoutePrefix('/%app.prefix%/v1'), + ])), + ]); + + $factory = $this->createFactory($collection, ['app.prefix' => 'api']); + + $this->assertSame('/api/v1', $factory->create('Foo')->getOperation('get')->getRoutePrefix()); + } + + public function testItResolvesOperationLevelSecurityExpressionParameter(): void + { + $collection = new ResourceMetadataCollection('Foo', [ + (new ApiResource())->withOperations(new Operations([ + 'get' => (new Get())->withSecurity('%app.security%'), + ])), + ]); + + $factory = $this->createFactory($collection, ['app.security' => 'is_granted("ROLE_ADMIN")']); + + $this->assertSame('is_granted("ROLE_ADMIN")', $factory->create('Foo')->getOperation('get')->getSecurity()); + } +} diff --git a/tests/Fixtures/TestBundle/ApiResource/SecurityFromContainerParameter.php b/tests/Fixtures/TestBundle/ApiResource/SecurityFromContainerParameter.php new file mode 100644 index 0000000000..6dab641e7a --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/SecurityFromContainerParameter.php @@ -0,0 +1,44 @@ + + * + * 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\Operation; + +/** + * Attribute-defined resource whose `security` expression comes from a container parameter + * (`%app.security.admin_only%` holds `is_granted("ROLE_ADMIN")`). Verifies the Symfony bridge + * resolves whole-string %param% references declared through PHP attributes, mirroring the + * YAML/XML extractor support (issue #8104). + */ +#[Get( + uriTemplate: '/security_from_container_parameter/{id}', + uriVariables: ['id'], + security: '%app.security.admin_only%', + provider: [SecurityFromContainerParameter::class, 'provide'], +)] +class SecurityFromContainerParameter +{ + private ?int $id = null; + + public function getId(): ?int + { + return $this->id; + } + + public static function provide(Operation $operation, array $uriVariables = [], array $context = []): self + { + return new self(); + } +} diff --git a/tests/Fixtures/TestBundle/Model/SecurityFromParameter.php b/tests/Fixtures/TestBundle/Model/SecurityFromParameter.php new file mode 100644 index 0000000000..66e1cd9392 --- /dev/null +++ b/tests/Fixtures/TestBundle/Model/SecurityFromParameter.php @@ -0,0 +1,36 @@ + + * + * 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\Model; + +use ApiPlatform\Metadata\Operation; + +/** + * Config-backed resource whose `security` expression is supplied through a container parameter + * (`%app.security.admin_only%`). Used to verify the YAML/XML extractor resolves a whole-string + * %param% into a working ExpressionLanguage expression (issue #8104). + */ +class SecurityFromParameter +{ + private ?int $id = null; + + public function getId(): ?int + { + return $this->id; + } + + public static function provide(Operation $operation, array $uriVariables = [], array $context = []): self + { + return new self(); + } +} diff --git a/tests/Fixtures/TestBundle/Resources/config/api_resources/resources.yaml b/tests/Fixtures/TestBundle/Resources/config/api_resources/resources.yaml index ba68a13d15..f784dfefa3 100644 --- a/tests/Fixtures/TestBundle/Resources/config/api_resources/resources.yaml +++ b/tests/Fixtures/TestBundle/Resources/config/api_resources/resources.yaml @@ -109,3 +109,12 @@ resources: ApiPlatform\Metadata\GetCollection: output: ApiPlatform\Tests\Fixtures\TestBundle\Dto\DummyIdCollectionDtoOutput provider: ApiPlatform\Tests\Fixtures\TestBundle\State\DummyCollectionDtoProvider + + ApiPlatform\Tests\Fixtures\TestBundle\Model\SecurityFromParameter: + security: '%app.security.admin_only%' + graphQlOperations: [ ] + operations: + ApiPlatform\Metadata\Get: + uriTemplate: /security_from_parameter/{id} + uriVariables: [id] + provider: 'ApiPlatform\Tests\Fixtures\TestBundle\Model\SecurityFromParameter::provide' diff --git a/tests/Fixtures/app/AppKernel.php b/tests/Fixtures/app/AppKernel.php index 6cb1bb8695..f9e925186b 100644 --- a/tests/Fixtures/app/AppKernel.php +++ b/tests/Fixtures/app/AppKernel.php @@ -110,6 +110,7 @@ protected function configureContainer(ContainerBuilder $c, LoaderInterface $load { $c->setParameter('kernel.project_dir', __DIR__); $c->setParameter('app.gen_id_default', $this->genIdDefault); + $c->setParameter('app.security.admin_only', 'is_granted("ROLE_ADMIN")'); $loader->load(__DIR__."/config/config_{$this->getEnvironment()}.yml"); diff --git a/tests/Functional/Security/SecurityFromContainerParameterAttributeTest.php b/tests/Functional/Security/SecurityFromContainerParameterAttributeTest.php new file mode 100644 index 0000000000..98034829bc --- /dev/null +++ b/tests/Functional/Security/SecurityFromContainerParameterAttributeTest.php @@ -0,0 +1,67 @@ + + * + * 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\Security; + +use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\SecurityFromContainerParameter; +use ApiPlatform\Tests\SetupClassResourcesTrait; +use Symfony\Component\Security\Core\User\InMemoryUser; + +/** + * End-to-end coverage for attribute parity of issue #8104: a resource declared through PHP + * attributes uses `security: '%app.security.admin_only%'`, where the container parameter holds the + * expression `is_granted("ROLE_ADMIN")`. The Symfony-bridge decorator must resolve the whole-string + * %param% into that expression so the access checker evaluates it (previously it reached + * ExpressionLanguage as the literal "%app.security.admin_only%" and threw). + */ +final class SecurityFromContainerParameterAttributeTest extends ApiTestCase +{ + use SetupClassResourcesTrait; + + protected static ?bool $alwaysBootKernel = false; + + /** + * @return class-string[] + */ + public static function getResources(): array + { + return [SecurityFromContainerParameter::class]; + } + + public function testGrantedRoleResolvesAndAllowsAccess(): void + { + $client = self::createClient(); + $client->loginUser(new InMemoryUser('admin', 'password', ['ROLE_ADMIN'])); + + $client->request('GET', '/security_from_container_parameter/1'); + $this->assertResponseIsSuccessful(); + } + + public function testMissingRoleIsDenied(): void + { + $client = self::createClient(); + $client->loginUser(new InMemoryUser('user', 'password', ['ROLE_USER'])); + + $client->request('GET', '/security_from_container_parameter/1'); + $this->assertResponseStatusCodeSame(403); + } + + public function testAnonymousIsDenied(): void + { + $client = self::createClient(); + + $client->request('GET', '/security_from_container_parameter/1'); + $this->assertResponseStatusCodeSame(401); + } +} diff --git a/tests/Functional/Security/SecurityFromContainerParameterTest.php b/tests/Functional/Security/SecurityFromContainerParameterTest.php new file mode 100644 index 0000000000..b3051a0fac --- /dev/null +++ b/tests/Functional/Security/SecurityFromContainerParameterTest.php @@ -0,0 +1,67 @@ + + * + * 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\Security; + +use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; +use ApiPlatform\Tests\Fixtures\TestBundle\Model\SecurityFromParameter; +use ApiPlatform\Tests\SetupClassResourcesTrait; +use Symfony\Component\Security\Core\User\InMemoryUser; + +/** + * End-to-end coverage for issue #8104: a config-based (YAML) resource declares + * `security: '%app.security.admin_only%'`, where the container parameter holds the expression + * `is_granted("ROLE_ADMIN")`. The extractor must resolve the whole-string %param% into that + * expression so it is actually evaluated by the access checker (previously it reached + * ExpressionLanguage as the literal "%app.security.admin_only%" and threw). + */ +final class SecurityFromContainerParameterTest extends ApiTestCase +{ + use SetupClassResourcesTrait; + + protected static ?bool $alwaysBootKernel = false; + + /** + * @return class-string[] + */ + public static function getResources(): array + { + return [SecurityFromParameter::class]; + } + + public function testGrantedRoleResolvesAndAllowsAccess(): void + { + $client = self::createClient(); + $client->loginUser(new InMemoryUser('admin', 'password', ['ROLE_ADMIN'])); + + $client->request('GET', '/security_from_parameter/1'); + $this->assertResponseIsSuccessful(); + } + + public function testMissingRoleIsDenied(): void + { + $client = self::createClient(); + $client->loginUser(new InMemoryUser('user', 'password', ['ROLE_USER'])); + + $client->request('GET', '/security_from_parameter/1'); + $this->assertResponseStatusCodeSame(403); + } + + public function testAnonymousIsDenied(): void + { + $client = self::createClient(); + + $client->request('GET', '/security_from_parameter/1'); + $this->assertResponseStatusCodeSame(401); + } +}