From c53459369ff857d07d5323d256997513a5c67fb5 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 10 Mar 2016 12:16:39 +0100 Subject: [PATCH] Fix Access resolving when field value is false or 0 or empty string --- README.md | 2 +- Resolver/ConfigResolver.php | 50 ++++++----- Tests/Resolver/ConfigResolverTest.php | 118 ++++++++++++++++++++++++-- 3 files changed, 141 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 285c23392..a43c4a256 100644 --- 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 index 37f93bc45..c8aeab141 100644 --- 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 index 891e94af8..909773bbd 100644 --- 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); + } }