Merge pull request #389 from mcg-web/fix-acl-with-array

Fix acl with array
overblog · Sep 21, 2018 · dfae362 · dfae362
2 parents daefb71 + 84371c7
commit dfae362
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 8 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -23,6 +23,7 @@ matrix:
         - php: hhvm
         - php: nightly
     allow_failures:
+        - php: hhvm
         - php: nightly
 
 cache:
@@ -35,8 +36,11 @@ before_install:
     - composer selfupdate
     - if [ $SYMFONY_VERSION ]; then composer require "symfony/symfony:${SYMFONY_VERSION}" "symfony/framework-bundle:${SYMFONY_VERSION}" --dev --no-update; fi;
     - if [ $GRAPHQLPHP_VERSION ]; then composer require "webonyx/graphql-php:${GRAPHQLPHP_VERSION}" --dev --no-update; fi;
+    - echo "memory_limit=3072M" >> "$HOME/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini"
 
-install: composer update --prefer-source --no-interaction --optimize-autoloader
+install:
+    - composer update --prefer-source --no-interaction --optimize-autoloader
+    - echo "memory_limit=256M" >> "$HOME/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini"
 
 script:
     - bin/phpunit --debug $( if [ $TEST_COVERAGE = true ]; then echo "-d xdebug.max_nesting_level=1000 --coverage-clover=build/logs/clover.xml"; fi; )

diff --git a/Resolver/AccessResolver.php b/Resolver/AccessResolver.php
@@ -14,6 +14,8 @@
 use GraphQL\Executor\Promise\Adapter\SyncPromise;
 use GraphQL\Executor\Promise\Promise;
 use GraphQL\Executor\Promise\PromiseAdapter;
+use GraphQL\Type\Definition\ListOfType;
+use GraphQL\Type\Definition\ResolveInfo;
 use Overblog\GraphQLBundle\Error\UserError;
 use Overblog\GraphQLBundle\Error\UserWarning;
 use Overblog\GraphQLBundle\Relay\Connection\Output\Connection;
@@ -66,13 +68,13 @@ function ($result) use ($accessChecker, $resolveArgs) {
 
     private function processFilter($result, $accessChecker, $resolveArgs)
     {
-        if (is_array($result)) {
-            $result = array_map(
-                function ($object) use ($accessChecker, $resolveArgs) {
-                    return $this->hasAccess($accessChecker, $object, $resolveArgs) ? $object : null;
-                },
-                $result
-            );
+        /** @var ResolveInfo $resolveInfo */
+        $resolveInfo = $resolveArgs[3];
+
+        if (self::isIterable($result) && $resolveInfo->returnType instanceof ListOfType) {
+            foreach ($result as $i => $object) {
+                $result[$i] = $this->hasAccess($accessChecker, $object, $resolveArgs) ? $object : null;
+            }
         } elseif ($result instanceof Connection) {
             $result->edges = array_map(
                 function (Edge $edge) use ($accessChecker, $resolveArgs) {
@@ -96,4 +98,18 @@ private function hasAccess(callable $accessChecker, $object, array $resolveArgs
 
         return $access;
     }
+
+    /**
+     * @param mixed $data
+     *
+     * @return bool
+     */
+    private static function isIterable($data)
+    {
+        if (function_exists('is_iterable')) {
+            return \is_iterable($data);
+        } else {
+            return \is_array($data) || (\is_object($data) && ($data instanceof \Traversable));
+        }
+    }
 }
diff --git a/Tests/Functional/App/config/access/mapping/access.types.yml b/Tests/Functional/App/config/access/mapping/access.types.yml
@@ -6,6 +6,10 @@ RootQuery:
             user:
                 type: User
                 resolve: '@=resolver("query")'
+            youShallNotSeeThisUnauthenticated:
+                type: SecureField
+                access: '@=isFullyAuthenticated()'
+                resolve: '@=[]'
 
 Mutation:
     type: object
@@ -47,6 +51,17 @@ User:
         interfaces: [Human]
         isTypeOf: true
 
+SecureField:
+    type: object
+    config:
+        fields:
+            secretValue:
+                type: String!
+                resolve: 'top secret'
+            youAreAuthenticated:
+                type: Boolean!
+                resolve: '@=isFullyAuthenticated()'
+
 friendConnection:
     type: relay-connection
     config:

diff --git a/Tests/Functional/Security/AccessTest.php b/Tests/Functional/Security/AccessTest.php
@@ -96,6 +96,38 @@ public function testNotAuthenticatedUserAccessToUserName()
         $this->assertResponse($this->userNameQuery, $expected, static::ANONYMOUS_USER, 'access');
     }
 
+    public function testNonAuthenticatedUserAccessSecuredFieldWhichInitiallyResolvesToArray()
+    {
+        $expected = [
+            'data' => [
+                'youShallNotSeeThisUnauthenticated' => null,
+            ],
+            'extensions' => [
+                'warnings' => [
+                    [
+                        'message' => 'Access denied to this field.',
+                        'locations' => [
+                            [
+                                'line' => 2,
+                                'column' => 3,
+                            ],
+                        ],
+                        'path' => ['youShallNotSeeThisUnauthenticated'],
+                    ],
+                ],
+            ],
+        ];
+        $query = <<<'EOF'
+{
+  youShallNotSeeThisUnauthenticated {
+    secretValue
+    youAreAuthenticated
+  }
+}
+EOF;
+        $this->assertResponse($query, $expected, static::ANONYMOUS_USER, 'access');
+    }
+
     public function testFullyAuthenticatedUserAccessToUserName()
     {
         $expected = [