From 6e2bbc76b475c4b173685af7fa0d8716cc2400a1 Mon Sep 17 00:00:00 2001 From: mauriau Date: Tue, 10 Dec 2024 15:31:38 +0100 Subject: [PATCH 1/5] feat(graphql): allow to configure max query depth and max query complexity --- src/GraphQl/Executor.php | 10 +++++++++- src/GraphQl/Tests/ExecutorTest.php | 18 ++++++++++++++++++ .../ApiPlatformExtension.php | 5 ++++- .../DependencyInjection/Configuration.php | 13 ++++++++----- .../Bundle/Resources/config/graphql.xml | 2 ++ .../DependencyInjection/ConfigurationTest.php | 2 ++ 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/GraphQl/Executor.php b/src/GraphQl/Executor.php index 5c2ecddf7f1..4a9b5a51c90 100644 --- a/src/GraphQl/Executor.php +++ b/src/GraphQl/Executor.php @@ -18,6 +18,8 @@ use GraphQL\Type\Schema; use GraphQL\Validator\DocumentValidator; use GraphQL\Validator\Rules\DisableIntrospection; +use GraphQL\Validator\Rules\QueryComplexity; +use GraphQL\Validator\Rules\QueryDepth; /** * Wrapper for the GraphQL facade. @@ -26,13 +28,19 @@ */ final class Executor implements ExecutorInterface { - public function __construct(private readonly bool $graphQlIntrospectionEnabled = true) + public function __construct(private readonly bool $graphQlIntrospectionEnabled = true, private readonly int $maxQueryComplexity = 100, private readonly int $maxQueryDepth = 100) { DocumentValidator::addRule( new DisableIntrospection( $this->graphQlIntrospectionEnabled ? DisableIntrospection::DISABLED : DisableIntrospection::ENABLED ) ); + + $queryComplexity = new QueryComplexity($this->maxQueryComplexity); + DocumentValidator::addRule($queryComplexity); + + $queryDepth = new QueryDepth($this->maxQueryDepth); + DocumentValidator::addRule($queryDepth); } /** diff --git a/src/GraphQl/Tests/ExecutorTest.php b/src/GraphQl/Tests/ExecutorTest.php index 43cc420dded..e6b64c77a85 100644 --- a/src/GraphQl/Tests/ExecutorTest.php +++ b/src/GraphQl/Tests/ExecutorTest.php @@ -16,6 +16,8 @@ use ApiPlatform\GraphQl\Executor; use GraphQL\Validator\DocumentValidator; use GraphQL\Validator\Rules\DisableIntrospection; +use GraphQL\Validator\Rules\QueryComplexity; +use GraphQL\Validator\Rules\QueryDepth; use PHPUnit\Framework\TestCase; /** @@ -38,4 +40,20 @@ public function testDisableIntrospectionQuery(): void $expected = new DisableIntrospection(DisableIntrospection::ENABLED); $this->assertEquals($expected, DocumentValidator::getRule(DisableIntrospection::class)); } + + public function testChangeValueOfMaxQueryDepth(): void + { + $executor = new Executor(true, 20); + + $expected = new QueryComplexity(20); + $this->assertEquals($expected, DocumentValidator::getRule(QueryComplexity::class)); + } + + public function testChangeValueOfMaxQueryComplexity(): void + { + $executor = new Executor(true, maxQueryDepth: 20); + + $expected = new QueryDepth(20); + $this->assertEquals($expected, DocumentValidator::getRule(QueryDepth::class)); + } } diff --git a/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index cfeabc1d642..4a3f09df3cb 100644 --- a/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -565,14 +565,17 @@ private function registerGraphQlConfiguration(ContainerBuilder $container, array { $enabled = $this->isConfigEnabled($container, $config['graphql']); $graphqlIntrospectionEnabled = $enabled && $this->isConfigEnabled($container, $config['graphql']['introspection']); - $graphiqlEnabled = $enabled && $this->isConfigEnabled($container, $config['graphql']['graphiql']); $graphqlPlayGroundEnabled = $enabled && $this->isConfigEnabled($container, $config['graphql']['graphql_playground']); + $maxQueryDepth = (int) $config['graphql']['max_query_depth']; + $maxQueryComplexity = (int) $config['graphql']['max_query_complexity']; if ($graphqlPlayGroundEnabled) { trigger_deprecation('api-platform/core', '3.1', 'GraphQL Playground is deprecated and will be removed in API Platform 4.0. Only GraphiQL will be available in the future. Set api_platform.graphql.graphql_playground to false in the configuration to remove this deprecation.'); } $container->setParameter('api_platform.graphql.enabled', $enabled); + $container->setParameter('api_platform.graphql.max_query_depth', $maxQueryDepth); + $container->setParameter('api_platform.graphql.max_query_complexity', $maxQueryComplexity); $container->setParameter('api_platform.graphql.introspection.enabled', $graphqlIntrospectionEnabled); $container->setParameter('api_platform.graphql.graphiql.enabled', $graphiqlEnabled); $container->setParameter('api_platform.graphql.graphql_playground.enabled', $graphqlPlayGroundEnabled); diff --git a/src/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/DependencyInjection/Configuration.php index c8619006d2e..0e26a10d5d9 100644 --- a/src/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/DependencyInjection/Configuration.php @@ -14,7 +14,6 @@ namespace ApiPlatform\Symfony\Bundle\DependencyInjection; use ApiPlatform\Doctrine\Common\Filter\OrderFilterInterface; -use ApiPlatform\Elasticsearch\State\Options; use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\Exception\InvalidArgumentException; use ApiPlatform\Metadata\Post; @@ -83,7 +82,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->defaultValue('0.0.0') ->end() ->booleanNode('show_webby')->defaultTrue()->info('If true, show Webby on the documentation page')->end() - ->booleanNode('use_symfony_listeners')->defaultFalse()->info(sprintf('Uses Symfony event listeners instead of the %s.', MainController::class))->end() + ->booleanNode('use_symfony_listeners')->defaultFalse()->info(\sprintf('Uses Symfony event listeners instead of the %s.', MainController::class))->end() ->scalarNode('name_converter')->defaultNull()->info('Specify a name converter to use.')->end() ->scalarNode('asset_package')->defaultNull()->info('Specify an asset package name to use.')->end() ->scalarNode('path_segment_name_generator')->defaultValue('api_platform.metadata.path_segment_name_generator.underscore')->info('Specify a path name generator to use.')->end() @@ -165,7 +164,7 @@ public function getConfigTreeBuilder(): TreeBuilder $this->addExceptionToStatusSection($rootNode); $this->addFormatSection($rootNode, 'formats', [ - 'jsonld' => ['mime_types' => ['application/ld+json']] + 'jsonld' => ['mime_types' => ['application/ld+json']], ]); $this->addFormatSection($rootNode, 'patch_formats', [ 'json' => ['mime_types' => ['application/merge-patch+json']], @@ -267,6 +266,10 @@ private function addGraphQlSection(ArrayNodeDefinition $rootNode): void ->arrayNode('introspection') ->canBeDisabled() ->end() + ->integerNode('max_query_depth')->defaultValue(100) + ->end() + ->integerNode('max_query_complexity')->defaultValue(100) + ->end() ->scalarNode('nesting_separator')->defaultValue('_')->info('The separator to use to filter nested fields.')->end() ->arrayNode('collection') ->addDefaultsIfNotSet() @@ -308,7 +311,7 @@ private function addSwaggerSection(ArrayNodeDefinition $rootNode): void ->end() ->validate() ->ifTrue(static fn ($v): bool => $v !== array_intersect($v, $supportedVersions)) - ->thenInvalid(sprintf('Only the versions %s are supported. Got %s.', implode(' and ', $supportedVersions), '%s')) + ->thenInvalid(\sprintf('Only the versions %s are supported. Got %s.', implode(' and ', $supportedVersions), '%s')) ->end() ->prototype('scalar')->end() ->end() @@ -542,7 +545,7 @@ private function addExceptionToStatusSection(ArrayNodeDefinition $rootNode): voi ->then(static function (array $exceptionToStatus): array { foreach ($exceptionToStatus as $httpStatusCode) { if ($httpStatusCode < 100 || $httpStatusCode >= 600) { - throw new InvalidConfigurationException(sprintf('The HTTP status code "%s" is not valid.', $httpStatusCode)); + throw new InvalidConfigurationException(\sprintf('The HTTP status code "%s" is not valid.', $httpStatusCode)); } } diff --git a/src/Symfony/Bundle/Resources/config/graphql.xml b/src/Symfony/Bundle/Resources/config/graphql.xml index c58b39b1c23..69e200575bb 100644 --- a/src/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Symfony/Bundle/Resources/config/graphql.xml @@ -7,6 +7,8 @@ %api_platform.graphql.introspection.enabled% + %api_platform.graphql.max_query_depth% + %api_platform.graphql.max_query_complexity% diff --git a/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php b/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php index 572de7d7960..b8b31a12eec 100644 --- a/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php +++ b/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php @@ -123,6 +123,8 @@ private function runDefaultConfigTests(array $doctrineIntegrationsToLoad = ['orm 'introspection' => [ 'enabled' => true, ], + 'max_query_depth' => 100, + 'max_query_complexity' => 100, 'nesting_separator' => '_', 'collection' => [ 'pagination' => [ From c039a79d802115c860cf9118e0dc6369e22e7553 Mon Sep 17 00:00:00 2001 From: mauriau Date: Wed, 11 Dec 2024 12:57:33 +0100 Subject: [PATCH 2/5] feat(graphql): set graphql max query depth to 20 and max complexity to 500 Set max query depth to 200 in test AppKernel, it's required for introspection.feature --- src/GraphQl/Executor.php | 2 +- src/Symfony/Bundle/DependencyInjection/Configuration.php | 4 ++-- tests/Fixtures/app/AppKernel.php | 1 + .../Symfony/Bundle/DependencyInjection/ConfigurationTest.php | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/GraphQl/Executor.php b/src/GraphQl/Executor.php index 4a9b5a51c90..36359a3534b 100644 --- a/src/GraphQl/Executor.php +++ b/src/GraphQl/Executor.php @@ -28,7 +28,7 @@ */ final class Executor implements ExecutorInterface { - public function __construct(private readonly bool $graphQlIntrospectionEnabled = true, private readonly int $maxQueryComplexity = 100, private readonly int $maxQueryDepth = 100) + public function __construct(private readonly bool $graphQlIntrospectionEnabled = true, private readonly int $maxQueryComplexity = 500, private readonly int $maxQueryDepth = 20) { DocumentValidator::addRule( new DisableIntrospection( diff --git a/src/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/DependencyInjection/Configuration.php index 0e26a10d5d9..aa5d2ad1791 100644 --- a/src/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/DependencyInjection/Configuration.php @@ -266,9 +266,9 @@ private function addGraphQlSection(ArrayNodeDefinition $rootNode): void ->arrayNode('introspection') ->canBeDisabled() ->end() - ->integerNode('max_query_depth')->defaultValue(100) + ->integerNode('max_query_depth')->defaultValue(20) ->end() - ->integerNode('max_query_complexity')->defaultValue(100) + ->integerNode('max_query_complexity')->defaultValue(500) ->end() ->scalarNode('nesting_separator')->defaultValue('_')->info('The separator to use to filter nested fields.')->end() ->arrayNode('collection') diff --git a/tests/Fixtures/app/AppKernel.php b/tests/Fixtures/app/AppKernel.php index 7d55824f4de..3300078d8f2 100644 --- a/tests/Fixtures/app/AppKernel.php +++ b/tests/Fixtures/app/AppKernel.php @@ -251,6 +251,7 @@ class_exists(NativePasswordHasher::class) ? 'password_hashers' : 'encoders' => [ ], 'graphql' => [ 'graphql_playground' => false, + 'max_query_depth' => 200, ], 'use_symfony_listeners' => $useSymfonyListeners, 'defaults' => [ diff --git a/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php b/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php index b8b31a12eec..c2690bbff9a 100644 --- a/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php +++ b/tests/Symfony/Bundle/DependencyInjection/ConfigurationTest.php @@ -123,8 +123,8 @@ private function runDefaultConfigTests(array $doctrineIntegrationsToLoad = ['orm 'introspection' => [ 'enabled' => true, ], - 'max_query_depth' => 100, - 'max_query_complexity' => 100, + 'max_query_depth' => 20, + 'max_query_complexity' => 500, 'nesting_separator' => '_', 'collection' => [ 'pagination' => [ From 886259685b2ca038c9ef8fb8e2d7a59cd1a75b5a Mon Sep 17 00:00:00 2001 From: mauriau Date: Thu, 12 Dec 2024 17:10:53 +0100 Subject: [PATCH 3/5] refactor(graphql): swap paremters order in DI --- src/Symfony/Bundle/Resources/config/graphql.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/Resources/config/graphql.xml b/src/Symfony/Bundle/Resources/config/graphql.xml index 69e200575bb..ee5f4fcc177 100644 --- a/src/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Symfony/Bundle/Resources/config/graphql.xml @@ -7,8 +7,8 @@ %api_platform.graphql.introspection.enabled% - %api_platform.graphql.max_query_depth% %api_platform.graphql.max_query_complexity% + %api_platform.graphql.max_query_depth% From ee4c125f97bf615a2457771000263aea81bfe8b9 Mon Sep 17 00:00:00 2001 From: mauriau Date: Tue, 17 Dec 2024 17:16:38 +0100 Subject: [PATCH 4/5] style(graphql): remove backslash on sprintf --- src/Symfony/Bundle/DependencyInjection/Configuration.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/DependencyInjection/Configuration.php index aa5d2ad1791..e7895b10ae8 100644 --- a/src/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/DependencyInjection/Configuration.php @@ -82,7 +82,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->defaultValue('0.0.0') ->end() ->booleanNode('show_webby')->defaultTrue()->info('If true, show Webby on the documentation page')->end() - ->booleanNode('use_symfony_listeners')->defaultFalse()->info(\sprintf('Uses Symfony event listeners instead of the %s.', MainController::class))->end() + ->booleanNode('use_symfony_listeners')->defaultFalse()->info(sprintf('Uses Symfony event listeners instead of the %s.', MainController::class))->end() ->scalarNode('name_converter')->defaultNull()->info('Specify a name converter to use.')->end() ->scalarNode('asset_package')->defaultNull()->info('Specify an asset package name to use.')->end() ->scalarNode('path_segment_name_generator')->defaultValue('api_platform.metadata.path_segment_name_generator.underscore')->info('Specify a path name generator to use.')->end() @@ -311,7 +311,7 @@ private function addSwaggerSection(ArrayNodeDefinition $rootNode): void ->end() ->validate() ->ifTrue(static fn ($v): bool => $v !== array_intersect($v, $supportedVersions)) - ->thenInvalid(\sprintf('Only the versions %s are supported. Got %s.', implode(' and ', $supportedVersions), '%s')) + ->thenInvalid(sprintf('Only the versions %s are supported. Got %s.', implode(' and ', $supportedVersions), '%s')) ->end() ->prototype('scalar')->end() ->end() @@ -545,7 +545,7 @@ private function addExceptionToStatusSection(ArrayNodeDefinition $rootNode): voi ->then(static function (array $exceptionToStatus): array { foreach ($exceptionToStatus as $httpStatusCode) { if ($httpStatusCode < 100 || $httpStatusCode >= 600) { - throw new InvalidConfigurationException(\sprintf('The HTTP status code "%s" is not valid.', $httpStatusCode)); + throw new InvalidConfigurationException(sprintf('The HTTP status code "%s" is not valid.', $httpStatusCode)); } } From e2ddeb67f0c563f918d5dd910c1b37df8950e368 Mon Sep 17 00:00:00 2001 From: mauriau Date: Tue, 17 Dec 2024 17:23:46 +0100 Subject: [PATCH 5/5] feat(graphql): add configuration on laravel, for max query depth and max query complexity --- src/Laravel/ApiPlatformProvider.php | 2 +- src/Laravel/config/api-platform.php | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Laravel/ApiPlatformProvider.php b/src/Laravel/ApiPlatformProvider.php index a13fb63b0e5..fe9123da09e 100644 --- a/src/Laravel/ApiPlatformProvider.php +++ b/src/Laravel/ApiPlatformProvider.php @@ -1294,7 +1294,7 @@ private function registerGraphQl(Application $app): void /** @var ConfigRepository */ $config = $app['config']; - return new Executor($config->get('api-platform.graphql.introspection.enabled') ?? false); + return new Executor($config->get('api-platform.graphql.introspection.enabled') ?? false, $config->get('api-platform.graphql.max_query_complexity'), $config->get('api-platform.graphql.max_query_depth')); }); $app->singleton(GraphiQlController::class, function (Application $app) { diff --git a/src/Laravel/config/api-platform.php b/src/Laravel/config/api-platform.php index f24a85a9a13..6056db6303c 100644 --- a/src/Laravel/config/api-platform.php +++ b/src/Laravel/config/api-platform.php @@ -62,7 +62,9 @@ 'graphql' => [ 'enabled' => false, 'nesting_separator' => '__', - 'introspection' => ['enabled' => true] + 'introspection' => ['enabled' => true], + 'max_query_complexity' => 500, + 'max_query_depth' => 200 ], 'exception_to_status' => [