From 8934da31786e0c237efbe11e73b74e058a5e675f Mon Sep 17 00:00:00 2001 From: Andrii Khomych Date: Fri, 1 Oct 2021 16:10:26 +0300 Subject: [PATCH 1/5] feat: Add configurable validation security rules: - Disable introspection - Query depth - Query complexity --- config/schema/graphql.schema.yml | 9 +++++ src/Entity/Server.php | 69 +++++++++++++++++++++++++++++++- src/Entity/ServerInterface.php | 54 +++++++++++++++++++++++++ src/Form/ServerForm.php | 26 ++++++++++++ 4 files changed, 156 insertions(+), 2 deletions(-) diff --git a/config/schema/graphql.schema.yml b/config/schema/graphql.schema.yml index fd3657a7a..331a3ff01 100644 --- a/config/schema/graphql.schema.yml +++ b/config/schema/graphql.schema.yml @@ -23,6 +23,15 @@ graphql.graphql_servers.*: batching: type: boolean label: 'Batching' + disable_introspection: + type: boolean + label: 'Disable Introspection' + query_depth: + type: integer + label: 'Max query depth' + query_complexity: + type: number + label: 'Max query complexity' schema_configuration: type: 'graphql.schema.[%parent.schema]' persisted_queries_settings: diff --git a/src/Entity/Server.php b/src/Entity/Server.php index e569ea343..722fdf983 100644 --- a/src/Entity/Server.php +++ b/src/Entity/Server.php @@ -25,6 +25,9 @@ use GraphQL\Server\Helper; use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Validator\DocumentValidator; +use GraphQL\Validator\Rules\DisableIntrospection; +use GraphQL\Validator\Rules\QueryComplexity; +use GraphQL\Validator\Rules\QueryDepth; /** * The main GraphQL configuration and request entry point. @@ -59,7 +62,10 @@ * "endpoint", * "debug_flag", * "caching", - * "batching" + * "batching", + * "disable_introspection", + * "query_depth", + * "query_complexity" * }, * links = { * "collection" = "/admin/config/graphql/servers", @@ -498,10 +504,69 @@ protected function getValidationRules() { return []; } - return array_values(DocumentValidator::defaultRules()); + $rules = array_values(DocumentValidator::defaultRules()); + if ($this->getDisableIntrospection()) { + $rules[DisableIntrospection::class] = new DisableIntrospection(); + } + if ($this->getQueryDepth()) { + $rules[QueryDepth::class] = new QueryDepth($this->query_depth); + } + if ($this->getQueryComplexity()) { + $rules[QueryComplexity::class] = new QueryComplexity($this->query_complexity); + } + + return $rules; }; } + /** + * {@inheritdoc} + */ + public function getDisableIntrospection() { + return (bool) $this->get('disable_introspection'); + } + + /** + * {@inheritdoc} + */ + public function setDisableIntrospection($introspection) { + $this->set('disable_introspection', $introspection); + return $this; + } + + /** + * {@inheritdoc} + */ + public function getQueryDepth() { + return $this->get('query_depth'); + } + + /** + * {@inheritdoc} + */ + public function setQueryDepth($depth) { + $this->set('query_depth', $depth); + return $this; + } + + /** + * Gets query complexity config. + * + * @return int|null + * The query complexity, NULL otherwise. + */ + public function getQueryComplexity() { + return $this->get('query_complexity'); + } + + /** + * {@inheritdoc} + */ + public function setQueryComplexity($complexity) { + $this->set('query_complexity', $complexity); + return $this; + } + /** * {@inheritDoc} */ diff --git a/src/Entity/ServerInterface.php b/src/Entity/ServerInterface.php index 5ad8edc9b..a91f64b9d 100644 --- a/src/Entity/ServerInterface.php +++ b/src/Entity/ServerInterface.php @@ -71,4 +71,58 @@ public function getPersistedQueryInstances(); */ public function getSortedPersistedQueryInstances(); + /** + * Gets disable introspection config. + * + * @return bool + * The disable introspection config, FALSE otherwise. + */ + public function getDisableIntrospection(); + + /** + * Sets disable introspection config. + * + * @param bool $introspection + * The value for the disable introspection config. + * + * @return $this + */ + public function setDisableIntrospection($introspection); + + /** + * Gets query depth config. + * + * @return int|null + * The query depth, NULL otherwise. + */ + public function getQueryDepth(); + + /** + * Sets query depth config. + * + * @param int $depth + * The value for the query depth config. + * + * @return $this + */ + public function setQueryDepth($depth); + + /** + * Gets query complexity config. + * + * @return int|null + * The query complexity, NULL otherwise. + */ + public function getQueryComplexity(); + + /** + * Sets query complexity config. + * + * @param int $complexity + * The value for the query complexity config. + * + * @return $this + */ + public function setQueryComplexity($complexity); + } diff --git a/src/Form/ServerForm.php b/src/Form/ServerForm.php index d8876765b..7b5a93191 100644 --- a/src/Form/ServerForm.php +++ b/src/Form/ServerForm.php @@ -186,6 +186,32 @@ public function form(array $form, FormStateInterface $formState): array { '#description' => $this->t('Whether caching of queries and partial results is enabled.'), ]; + $form['validation'] = [ + '#title' => $this->t('Validation rules'), + '#type' => 'fieldset', + ]; + + $form['validation']['disable_introspection'] = [ + '#title' => $this->t('Disable introspection'), + '#type' => 'checkbox', + '#default_value' => $server->getDisableIntrospection(), + '#description' => $this->t('Security rule: Whether introspection should be disabled.'), + ]; + + $form['validation']['query_depth'] = [ + '#title' => $this->t('Max query depth'), + '#type' => 'number', + '#default_value' => $server->getQueryDepth(), + '#description' => $this->t('Security rule: The maximum allowed depth of nested queries. Leave empty to set unlimited.'), + ]; + + $form['validation']['query_complexity'] = [ + '#title' => $this->t('Max query complexity'), + '#default_value' => $server->getQueryComplexity(), + '#type' => 'number', + '#description' => $this->t('Security rule: The maximum allowed complexity of a query. Leave empty to set unlimited.'), + ]; + $debug_flags = $server->get('debug_flag') ?? 0; $form['debug_flag'] = [ '#title' => $this->t('Debug settings'), From 35da2647a0bfa5ea4c955360b8f83d357d6e8737 Mon Sep 17 00:00:00 2001 From: Andrii Khomych Date: Tue, 5 Oct 2021 18:21:53 +0300 Subject: [PATCH 2/5] Implements PR CRs. --- config/schema/graphql.schema.yml | 2 +- src/Entity/Server.php | 47 +++++++++++++++++++-------- src/Entity/ServerInterface.php | 54 -------------------------------- src/Form/ServerForm.php | 2 +- 4 files changed, 36 insertions(+), 69 deletions(-) diff --git a/config/schema/graphql.schema.yml b/config/schema/graphql.schema.yml index 331a3ff01..f90dc52f0 100644 --- a/config/schema/graphql.schema.yml +++ b/config/schema/graphql.schema.yml @@ -30,7 +30,7 @@ graphql.graphql_servers.*: type: integer label: 'Max query depth' query_complexity: - type: number + type: integer label: 'Max query complexity' schema_configuration: type: 'graphql.schema.[%parent.schema]' diff --git a/src/Entity/Server.php b/src/Entity/Server.php index 722fdf983..5ae50afae 100644 --- a/src/Entity/Server.php +++ b/src/Entity/Server.php @@ -520,31 +520,47 @@ protected function getValidationRules() { } /** - * {@inheritdoc} + * Gets disable introspection config. + * + * @return bool + * The disable introspection config, FALSE otherwise. */ - public function getDisableIntrospection() { + public function getDisableIntrospection(): bool { return (bool) $this->get('disable_introspection'); } /** - * {@inheritdoc} + * Sets disable introspection config. + * + * @param bool $introspection + * The value for the disable introspection config. + * + * @return $this */ - public function setDisableIntrospection($introspection) { + public function setDisableIntrospection(bool $introspection) { $this->set('disable_introspection', $introspection); return $this; } /** - * {@inheritdoc} + * Gets query depth config. + * + * @return int|null + * The query depth, NULL otherwise. */ - public function getQueryDepth() { - return $this->get('query_depth'); + public function getQueryDepth(): ?int { + return (int) $this->get('query_depth'); } /** - * {@inheritdoc} + * Sets query depth config. + * + * @param int $depth + * The value for the query depth config. + * + * @return $this */ - public function setQueryDepth($depth) { + public function setQueryDepth(int $depth) { $this->set('query_depth', $depth); return $this; } @@ -555,14 +571,19 @@ public function setQueryDepth($depth) { * @return int|null * The query complexity, NULL otherwise. */ - public function getQueryComplexity() { - return $this->get('query_complexity'); + public function getQueryComplexity(): ?int { + return (int) $this->get('query_complexity'); } /** - * {@inheritdoc} + * Sets query complexity config. + * + * @param int $complexity + * The value for the query complexity config. + * + * @return $this */ - public function setQueryComplexity($complexity) { + public function setQueryComplexity(int $complexity) { $this->set('query_complexity', $complexity); return $this; } diff --git a/src/Entity/ServerInterface.php b/src/Entity/ServerInterface.php index a91f64b9d..5ad8edc9b 100644 --- a/src/Entity/ServerInterface.php +++ b/src/Entity/ServerInterface.php @@ -71,58 +71,4 @@ public function getPersistedQueryInstances(); */ public function getSortedPersistedQueryInstances(); - /** - * Gets disable introspection config. - * - * @return bool - * The disable introspection config, FALSE otherwise. - */ - public function getDisableIntrospection(); - - /** - * Sets disable introspection config. - * - * @param bool $introspection - * The value for the disable introspection config. - * - * @return $this - */ - public function setDisableIntrospection($introspection); - - /** - * Gets query depth config. - * - * @return int|null - * The query depth, NULL otherwise. - */ - public function getQueryDepth(); - - /** - * Sets query depth config. - * - * @param int $depth - * The value for the query depth config. - * - * @return $this - */ - public function setQueryDepth($depth); - - /** - * Gets query complexity config. - * - * @return int|null - * The query complexity, NULL otherwise. - */ - public function getQueryComplexity(); - - /** - * Sets query complexity config. - * - * @param int $complexity - * The value for the query complexity config. - * - * @return $this - */ - public function setQueryComplexity($complexity); - } diff --git a/src/Form/ServerForm.php b/src/Form/ServerForm.php index 7b5a93191..cc1d3f3ea 100644 --- a/src/Form/ServerForm.php +++ b/src/Form/ServerForm.php @@ -87,7 +87,7 @@ public function ajaxSchemaConfigurationForm(array $form) { */ public function form(array $form, FormStateInterface $formState): array { $form = parent::form($form, $formState); - /** @var \Drupal\graphql\Entity\ServerInterface $server */ + /** @var \Drupal\graphql\Entity\Server $server */ $server = $this->entity; $schemas = array_map(function ($definition) { return $definition['name'] ?? $definition['id']; From a30121b0c4bb52812ece462e89deb9038d7bacfa Mon Sep 17 00:00:00 2001 From: Andrii Khomych Date: Wed, 6 Oct 2021 08:17:29 +0300 Subject: [PATCH 3/5] Fix the server form to use a config property instead of the getter. Fix server setters and fix rules logic. --- src/Entity/Server.php | 14 +++++++------- src/Form/ServerForm.php | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Entity/Server.php b/src/Entity/Server.php index 5ae50afae..8424f97df 100644 --- a/src/Entity/Server.php +++ b/src/Entity/Server.php @@ -506,13 +506,13 @@ protected function getValidationRules() { $rules = array_values(DocumentValidator::defaultRules()); if ($this->getDisableIntrospection()) { - $rules[DisableIntrospection::class] = new DisableIntrospection(); + $rules[] = new DisableIntrospection(); } if ($this->getQueryDepth()) { - $rules[QueryDepth::class] = new QueryDepth($this->query_depth); + $rules[] = new QueryDepth($this->getQueryDepth()); } if ($this->getQueryComplexity()) { - $rules[QueryComplexity::class] = new QueryComplexity($this->query_complexity); + $rules[] = new QueryComplexity($this->getQueryComplexity()); } return $rules; @@ -555,12 +555,12 @@ public function getQueryDepth(): ?int { /** * Sets query depth config. * - * @param int $depth + * @param int|null $depth * The value for the query depth config. * * @return $this */ - public function setQueryDepth(int $depth) { + public function setQueryDepth(?int $depth) { $this->set('query_depth', $depth); return $this; } @@ -578,12 +578,12 @@ public function getQueryComplexity(): ?int { /** * Sets query complexity config. * - * @param int $complexity + * @param int|null $complexity * The value for the query complexity config. * * @return $this */ - public function setQueryComplexity(int $complexity) { + public function setQueryComplexity(?int $complexity) { $this->set('query_complexity', $complexity); return $this; } diff --git a/src/Form/ServerForm.php b/src/Form/ServerForm.php index cc1d3f3ea..dbb735aee 100644 --- a/src/Form/ServerForm.php +++ b/src/Form/ServerForm.php @@ -194,20 +194,20 @@ public function form(array $form, FormStateInterface $formState): array { $form['validation']['disable_introspection'] = [ '#title' => $this->t('Disable introspection'), '#type' => 'checkbox', - '#default_value' => $server->getDisableIntrospection(), + '#default_value' => $server->get('disable_introspection'), '#description' => $this->t('Security rule: Whether introspection should be disabled.'), ]; $form['validation']['query_depth'] = [ '#title' => $this->t('Max query depth'), '#type' => 'number', - '#default_value' => $server->getQueryDepth(), + '#default_value' => $server->get('query_depth'), '#description' => $this->t('Security rule: The maximum allowed depth of nested queries. Leave empty to set unlimited.'), ]; $form['validation']['query_complexity'] = [ '#title' => $this->t('Max query complexity'), - '#default_value' => $server->getQueryComplexity(), + '#default_value' => $server->get('query_complexity'), '#type' => 'number', '#description' => $this->t('Security rule: The maximum allowed complexity of a query. Leave empty to set unlimited.'), ]; From 41ac0fa57131fe9bbf292356e2a56110a607173b Mon Sep 17 00:00:00 2001 From: Andrii Khomych Date: Wed, 6 Oct 2021 08:20:22 +0300 Subject: [PATCH 4/5] Revert back ServerForm to use ServerInterface. --- src/Form/ServerForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Form/ServerForm.php b/src/Form/ServerForm.php index dbb735aee..c1b747e37 100644 --- a/src/Form/ServerForm.php +++ b/src/Form/ServerForm.php @@ -87,7 +87,7 @@ public function ajaxSchemaConfigurationForm(array $form) { */ public function form(array $form, FormStateInterface $formState): array { $form = parent::form($form, $formState); - /** @var \Drupal\graphql\Entity\Server $server */ + /** @var \Drupal\graphql\Entity\ServerInterface $server */ $server = $this->entity; $schemas = array_map(function ($definition) { return $definition['name'] ?? $definition['id']; From 01a510d569c43e614f4b3e964a6f59c381d661a8 Mon Sep 17 00:00:00 2001 From: Andrii Khomych Date: Thu, 7 Oct 2021 10:04:10 +0300 Subject: [PATCH 5/5] Refactor to use variables instead of direct config call. --- src/Entity/Server.php | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Entity/Server.php b/src/Entity/Server.php index 8424f97df..003d66744 100644 --- a/src/Entity/Server.php +++ b/src/Entity/Server.php @@ -129,6 +129,27 @@ class Server extends ConfigEntityBase implements ServerInterface { */ public $batching = TRUE; + /** + * Whether to disable query introspection. + * + * @var bool + */ + public $disable_introspection = FALSE; + + /** + * The query complexity. + * + * @var int|null + */ + public $query_complexity = NULL; + + /** + * The query depth. + * + * @var int|null + */ + public $query_depth = NULL; + /** * The server's endpoint. * @@ -143,7 +164,6 @@ class Server extends ConfigEntityBase implements ServerInterface { */ public $persisted_queries_settings = []; - /** * Persisted query plugin instances available on this server. * @@ -526,7 +546,7 @@ protected function getValidationRules() { * The disable introspection config, FALSE otherwise. */ public function getDisableIntrospection(): bool { - return (bool) $this->get('disable_introspection'); + return (bool) $this->disable_introspection; } /** @@ -538,7 +558,7 @@ public function getDisableIntrospection(): bool { * @return $this */ public function setDisableIntrospection(bool $introspection) { - $this->set('disable_introspection', $introspection); + $this->disable_introspection = $introspection; return $this; } @@ -549,7 +569,7 @@ public function setDisableIntrospection(bool $introspection) { * The query depth, NULL otherwise. */ public function getQueryDepth(): ?int { - return (int) $this->get('query_depth'); + return (int) $this->query_depth; } /** @@ -561,7 +581,7 @@ public function getQueryDepth(): ?int { * @return $this */ public function setQueryDepth(?int $depth) { - $this->set('query_depth', $depth); + $this->query_depth = $depth; return $this; } @@ -572,7 +592,7 @@ public function setQueryDepth(?int $depth) { * The query complexity, NULL otherwise. */ public function getQueryComplexity(): ?int { - return (int) $this->get('query_complexity'); + return (int) $this->query_complexity; } /** @@ -584,7 +604,7 @@ public function getQueryComplexity(): ?int { * @return $this */ public function setQueryComplexity(?int $complexity) { - $this->set('query_complexity', $complexity); + $this->query_complexity = $complexity; return $this; }