Skip to content

Fix Access resolving when field value is false or 0 or empty string #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 10, 2016
Merged

Fix Access resolving when field value is false or 0 or empty string #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 28 additions & 22 deletions Resolver/ConfigResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down
118 changes: 112 additions & 6 deletions Tests/Resolver/ConfigResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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,
Expand All @@ -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'],
Expand All @@ -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);
}
}