From 3995b976a0028df3345f9556b8e45b97f59abe3c Mon Sep 17 00:00:00 2001 From: Philipp Melab Date: Thu, 31 Oct 2019 16:10:44 +0100 Subject: [PATCH] Mark entity query as "insecure" (#935) * Mark entity query fields as "insecure". * Added test case for insecure entity query filters. * Added comment to the entity query field why it is marked as insecure. --- .../Fields/EntityQuery/EntityQuery.php | 6 ++++- .../Kernel/EntityQuery/EntityQueryTest.php | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/modules/graphql_core/src/Plugin/GraphQL/Fields/EntityQuery/EntityQuery.php b/modules/graphql_core/src/Plugin/GraphQL/Fields/EntityQuery/EntityQuery.php index b91ba5079..ab7fddfef 100644 --- a/modules/graphql_core/src/Plugin/GraphQL/Fields/EntityQuery/EntityQuery.php +++ b/modules/graphql_core/src/Plugin/GraphQL/Fields/EntityQuery/EntityQuery.php @@ -17,7 +17,7 @@ /** * @GraphQLField( * id = "entity_query", - * secure = true, + * secure = false, * type = "EntityQueryResult", * arguments = { * "filter" = "EntityQueryFilterInput", @@ -37,6 +37,10 @@ * }, * deriver = "Drupal\graphql_core\Plugin\Deriver\Fields\EntityQueryDeriver" * ) + * + * This field is marked as not secure because it does not enforce entity field + * access over a chain of filters. For example node.uid.pass could be used as + * filter input which would disclose information about Drupal's password hashes. */ class EntityQuery extends FieldPluginBase implements ContainerFactoryPluginInterface { use DependencySerializationTrait; diff --git a/modules/graphql_core/tests/src/Kernel/EntityQuery/EntityQueryTest.php b/modules/graphql_core/tests/src/Kernel/EntityQuery/EntityQueryTest.php index 9168dab30..0566204c1 100644 --- a/modules/graphql_core/tests/src/Kernel/EntityQuery/EntityQueryTest.php +++ b/modules/graphql_core/tests/src/Kernel/EntityQuery/EntityQueryTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\graphql_core\Kernel\EntityQuery; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Tests\graphql_core\Kernel\GraphQLContentTestBase; /** @@ -107,4 +108,26 @@ public function testEntityQuery() { ], $metadata); } + /** + * Make sure entity filters are properly secured. + */ + public function testFilterSecurity() { + $metadata = new CacheableMetadata(); + $metadata->addCacheContexts([ + 'languages:language_content', + 'languages:language_interface', + 'languages:language_url', + 'user.permissions', + ]); + $metadata->addCacheTags(['graphql', 'user_list']); + $this->assertResults('query { userQuery (filter: { conditions: [ { field: "pass", value: "foo" } ] }) { count } }', [], [ + 'userQuery' => [ + // TODO: With proper access checking for filters this value should + // become "2" and the entity query field can be marked as secure + // again. + 'count' => 0, + ] + ], $metadata); + } + }