diff --git a/CHANGELOG.md b/CHANGELOG.md index 17c0a4195a9..3be2c274f36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## v3.4.17 + +### Bug fixes + +* [55712452b](https://github.com/api-platform/core/commit/55712452b4f630978537bdb2a07dc958202336bb) fix(graphql): access to unauthorized resource using node Relay +* [7cb5a6db8](https://github.com/api-platform/core/commit/7cb5a6db87241d95e6c324318fe861bd4f1820cf) fix: allow parameter provider as object (#7032) +* [cba3acfbd](https://github.com/api-platform/core/commit/cba3acfbd517763cf320167250c5bed6d569696a) fix(graphql): property security might be cached w/ different objects +* [da2e86809](https://github.com/api-platform/core/commit/da2e86809d4a8dec294dc2fc148d92406f1f7fd1) fix: header parameter should be case insensitive (#7031) +* [f4c426d71](https://github.com/api-platform/core/commit/f4c426d719b01debaa993b00d03cce8964057ecc) Revert "fix(doctrine): throw an exception when a filter is not found in a par…" (#7046) + ## v3.4.16 ### Bug fixes diff --git a/src/GraphQl/Metadata/RuntimeOperationMetadataFactory.php b/src/GraphQl/Metadata/RuntimeOperationMetadataFactory.php new file mode 100644 index 00000000000..46fef884fec --- /dev/null +++ b/src/GraphQl/Metadata/RuntimeOperationMetadataFactory.php @@ -0,0 +1,55 @@ +<?php + +/* + * This file is part of the API Platform project. + * + * (c) Kévin Dunglas <dunglas@gmail.com> + * + * 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\GraphQl\Metadata; + +use ApiPlatform\Metadata\Exception\InvalidArgumentException; +use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\Operation; +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; +use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; +use Symfony\Component\Routing\Exception\ExceptionInterface as RoutingExceptionInterface; +use Symfony\Component\Routing\RouterInterface; + +/** + * This factory runs in the ResolverFactory and is used to find out a Relay node's operation. + */ +final class RuntimeOperationMetadataFactory implements OperationMetadataFactoryInterface +{ + public function __construct(private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly RouterInterface $router) + { + } + + public function create(string $uriTemplate, array $context = []): ?Operation + { + try { + $parameters = $this->router->match($uriTemplate); + } catch (RoutingExceptionInterface $e) { + throw new InvalidArgumentException(\sprintf('No route matches "%s".', $uriTemplate), $e->getCode(), $e); + } + + if (!isset($parameters['_api_resource_class'])) { + throw new InvalidArgumentException(\sprintf('The route "%s" is not an API route, it has no resource class in the defaults.', $uriTemplate)); + } + + foreach ($this->resourceMetadataCollectionFactory->create($parameters['_api_resource_class']) as $resource) { + foreach ($resource->getGraphQlOperations() ?? [] as $operation) { + if ($operation instanceof Query && !$operation->getResolver()) { + return $operation; + } + } + } + + throw new InvalidArgumentException(\sprintf('No operation found for id "%s".', $uriTemplate)); + } +} diff --git a/src/GraphQl/Resolver/Factory/ResolverFactory.php b/src/GraphQl/Resolver/Factory/ResolverFactory.php index c7ba03283ee..1aca8b41bab 100644 --- a/src/GraphQl/Resolver/Factory/ResolverFactory.php +++ b/src/GraphQl/Resolver/Factory/ResolverFactory.php @@ -15,21 +15,28 @@ use ApiPlatform\GraphQl\State\Provider\NoopProvider; use ApiPlatform\Metadata\DeleteOperationInterface; +use ApiPlatform\Metadata\Exception\InvalidArgumentException; use ApiPlatform\Metadata\GraphQl\Mutation; use ApiPlatform\Metadata\GraphQl\Operation; use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\State\Pagination\ArrayPaginator; use ApiPlatform\State\ProcessorInterface; use ApiPlatform\State\ProviderInterface; use GraphQL\Type\Definition\ResolveInfo; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class ResolverFactory implements ResolverFactoryInterface { public function __construct( private readonly ProviderInterface $provider, private readonly ProcessorInterface $processor, + private readonly ?OperationMetadataFactoryInterface $operationMetadataFactory = null, ) { + if (!$operationMetadataFactory) { + throw new InvalidArgumentException(\sprintf('Not injecting the "%s" exposes Relay nodes to a security risk.', OperationMetadataFactoryInterface::class)); + } } public function __invoke(?string $resourceClass = null, ?string $rootClass = null, ?Operation $operation = null, ?PropertyMetadataFactoryInterface $propertyMetadataFactory = null): callable @@ -70,7 +77,13 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul private function resolve(?array $source, array $args, ResolveInfo $info, ?string $rootClass = null, ?Operation $operation = null, mixed $body = null) { // Handles relay nodes - $operation ??= new Query(); + if (!$operation) { + if (!isset($args['id'])) { + throw new NotFoundHttpException('No node found.'); + } + + $operation = $this->operationMetadataFactory->create($args['id']); + } $graphQlContext = []; $context = ['source' => $source, 'args' => $args, 'info' => $info, 'root_class' => $rootClass, 'graphql_context' => &$graphQlContext]; diff --git a/src/GraphQl/Serializer/ItemNormalizer.php b/src/GraphQl/Serializer/ItemNormalizer.php index d0c7c384079..06ea9df299a 100644 --- a/src/GraphQl/Serializer/ItemNormalizer.php +++ b/src/GraphQl/Serializer/ItemNormalizer.php @@ -89,6 +89,8 @@ public function normalize(mixed $object, ?string $format = null, array $context if ($this->isCacheKeySafe($context)) { $context['cache_key'] = $this->getCacheKey($format, $context); + } else { + $context['cache_key'] = false; } unset($context['operation_name'], $context['operation']); // Remove operation and operation_name only when cache key has been created diff --git a/src/GraphQl/Tests/Metadata/RuntimeOperationMetadataFactoryTest.php b/src/GraphQl/Tests/Metadata/RuntimeOperationMetadataFactoryTest.php new file mode 100644 index 00000000000..5dfbfc22351 --- /dev/null +++ b/src/GraphQl/Tests/Metadata/RuntimeOperationMetadataFactoryTest.php @@ -0,0 +1,144 @@ +<?php + +/* + * This file is part of the API Platform project. + * + * (c) Kévin Dunglas <dunglas@gmail.com> + * + * 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\GraphQl\Tests\Metadata; + +use ApiPlatform\GraphQl\Metadata\RuntimeOperationMetadataFactory; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; +use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; +use PHPUnit\Framework\TestCase; +use Symfony\Component\Routing\Exception\ResourceNotFoundException; +use Symfony\Component\Routing\RouterInterface; + +class RuntimeOperationMetadataFactoryTest extends TestCase +{ + public function testCreate(): void + { + $resourceClass = 'Dummy'; + $operationName = 'item_query'; + + $operation = (new Query())->withName($operationName); + $resourceMetadata = (new ApiResource())->withGraphQlOperations([$operationName => $operation]); + $resourceMetadataCollection = new ResourceMetadataCollection($resourceClass, [$resourceMetadata]); + + $resourceMetadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactory->expects($this->once()) + ->method('create') + ->with($resourceClass) + ->willReturn($resourceMetadataCollection); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->once()) + ->method('match') + ->with('/dummies/1') + ->willReturn([ + '_api_resource_class' => $resourceClass, + '_api_operation_name' => $operationName, + ]); + + $factory = new RuntimeOperationMetadataFactory($resourceMetadataCollectionFactory, $router); + $this->assertEquals($operation, $factory->create('/dummies/1')); + } + + public function testCreateThrowsExceptionWhenRouteNotFound(): void + { + $this->expectException(\ApiPlatform\Metadata\Exception\InvalidArgumentException::class); + $this->expectExceptionMessage('No route matches "/unknown".'); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->once()) + ->method('match') + ->with('/unknown') + ->willThrowException(new ResourceNotFoundException()); + + $resourceMetadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + + $factory = new RuntimeOperationMetadataFactory($resourceMetadataCollectionFactory, $router); + $factory->create('/unknown'); + } + + public function testCreateThrowsExceptionWhenResourceClassMissing(): void + { + $this->expectException(\ApiPlatform\Metadata\Exception\InvalidArgumentException::class); + $this->expectExceptionMessage('The route "/dummies/1" is not an API route, it has no resource class in the defaults.'); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->once()) + ->method('match') + ->with('/dummies/1') + ->willReturn([]); + + $resourceMetadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + + $factory = new RuntimeOperationMetadataFactory($resourceMetadataCollectionFactory, $router); + $factory->create('/dummies/1'); + } + + public function testCreateThrowsExceptionWhenOperationNotFound(): void + { + $this->expectException(\ApiPlatform\Metadata\Exception\InvalidArgumentException::class); + $this->expectExceptionMessage('No operation found for id "/dummies/1".'); + + $resourceClass = 'Dummy'; + + $resourceMetadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactory->expects($this->once()) + ->method('create') + ->with($resourceClass) + ->willReturn(new ResourceMetadataCollection($resourceClass, [new ApiResource()])); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->once()) + ->method('match') + ->with('/dummies/1') + ->willReturn([ + '_api_resource_class' => $resourceClass, + ]); + + $factory = new RuntimeOperationMetadataFactory($resourceMetadataCollectionFactory, $router); + $factory->create('/dummies/1'); + } + + public function testCreateIgnoresOperationsWithResolvers(): void + { + $this->expectException(\ApiPlatform\Metadata\Exception\InvalidArgumentException::class); + $this->expectExceptionMessage('No operation found for id "/dummies/1".'); + + $resourceClass = 'Dummy'; + $operationName = 'item_query'; + + $operation = (new Query())->withResolver('t')->withName($operationName); + $resourceMetadata = (new ApiResource())->withGraphQlOperations([$operationName => $operation]); + $resourceMetadataCollection = new ResourceMetadataCollection($resourceClass, [$resourceMetadata]); + + $resourceMetadataCollectionFactory = $this->createMock(ResourceMetadataCollectionFactoryInterface::class); + $resourceMetadataCollectionFactory->expects($this->once()) + ->method('create') + ->with($resourceClass) + ->willReturn($resourceMetadataCollection); + + $router = $this->createMock(RouterInterface::class); + $router->expects($this->once()) + ->method('match') + ->with('/dummies/1') + ->willReturn([ + '_api_resource_class' => $resourceClass, + '_api_operation_name' => $operationName, + ]); + + $factory = new RuntimeOperationMetadataFactory($resourceMetadataCollectionFactory, $router); + $factory->create('/dummies/1'); + } +} diff --git a/src/GraphQl/Tests/Resolver/Factory/ResolverFactoryTest.php b/src/GraphQl/Tests/Resolver/Factory/ResolverFactoryTest.php index ee0be4aef60..8720c1b2a3c 100644 --- a/src/GraphQl/Tests/Resolver/Factory/ResolverFactoryTest.php +++ b/src/GraphQl/Tests/Resolver/Factory/ResolverFactoryTest.php @@ -18,6 +18,7 @@ use ApiPlatform\Metadata\GraphQl\Mutation; use ApiPlatform\Metadata\GraphQl\Operation; use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\Operation\Factory\OperationMetadataFactoryInterface; use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\State\ProcessorInterface; use ApiPlatform\State\ProviderInterface; @@ -45,7 +46,7 @@ public function testGraphQlResolver(?string $resourceClass = null, ?string $root $resolveInfo = $this->createMock(ResolveInfo::class); $resolveInfo->fieldName = 'test'; - $resolverFactory = new ResolverFactory($provider, $processor); + $resolverFactory = new ResolverFactory($provider, $processor, $this->createMock(OperationMetadataFactoryInterface::class)); $this->assertEquals($resolverFactory->__invoke($resourceClass, $rootClass, $operation, $propertyMetadataFactory)(['test' => null], [], [], $resolveInfo), $returnValue); } @@ -56,4 +57,21 @@ public static function graphQlQueries(): array ['Dummy', 'Dummy', new Mutation(), (new Mutation())->withValidate(true), (new Mutation())->withValidate(true)->withWrite(true)], ]; } + + public function testGraphQlResolverWithNode(): void + { + $returnValue = new \stdClass(); + $op = new Query(name: 'hi'); + $provider = $this->createMock(ProviderInterface::class); + $provider->expects($this->once())->method('provide')->with($op)->willReturn($returnValue); + $processor = $this->createMock(ProcessorInterface::class); + $processor->expects($this->once())->method('process')->with($returnValue, $op)->willReturn($returnValue); + $resolveInfo = $this->createMock(ResolveInfo::class); + $resolveInfo->fieldName = 'test'; + + $operationFactory = $this->createMock(OperationMetadataFactoryInterface::class); + $operationFactory->method('create')->with('/foo')->willReturn($op); + $resolverFactory = new ResolverFactory($provider, $processor, $operationFactory); + $this->assertSame($returnValue, $resolverFactory->__invoke()([], ['id' => '/foo'], [], $resolveInfo)); + } } diff --git a/src/Metadata/Resource/Factory/OperationDefaultsTrait.php b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php index dab37b23dae..f83cd4e80af 100644 --- a/src/Metadata/Resource/Factory/OperationDefaultsTrait.php +++ b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php @@ -112,7 +112,7 @@ private function getDefaultHttpOperations($resource): iterable private function addDefaultGraphQlOperations(ApiResource $resource): ApiResource { - $operations = enum_exists($resource->getClass()) ? [new QueryCollection(paginationEnabled: false), new Query()] : [new QueryCollection(), new Query(), (new Mutation())->withName('update'), (new DeleteMutation())->withName('delete'), (new Mutation())->withName('create')]; + $operations = enum_exists($resource->getClass()) ? [new Query(), new QueryCollection(paginationEnabled: false)] : [new Query(), new QueryCollection(), (new Mutation())->withName('update'), (new DeleteMutation())->withName('delete'), (new Mutation())->withName('create')]; $graphQlOperations = []; foreach ($operations as $operation) { [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); diff --git a/src/Symfony/Bundle/Resources/config/graphql.xml b/src/Symfony/Bundle/Resources/config/graphql.xml index f532c6ab500..807b7173247 100644 --- a/src/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Symfony/Bundle/Resources/config/graphql.xml @@ -191,6 +191,12 @@ <service id="api_platform.graphql.resolver.factory.item" class="ApiPlatform\GraphQl\Resolver\Factory\ResolverFactory" public="false"> <argument type="service" id="api_platform.graphql.state_provider" /> <argument type="service" id="api_platform.graphql.state_processor" /> + <argument type="service" id="api_platform.graphql.runtime_operation_metadata_factory" /> + </service> + + <service id="api_platform.graphql.runtime_operation_metadata_factory" class="ApiPlatform\GraphQl\Metadata\RuntimeOperationMetadataFactory" public="false"> + <argument type="service" id="api_platform.metadata.resource.metadata_collection_factory" /> + <argument type="service" id="api_platform.router" /> </service> <!-- Resolver Stages --> diff --git a/tests/Fixtures/TestBundle/Document/SecuredDummyCollection.php b/tests/Fixtures/TestBundle/Document/SecuredDummyCollection.php new file mode 100644 index 00000000000..7356c404f86 --- /dev/null +++ b/tests/Fixtures/TestBundle/Document/SecuredDummyCollection.php @@ -0,0 +1,60 @@ +<?php + +/* + * This file is part of the API Platform project. + * + * (c) Kévin Dunglas <dunglas@gmail.com> + * + * 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\Document; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\GraphQl\QueryCollection; +use ApiPlatform\Metadata\NotExposed; +use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; + +/** + * Secured resource. + */ +#[ApiResource( + operations: [ + new NotExposed(), + ], + graphQlOperations: [ + new Query(), + new QueryCollection(), + ], + security: 'is_granted(\'ROLE_USER\')' +)] +#[ODM\Document] +class SecuredDummyCollection +{ + #[ODM\Id(strategy: 'INCREMENT', type: 'int')] + public ?int $id = null; + + /** + * @var string The title + */ + #[ODM\Field] + public string $title; + + /** + * @var string Secret property, only readable/writable by owners + */ + #[ApiProperty(security: 'object == null or object.owner == user', securityPostDenormalize: 'object.owner == user')] + #[ODM\Field] + public ?string $ownerOnlyProperty = null; + + /** + * @var string The owner + */ + #[ODM\Field] + public string $owner; +} diff --git a/tests/Fixtures/TestBundle/Document/SecuredDummyCollectionParent.php b/tests/Fixtures/TestBundle/Document/SecuredDummyCollectionParent.php new file mode 100644 index 00000000000..efe23f5372d --- /dev/null +++ b/tests/Fixtures/TestBundle/Document/SecuredDummyCollectionParent.php @@ -0,0 +1,43 @@ +<?php + +/* + * This file is part of the API Platform project. + * + * (c) Kévin Dunglas <dunglas@gmail.com> + * + * 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\Document; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\GraphQl\QueryCollection; +use ApiPlatform\Metadata\NotExposed; +use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; + +/** + * Secured resource. + */ +#[ApiResource( + operations: [ + new NotExposed(), + ], + graphQlOperations: [ + new Query(), + new QueryCollection(), + ], + security: 'is_granted(\'ROLE_USER\')' +)] +#[ODM\Document] +class SecuredDummyCollectionParent +{ + #[ODM\Id(strategy: 'INCREMENT', type: 'int')] + public ?int $id = null; + + #[ODM\ReferenceOne(targetDocument: SecuredDummyCollection::class)] + public ?SecuredDummyCollection $child = null; +} diff --git a/tests/Fixtures/TestBundle/Entity/SecuredDummyCollection.php b/tests/Fixtures/TestBundle/Entity/SecuredDummyCollection.php new file mode 100644 index 00000000000..03abb4a3000 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/SecuredDummyCollection.php @@ -0,0 +1,62 @@ +<?php + +/* + * This file is part of the API Platform project. + * + * (c) Kévin Dunglas <dunglas@gmail.com> + * + * 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\Entity; + +use ApiPlatform\Metadata\ApiProperty; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\GraphQl\QueryCollection; +use ApiPlatform\Metadata\NotExposed; +use Doctrine\ORM\Mapping as ORM; + +/** + * Secured resource. + */ +#[ApiResource( + operations: [ + new NotExposed(), + ], + graphQlOperations: [ + new Query(), + new QueryCollection(), + ], + security: 'is_granted(\'ROLE_USER\')' +)] +#[ORM\Entity] +class SecuredDummyCollection +{ + #[ORM\Column(type: 'integer')] + #[ORM\Id] + #[ORM\GeneratedValue(strategy: 'AUTO')] + public ?int $id = null; + + /** + * @var string The title + */ + #[ORM\Column] + public string $title; + + /** + * @var string Secret property, only readable/writable by owners + */ + #[ApiProperty(security: 'object == null or object.owner == user', securityPostDenormalize: 'object.owner == user')] + #[ORM\Column] + public ?string $ownerOnlyProperty = null; + + /** + * @var string The owner + */ + #[ORM\Column] + public string $owner; +} diff --git a/tests/Fixtures/TestBundle/Entity/SecuredDummyCollectionParent.php b/tests/Fixtures/TestBundle/Entity/SecuredDummyCollectionParent.php new file mode 100644 index 00000000000..ad053cfc456 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/SecuredDummyCollectionParent.php @@ -0,0 +1,46 @@ +<?php + +/* + * This file is part of the API Platform project. + * + * (c) Kévin Dunglas <dunglas@gmail.com> + * + * 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\Entity; + +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\GraphQl\QueryCollection; +use ApiPlatform\Metadata\NotExposed; +use Doctrine\ORM\Mapping as ORM; + +/** + * Secured resource. + */ +#[ApiResource( + operations: [ + new NotExposed(), + ], + graphQlOperations: [ + new Query(), + new QueryCollection(), + ], + security: 'is_granted(\'ROLE_USER\')' +)] +#[ORM\Entity] +class SecuredDummyCollectionParent +{ + #[ORM\Column(type: 'integer')] + #[ORM\Id] + #[ORM\GeneratedValue(strategy: 'AUTO')] + public ?int $id = null; + + #[ORM\ManyToOne] + #[ORM\JoinColumn(nullable: false)] + public SecuredDummyCollection $child; +}