Skip to content

Commit 3995b97

Browse files
authored
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.
1 parent 7d2b82c commit 3995b97

File tree

2 files changed

+28
-1
lines changed

2 files changed

+28
-1
lines changed

modules/graphql_core/src/Plugin/GraphQL/Fields/EntityQuery/EntityQuery.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
/**
1818
* @GraphQLField(
1919
* id = "entity_query",
20-
* secure = true,
20+
* secure = false,
2121
* type = "EntityQueryResult",
2222
* arguments = {
2323
* "filter" = "EntityQueryFilterInput",
@@ -37,6 +37,10 @@
3737
* },
3838
* deriver = "Drupal\graphql_core\Plugin\Deriver\Fields\EntityQueryDeriver"
3939
* )
40+
*
41+
* This field is marked as not secure because it does not enforce entity field
42+
* access over a chain of filters. For example node.uid.pass could be used as
43+
* filter input which would disclose information about Drupal's password hashes.
4044
*/
4145
class EntityQuery extends FieldPluginBase implements ContainerFactoryPluginInterface {
4246
use DependencySerializationTrait;

modules/graphql_core/tests/src/Kernel/EntityQuery/EntityQueryTest.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Drupal\Tests\graphql_core\Kernel\EntityQuery;
44

5+
use Drupal\Core\Cache\CacheableMetadata;
56
use Drupal\Tests\graphql_core\Kernel\GraphQLContentTestBase;
67

78
/**
@@ -107,4 +108,26 @@ public function testEntityQuery() {
107108
], $metadata);
108109
}
109110

111+
/**
112+
* Make sure entity filters are properly secured.
113+
*/
114+
public function testFilterSecurity() {
115+
$metadata = new CacheableMetadata();
116+
$metadata->addCacheContexts([
117+
'languages:language_content',
118+
'languages:language_interface',
119+
'languages:language_url',
120+
'user.permissions',
121+
]);
122+
$metadata->addCacheTags(['graphql', 'user_list']);
123+
$this->assertResults('query { userQuery (filter: { conditions: [ { field: "pass", value: "foo" } ] }) { count } }', [], [
124+
'userQuery' => [
125+
// TODO: With proper access checking for filters this value should
126+
// become "2" and the entity query field can be marked as secure
127+
// again.
128+
'count' => 0,
129+
]
130+
], $metadata);
131+
}
132+
110133
}

0 commit comments

Comments
 (0)