From c9f96ed012e96535977d7f1cc3aac11c9ab8f4f5 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Mon, 23 May 2016 10:38:22 +0200 Subject: [PATCH] Mutation access denied throws error instead of a warning --- Resolver/AccessResolver.php | 20 ++++++++++++-------- Tests/Functional/Security/AccessTest.php | 16 +++++++--------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/Resolver/AccessResolver.php b/Resolver/AccessResolver.php index 94279f5d0..46cf1ead7 100644 --- a/Resolver/AccessResolver.php +++ b/Resolver/AccessResolver.php @@ -11,6 +11,7 @@ namespace Overblog\GraphQLBundle\Resolver; +use Overblog\GraphQLBundle\Error\UserError; use Overblog\GraphQLBundle\Error\UserWarning; use Overblog\GraphQLBundle\Relay\Connection\Output\Connection; use Overblog\GraphQLBundle\Relay\Connection\Output\Edge; @@ -21,7 +22,11 @@ public function resolve(callable $accessChecker, callable $resolveCallback, arra { // operation is mutation and is mutation field if ($isMutation) { - $result = $this->checkAccess($accessChecker, null, $resolveArgs, true) ? call_user_func_array($resolveCallback, $resolveArgs) : null; + if (!$this->hasAccess($accessChecker, null, $resolveArgs)) { + throw new UserError('Access denied to this field.'); + } + + $result = call_user_func_array($resolveCallback, $resolveArgs); } else { $result = $this->filterResultUsingAccess($accessChecker, $resolveCallback, $resolveArgs); } @@ -37,7 +42,7 @@ private function filterResultUsingAccess(callable $accessChecker, callable $reso case is_array($result): $result = array_map( function ($object) use ($accessChecker, $resolveArgs) { - return $this->checkAccess($accessChecker, $object, $resolveArgs) ? $object : null; + return $this->hasAccess($accessChecker, $object, $resolveArgs) ? $object : null; }, $result ); @@ -45,7 +50,7 @@ function ($object) use ($accessChecker, $resolveArgs) { case $result instanceof Connection: $result->edges = array_map( function (Edge $edge) use ($accessChecker, $resolveArgs) { - $edge->node = $this->checkAccess($accessChecker, $edge->node, $resolveArgs) ? $edge->node : null; + $edge->node = $this->hasAccess($accessChecker, $edge->node, $resolveArgs) ? $edge->node : null; return $edge; }, @@ -53,14 +58,16 @@ function (Edge $edge) use ($accessChecker, $resolveArgs) { ); break; default: - $this->checkAccess($accessChecker, $result, $resolveArgs, true); + if (!$this->hasAccess($accessChecker, $result, $resolveArgs)) { + throw new UserWarning('Access denied to this field.'); + } break; } return $result; } - private function checkAccess(callable $accessChecker, $object, array $resolveArgs = [], $throwException = false) + private function hasAccess(callable $accessChecker, $object, array $resolveArgs = []) { $resolveArgs[] = $object; @@ -69,9 +76,6 @@ private function checkAccess(callable $accessChecker, $object, array $resolveArg } catch (\Exception $e) { $access = false; } - if ($throwException && !$access) { - throw new UserWarning('Access denied to this field.'); - } return $access; } diff --git a/Tests/Functional/Security/AccessTest.php b/Tests/Functional/Security/AccessTest.php index 1a650a272..2a58d7781 100644 --- a/Tests/Functional/Security/AccessTest.php +++ b/Tests/Functional/Security/AccessTest.php @@ -228,15 +228,13 @@ public function testMutationNotAllowedUser() 'data' => [ 'simpleMutationWithThunkFields' => null, ], - 'extensions' => [ - 'warnings' => [ - [ - 'message' => 'Access denied to this field.', - 'locations' => [ - [ - 'line' => 2, - 'column' => 3, - ], + 'errors' => [ + [ + 'message' => 'Access denied to this field.', + 'locations' => [ + [ + 'line' => 2, + 'column' => 3, ], ], ],