Skip to content

Commit c23cbfe

Browse files
akommJeremiah VALERIE
authored andcommitted
Fix data leaked to unauthorized users
fixed #387
1 parent daefb71 commit c23cbfe

File tree

3 files changed

+53
-1
lines changed

3 files changed

+53
-1
lines changed

Resolver/AccessResolver.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
use GraphQL\Executor\Promise\Adapter\SyncPromise;
1515
use GraphQL\Executor\Promise\Promise;
1616
use GraphQL\Executor\Promise\PromiseAdapter;
17+
use GraphQL\Type\Definition\ListOfType;
18+
use GraphQL\Type\Definition\ResolveInfo;
1719
use Overblog\GraphQLBundle\Error\UserError;
1820
use Overblog\GraphQLBundle\Error\UserWarning;
1921
use Overblog\GraphQLBundle\Relay\Connection\Output\Connection;
@@ -66,7 +68,10 @@ function ($result) use ($accessChecker, $resolveArgs) {
6668

6769
private function processFilter($result, $accessChecker, $resolveArgs)
6870
{
69-
if (is_array($result)) {
71+
/** @var ResolveInfo $resolveInfo */
72+
$resolveInfo = $resolveArgs[3];
73+
74+
if (\is_array($result) && $resolveInfo->returnType instanceof ListOfType) {
7075
$result = array_map(
7176
function ($object) use ($accessChecker, $resolveArgs) {
7277
return $this->hasAccess($accessChecker, $object, $resolveArgs) ? $object : null;

Tests/Functional/App/config/access/mapping/access.types.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ RootQuery:
66
user:
77
type: User
88
resolve: '@=resolver("query")'
9+
youShallNotSeeThisUnauthenticated:
10+
type: SecureField
11+
access: '@=isFullyAuthenticated()'
12+
resolve: '@=[]'
913

1014
Mutation:
1115
type: object
@@ -47,6 +51,17 @@ User:
4751
interfaces: [Human]
4852
isTypeOf: true
4953

54+
SecureField:
55+
type: object
56+
config:
57+
fields:
58+
secretValue:
59+
type: String!
60+
resolve: 'top secret'
61+
youAreAuthenticated:
62+
type: Boolean!
63+
resolve: '@=isFullyAuthenticated()'
64+
5065
friendConnection:
5166
type: relay-connection
5267
config:

Tests/Functional/Security/AccessTest.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,38 @@ public function testNotAuthenticatedUserAccessToUserName()
9696
$this->assertResponse($this->userNameQuery, $expected, static::ANONYMOUS_USER, 'access');
9797
}
9898

99+
public function testNonAuthenticatedUserAccessSecuredFieldWhichInitiallyResolvesToArray(): void
100+
{
101+
$expected = [
102+
'data' => [
103+
'youShallNotSeeThisUnauthenticated' => null,
104+
],
105+
'extensions' => [
106+
'warnings' => [
107+
[
108+
'message' => 'Access denied to this field.',
109+
'locations' => [
110+
[
111+
'line' => 2,
112+
'column' => 3,
113+
],
114+
],
115+
'path' => ['youShallNotSeeThisUnauthenticated'],
116+
],
117+
],
118+
],
119+
];
120+
$query = <<<'EOF'
121+
{
122+
youShallNotSeeThisUnauthenticated {
123+
secretValue
124+
youAreAuthenticated
125+
}
126+
}
127+
EOF;
128+
$this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'access');
129+
}
130+
99131
public function testFullyAuthenticatedUserAccessToUserName()
100132
{
101133
$expected = [

0 commit comments

Comments
 (0)