From c3f17bddbe733843834308a6740a8c6454dd555c Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Fri, 7 Dec 2018 08:17:16 +0100 Subject: [PATCH 1/2] Add travis php 7.3 build --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index ea48ee5fd..dba2c72cc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,9 +42,9 @@ jobs: env: SYMFONY_VERSION=3.3.* PHPUNIT_VERSION=^6.0 - php: 7.2 env: SYMFONY_VERSION=3.4.* - - php: 7.2 + - php: 7.3 env: SYMFONY_VERSION=4.1.* - - php: 7.2 + - php: 7.3 env: SYMFONY_VERSION=4.2.* - php: nightly env: COMPOSER_UPDATE_FLAGS=--ignore-platform-reqs From ecfac7df4fd3b7e429f4e3886bc060dedce0b33d Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Tue, 11 Dec 2018 08:03:56 +0100 Subject: [PATCH 2/2] Allow promise for AccessResolver::checkAccessForStrictMode --- src/Resolver/AccessResolver.php | 61 ++++++++++++++----- .../App/Resolver/ConnectionResolver.php | 5 ++ .../config/access/mapping/access.types.yml | 4 ++ .../App/config/connection/services.yml | 1 + tests/Functional/Security/AccessTest.php | 40 +++++++++++- 5 files changed, 94 insertions(+), 17 deletions(-) diff --git a/src/Resolver/AccessResolver.php b/src/Resolver/AccessResolver.php index 5be94d965..ce702e68d 100644 --- a/src/Resolver/AccessResolver.php +++ b/src/Resolver/AccessResolver.php @@ -28,22 +28,18 @@ public function resolve(callable $accessChecker, callable $resolveCallback, arra return $this->checkAccessForStrictMode($accessChecker, $resolveCallback, $resolveArgs); } - $result = \call_user_func_array($resolveCallback, $resolveArgs); + $resultOrPromise = \call_user_func_array($resolveCallback, $resolveArgs); - if ($result instanceof Promise) { - $result = $result->adoptedPromise; - } - - if ($this->promiseAdapter->isThenable($result) || $result instanceof SyncPromise) { - return $this->promiseAdapter->then( - new Promise($result, $this->promiseAdapter), + if ($this->isThenable($resultOrPromise)) { + return $this->createPromise( + $resultOrPromise, function ($result) use ($accessChecker, $resolveArgs) { return $this->processFilter($result, $accessChecker, $resolveArgs); } ); } - return $this->processFilter($result, $accessChecker, $resolveArgs); + return $this->processFilter($resultOrPromise, $accessChecker, $resolveArgs); } private static function isMutationRootField(ResolveInfo $info) @@ -53,12 +49,21 @@ private static function isMutationRootField(ResolveInfo $info) private function checkAccessForStrictMode(callable $accessChecker, callable $resolveCallback, array $resolveArgs = []) { - if (!$this->hasAccess($accessChecker, $resolveArgs)) { - $exceptionClassName = self::isMutationRootField($resolveArgs[3]) ? UserError::class : UserWarning::class; - throw new $exceptionClassName('Access denied to this field.'); - } + $promiseOrHasAccess = $this->hasAccess($accessChecker, $resolveArgs); + $callback = function ($hasAccess) use ($resolveArgs, $resolveCallback) { + if (!$hasAccess) { + $exceptionClassName = self::isMutationRootField($resolveArgs[3]) ? UserError::class : UserWarning::class; + throw new $exceptionClassName('Access denied to this field.'); + } + + return \call_user_func_array($resolveCallback, $resolveArgs); + }; - return \call_user_func_array($resolveCallback, $resolveArgs); + if ($this->isThenable($promiseOrHasAccess)) { + return $this->createPromise($promiseOrHasAccess, $callback); + } else { + return $callback($promiseOrHasAccess); + } } private function processFilter($result, $accessChecker, $resolveArgs) @@ -89,9 +94,33 @@ function (Edge $edge) use ($accessChecker, $resolveArgs) { private function hasAccess(callable $accessChecker, array $resolveArgs = [], $object = null) { $resolveArgs[] = $object; - $access = (bool) \call_user_func_array($accessChecker, $resolveArgs); + $accessOrPromise = \call_user_func_array($accessChecker, $resolveArgs); - return $access; + return $accessOrPromise; + } + + private function isThenable($object) + { + $object = $this->extractAdoptedPromise($object); + + return $this->promiseAdapter->isThenable($object) || $object instanceof SyncPromise; + } + + private function extractAdoptedPromise($object) + { + if ($object instanceof Promise) { + $object = $object->adoptedPromise; + } + + return $object; + } + + private function createPromise($promise, callable $onFulfilled = null) + { + return $this->promiseAdapter->then( + new Promise($this->extractAdoptedPromise($promise), $this->promiseAdapter), + $onFulfilled + ); } /** diff --git a/tests/Functional/App/Resolver/ConnectionResolver.php b/tests/Functional/App/Resolver/ConnectionResolver.php index 84c2c0ca8..8418229c1 100644 --- a/tests/Functional/App/Resolver/ConnectionResolver.php +++ b/tests/Functional/App/Resolver/ConnectionResolver.php @@ -80,4 +80,9 @@ public function resolveQuery() return $this->allUsers[0]; } + + public function resolvePromiseFullFilled($value) + { + return $this->promiseAdapter->createFulfilled($value); + } } diff --git a/tests/Functional/App/config/access/mapping/access.types.yml b/tests/Functional/App/config/access/mapping/access.types.yml index 30e0589c4..2c4b2b206 100644 --- a/tests/Functional/App/config/access/mapping/access.types.yml +++ b/tests/Functional/App/config/access/mapping/access.types.yml @@ -38,6 +38,10 @@ User: access: "@=hasRole('ROLE_ADMIN')" resolve: ['ROLE_USER'] isEnabled: + # access as a promise + access: "@=resolver('promise', [args['hasAccess']])" + args: + hasAccess: {type: Boolean!} type: Boolean resolve: true friends: diff --git a/tests/Functional/App/config/connection/services.yml b/tests/Functional/App/config/connection/services.yml index 2e8809ac0..4af041356 100644 --- a/tests/Functional/App/config/connection/services.yml +++ b/tests/Functional/App/config/connection/services.yml @@ -8,3 +8,4 @@ services: - { name: "overblog_graphql.resolver", alias: "node", method: "resolveNode" } - { name: "overblog_graphql.resolver", alias: "query", method: "resolveQuery" } - { name: "overblog_graphql.resolver", alias: "connection", method: "resolveConnection" } + - { name: "overblog_graphql.resolver", alias: "promise", method: "resolvePromiseFullFilled" } diff --git a/tests/Functional/Security/AccessTest.php b/tests/Functional/Security/AccessTest.php index f484d18cc..202a05e4e 100644 --- a/tests/Functional/Security/AccessTest.php +++ b/tests/Functional/Security/AccessTest.php @@ -15,7 +15,7 @@ class AccessTest extends TestCase private $userRolesQuery = 'query { user { roles } }'; - private $userIsEnabledQuery = 'query { user { isEnabled } }'; + private $userIsEnabledQuery = 'query ($hasAccess: Boolean = true) { user { isEnabled(hasAccess: $hasAccess) } }'; private $userFriendsQuery = <<<'EOF' query { @@ -66,6 +66,44 @@ public function testCustomClassLoaderNotRegister() $this->assertResponse($this->userNameQuery, [], static::ANONYMOUS_USER, 'access'); } + public function testNotAuthenticatedUserAccessAsPromisedFulfilledTrue() + { + $this->assertResponse( + $this->userIsEnabledQuery, + ['data' => ['user' => ['isEnabled' => true]]], + static::ANONYMOUS_USER, + 'access' + ); + } + + public function testNotAuthenticatedUserAccessAsPromisedFulfilledFalse() + { + $this->assertResponse( + $this->userIsEnabledQuery, + [ + 'data' => [ + 'user' => [ + 'isEnabled' => null, + ], + ], + 'extensions' => [ + 'warnings' => [ + [ + 'message' => 'Access denied to this field.', + 'category' => 'user', + 'locations' => [['line' => 1, 'column' => 45]], + 'path' => ['user', 'isEnabled'], + ], + ], + ], + ], + static::ANONYMOUS_USER, + 'access', + '', + ['hasAccess' => false] + ); + } + public function testNotAuthenticatedUserAccessToUserName() { $expected = [