Merge pull request #1 from overblog/fix_access_resolve_false_values

Fix Access resolving when field value is false or 0 or empty string
overblog · Mar 10, 2016 · 62e1fb3 · 62e1fb3
2 parents b9400f7 + c534593
commit 62e1fb3
Show file tree

Hide file tree

Showing 3 changed files with 141 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -707,7 +707,7 @@ Expression | Description | Scope
 **request** | Refers to the current request. | Request
 **token** | Refers to the token which is currently in the security token storage. | Token 
 **user** | Refers to the user which is currently in the security token storage. | Valid Token
-**object** | Refers to the object for which access is being requested. | only available for `config.fields.*.access`
+**object** | Refers to the value of the field for which access is being requested. For array `object` will be each item of the array. For Relay connection `object` will be the node of each connection edges. | only available for `config.fields.*.access`
 **value** | Resolver value | only available in resolve context 
 **args** | Resolver args array | only available in resolve context 
 **info** | Resolver GraphQL\Type\Definition\ResolveInfo Object | only available in resolve context

diff --git a/Resolver/ConfigResolver.php b/Resolver/ConfigResolver.php
@@ -256,7 +256,7 @@ private function resolveAccessAndWrapResolveCallback($expression, callable $reso
 
             $values = call_user_func_array([$this, 'resolveResolveCallbackArgs'], $args);
 
-            $checkAccess = function ($object) use ($expression, $values) {
+            $checkAccess = function ($object, $throwException = false) use ($expression, $values) {
                 try {
                     $access = $this->resolveUsingExpressionLanguageIfNeeded(
                         $expression,
@@ -266,33 +266,39 @@ private function resolveAccessAndWrapResolveCallback($expression, callable $reso
                     $access = false;
                 }
 
-                if (!$access) {
+                if ($throwException && !$access) {
                     throw new UserError('Access denied to this field.');
                 }
 
-                return true;
+                return $access;
             };
 
-            if (is_array($result) || $result instanceof \ArrayAccess) {
-                $result = array_filter(
-                    array_map(
-                        function ($object) use ($checkAccess) {
-                            return $checkAccess($object) ? $object : null;
+            switch (true) {
+                case is_array($result) || $result instanceof \ArrayAccess:
+                    $result = array_filter(
+                        array_map(
+                            function ($object) use ($checkAccess) {
+                                return $checkAccess($object) ? $object : null;
+                            },
+                            $result
+                        )
+                    );
+                    break;
+
+                case $result instanceof Connection:
+                    $result->edges = array_map(
+                        function (Edge $edge) use ($checkAccess) {
+                            $edge->node = $checkAccess($edge->node) ? $edge->node : null;
+
+                            return $edge;
                         },
-                        $result
-                    )
-                );
-            } elseif ($result instanceof Connection) {
-                $result->edges = array_map(
-                    function (Edge $edge) use ($checkAccess) {
-                        $edge->node = $checkAccess($edge->node) ? $edge->node : null;
-
-                        return $edge;
-                    },
-                    $result->edges
-                );
-            } elseif (!empty($result) && !$checkAccess($result)) {
-                $result = null;
+                        $result->edges
+                    );
+                    break;
+
+                default:
+                    $checkAccess($result, true);
+                    break;
             }
 
             return $result;

diff --git a/Tests/Resolver/ConfigResolverTest.php b/Tests/Resolver/ConfigResolverTest.php
@@ -12,17 +12,21 @@
 namespace Overblog\GraphQLBundle\Tests\Resolver;
 
 use Overblog\GraphQLBundle\ExpressionLanguage\ExpressionLanguage;
+use Overblog\GraphQLBundle\Relay\Connection\Output\ConnectionBuilder;
 use Overblog\GraphQLBundle\Resolver\ConfigResolver;
+use Overblog\GraphQLBundle\Tests\DIContainerMockTrait;
+use Symfony\Component\ExpressionLanguage\Expression;
 
 class ConfigResolverTest extends \PHPUnit_Framework_TestCase
 {
+    use DIContainerMockTrait;
+
     /** @var  ConfigResolver */
-    private static $configResolver;
+    private $configResolver;
 
     public function setUp()
     {
-        $container = $this->getMockBuilder('Symfony\Component\DependencyInjection\Container')
-            ->getMock();
+        $container = $this->getDIContainerMock();
         $container
             ->method('get')
             ->will($this->returnValue(new \stdClass()));
@@ -51,7 +55,7 @@ public function setUp()
             ->method('resolve')
             ->will($this->returnValue(new \stdClass()));
 
-        self::$configResolver = new ConfigResolver(
+        $this->configResolver = new ConfigResolver(
             $typeResolver,
             $fieldResolver,
             $argResolver,
@@ -66,12 +70,12 @@ public function setUp()
      */
     public function testConfigNotArrayOrImplementArrayAccess()
     {
-        self::$configResolver->resolve('Not Array');
+        $this->configResolver->resolve('Not Array');
     }
 
     public function testResolveValues()
     {
-        $config = self::$configResolver->resolve(
+        $config = $this->configResolver->resolve(
             [
                 'values' => [
                     'test' => ['value' => 'my test value'],
@@ -91,4 +95,106 @@ public function testResolveValues()
 
         $this->assertEquals($expected, $config);
     }
+
+    /**
+     * @expectedException \Overblog\GraphQLBundle\Error\UserError
+     * @expectedExceptionMessage Access denied to this field
+     */
+    public function testResolveAccessAndWrapResolveCallbackWithScalarValueAndAccessDenied()
+    {
+        $callback = $this->invokeResolveAccessAndWrapResolveCallback(false);
+        $callback('toto');
+    }
+
+    /**
+     * @expectedException \Overblog\GraphQLBundle\Error\UserError
+     * @expectedExceptionMessage Access denied to this field
+     */
+    public function testResolveAccessAndWrapResolveCallbackWithScalarValueAndExpressionEvalThrowingException()
+    {
+        $callback = $this->invokeResolveAccessAndWrapResolveCallback('@=oups');
+        $callback('titi');
+    }
+
+    public function testResolveAccessAndWrapResolveCallbackWithScalarValueAndAccessDeniedGranted()
+    {
+        $callback = $this->invokeResolveAccessAndWrapResolveCallback(true);
+        $this->assertEquals('toto', $callback('toto'));
+    }
+
+    public function testResolveAccessAndWrapResolveCallbackWithArrayAndAccessDeniedToEveryItemStartingByTo()
+    {
+        $callback = $this->invokeResolveAccessAndWrapResolveCallback('@=not(object matches "/^to.*/i")');
+        $this->assertEquals(
+            [
+                'tata',
+                'titi',
+                'tata',
+            ],
+            $callback(
+                [
+                    'tata',
+                    'titi',
+                    'tata',
+                    'toto',
+                    'tota',
+                ]
+            )
+        );
+    }
+
+    public function testResolveAccessAndWrapResolveCallbackWithRelayConnectionAndAccessGrantedToEveryNodeStartingByTo()
+    {
+        $callback = $this->invokeResolveAccessAndWrapResolveCallback('@=object matches "/^to.*/i"');
+        $this->assertEquals(
+            ConnectionBuilder::connectionFromArray(['toto', 'toti', null, null, null]),
+            $callback(
+                ConnectionBuilder::connectionFromArray(['toto', 'toti', 'coco', 'foo', 'bar'])
+            )
+        );
+    }
+
+    /**
+     * @param bool|string   $hasAccess
+     * @param callable|null $callback
+     *
+     * @return callback
+     */
+    private function invokeResolveAccessAndWrapResolveCallback($hasAccess, callable $callback = null)
+    {
+        if (null === $callback) {
+            $callback = function ($value) {
+                return $value;
+            };
+        }
+
+        return $this->invokeMethod(
+            $this->configResolver,
+            'resolveAccessAndWrapResolveCallback',
+            [
+                $hasAccess,
+                $callback,
+            ]
+        );
+    }
+
+    /**
+     * Call protected/private method of a class.
+     *
+     * @see https://jtreminio.com/2013/03/unit-testing-tutorial-part-3-testing-protected-private-methods-coverage-reports-and-crap/
+     *
+     * @param object $object     Instantiated object that we will run method on.
+     * @param string $methodName Method name to call
+     * @param array  $parameters Array of parameters to pass into method.
+     *
+     * @return mixed Method return.
+     */
+    private function invokeMethod($object, $methodName, array $parameters = [])
+    {
+        $reflection = new \ReflectionClass(get_class($object));
+        $method = $reflection->getMethod($methodName);
+        $method->setAccessible(true);
+
+        return $method->invokeArgs($object, $parameters);
+    }
 }