diff --git a/src/Metadata/Extractor/ResourceExtractorTrait.php b/src/Metadata/Extractor/ResourceExtractorTrait.php index d56abf3c63..ad9ff74457 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': @@ -71,6 +78,22 @@ private function phpize(\SimpleXMLElement|array|null $resource, string $key, str throw new InvalidArgumentException(\sprintf('The property "%s" must be a "%s", "%s" given.', $key, $type, \gettype($resource[$key]))); } + /** + * 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 (it is exactly what throws today), 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. + */ + private function resolveExpressionPlaceholder(mixed $value): mixed + { + if (!\is_string($value) || !preg_match('/^%[^%\s]+%$/', trim($value))) { + return $value; + } + + return $this->resolve(trim($value)); + } + private function buildArgs(\SimpleXMLElement $resource): ?array { if (!isset($resource->args->arg)) { diff --git a/src/Metadata/Extractor/XmlResourceExtractor.php b/src/Metadata/Extractor/XmlResourceExtractor.php index 6ebc1a66bf..a1888d5d0d 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'), @@ -131,11 +131,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 e7dd40093c..26cb821e6e 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'), @@ -154,11 +154,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 98ca94248e..36948afcf3 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; /** @@ -417,6 +418,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 6384942192..2cf8b70cba 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 @@ -575,6 +576,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/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/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); + } +}