Skip to content

Commit

Permalink
Mark entity query as "insecure" (#935)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
pmelab authored Oct 31, 2019
1 parent 7d2b82c commit 3995b97
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/**
* @GraphQLField(
* id = "entity_query",
* secure = true,
* secure = false,
* type = "EntityQueryResult",
* arguments = {
* "filter" = "EntityQueryFilterInput",
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Drupal\Tests\graphql_core\Kernel\EntityQuery;

use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Tests\graphql_core\Kernel\GraphQLContentTestBase;

/**
Expand Down Expand Up @@ -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);
}

}

0 comments on commit 3995b97

Please sign in to comment.