Merge pull request #727 from torunar/master

Pass query/mutation context into the field privacy detection for better access control
rebing · Apr 3, 2021 · ab6bd09 · ab6bd09
2 parents 9f650fc + 777ba94
commit ab6bd09
Show file tree

Hide file tree

Showing 14 changed files with 214 additions and 85 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,17 @@ CHANGELOG
 [Next release](https://github.com/rebing/graphql-laravel/compare/6.5.0...master)
 --------------
 
+## Breaking changes
+- Signtature of `\Rebing\GraphQL\Support\Privacy::validate` changed, now it accepts both query/mutation arguments and the query/mutation context.
+  Update your existing privacy policies this way:
+  ```diff
+  -public function validate(array $queryArgs): bool
+  +public function validate(array $queryArgs, $queryContext = null): bool
+  ```
+
+### Added
+- Ability to pass query/mutation context to the field privacy handler (both closure and class) [\#727 / torunar](https://github.com/rebing/graphql-laravel/pull/727)
+
 2021-04-03, 6.5.0
 -----------------
 ### Fixed
@@ -49,7 +60,7 @@ Same as 6.1.0-rc1!
   Be sure to read up on breaking changes in graphql-php => https://github.com/webonyx/graphql-php/releases/tag/v14.0.0
 - Remove support for Laravel < 6.0 [\#651 / mfn](https://github.com/rebing/graphql-laravel/pull/651)
   This also bumps the minimum required version to PHP 7.2
-  
+
 ### Added
 - Support for Laravel 8 [\#672 / mfn](https://github.com/rebing/graphql-laravel/pull/672)
 

diff --git a/README.md b/README.md
@@ -1073,7 +1073,7 @@ class UserType extends GraphQLType
             'email' => [
                 'type'          => Type::string(),
                 'description'   => 'The email of user',
-                'privacy'       => function(array $args): bool {
+                'privacy'       => function(array $args, $ctx): bool {
                     return $args['id'] == Auth::id();
                 }
             ]
@@ -1093,9 +1093,9 @@ use Rebing\GraphQL\Support\Privacy;
 
 class MePrivacy extends Privacy
 {
-    public function validate(array $queryArgs): bool
+    public function validate(array $queryArgs, $queryContext = null): bool
     {
-        return $args['id'] == Auth::id();
+        return $queryArgs['id'] == Auth::id();
     }
 }
 ```

diff --git a/example/Privacy/MePrivacy.php b/example/Privacy/MePrivacy.php
@@ -7,8 +7,8 @@
 
 class MePrivacy extends Privacy
 {
-    public function validate(array $args): bool
+    public function validate(array $queryArgs, $queryContext = null): bool
     {
-        return $args['id'] == Auth::id();
+        return $queryArgs['id'] == Auth::id();
     }
 }
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -266,7 +266,7 @@ parameters:
 			path: src/Support/PaginationType.php
 
 		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\Privacy\\:\\:validate\\(\\) has parameter \\$queryArgs with no value type specified in iterable type array\\.$#"
+			message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\Privacy\\:\\:fire\\(\\) has parameter \\$args with no typehint specified\\.$#"
 			count: 1
 			path: src/Support/Privacy.php
 
@@ -365,16 +365,6 @@ parameters:
 			count: 1
 			path: src/Support/SelectFields.php
 
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\SelectFields\\:\\:validateField\\(\\) has parameter \\$queryArgs with no value type specified in iterable type array\\.$#"
-			count: 1
-			path: src/Support/SelectFields.php
-
-		-
-			message: "#^Parameter \\#1 \\$callback of function call_user_func expects callable\\(\\)\\: mixed, array\\(\\*NEVER\\*, 'fire'\\) given\\.$#"
-			count: 1
-			path: src/Support/SelectFields.php
-
 		-
 			message: "#^Method Rebing\\\\GraphQL\\\\Support\\\\SelectFields\\:\\:isQueryable\\(\\) has parameter \\$fieldObject with no value type specified in iterable type array\\.$#"
 			count: 1
@@ -1170,21 +1160,6 @@ parameters:
 			count: 1
 			path: tests/Database/SelectFields/ValidateFieldTests/PostType.php
 
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Database\\\\SelectFields\\\\ValidateFieldTests\\\\PrivacyAllowed\\:\\:validate\\(\\) has parameter \\$queryArgs with no value type specified in iterable type array\\.$#"
-			count: 1
-			path: tests/Database/SelectFields/ValidateFieldTests/PrivacyAllowed.php
-
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Database\\\\SelectFields\\\\ValidateFieldTests\\\\PrivacyArgs\\:\\:validate\\(\\) has parameter \\$queryArgs with no value type specified in iterable type array\\.$#"
-			count: 1
-			path: tests/Database/SelectFields/ValidateFieldTests/PrivacyArgs.php
-
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Database\\\\SelectFields\\\\ValidateFieldTests\\\\PrivacyDenied\\:\\:validate\\(\\) has parameter \\$queryArgs with no value type specified in iterable type array\\.$#"
-			count: 1
-			path: tests/Database/SelectFields/ValidateFieldTests/PrivacyDenied.php
-
 		-
 			message: "#^Property Rebing\\\\GraphQL\\\\Tests\\\\Database\\\\SelectFields\\\\ValidateFieldTests\\\\ValidateFieldsQuery\\:\\:\\$attributes has no typehint specified\\.$#"
 			count: 1
@@ -1215,6 +1190,26 @@ parameters:
 			count: 1
 			path: tests/Support/Objects/CustomExampleType.php
 
+		-
+			message: "#^Property Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:\\$attributes has no typehint specified\\.$#"
+			count: 1
+			path: tests/Support/Objects/CustomExamplesQuery.php
+
+		-
+			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:resolve\\(\\) has no return typehint specified\\.$#"
+			count: 1
+			path: tests/Support/Objects/CustomExamplesQuery.php
+
+		-
+			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:resolve\\(\\) has parameter \\$args with no typehint specified\\.$#"
+			count: 1
+			path: tests/Support/Objects/CustomExamplesQuery.php
+
+		-
+			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:resolve\\(\\) has parameter \\$root with no typehint specified\\.$#"
+			count: 1
+			path: tests/Support/Objects/CustomExamplesQuery.php
+
 		-
 			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\ErrorFormatter\\:\\:formatError\\(\\) return type has no value type specified in iterable type array\\.$#"
 			count: 1
@@ -1500,26 +1495,6 @@ parameters:
 			count: 1
 			path: tests/Support/Objects/ExamplesQuery.php
 
-		-
-			message: "#^Property Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:\\$attributes has no typehint specified\\.$#"
-			count: 1
-			path: tests/Support/Objects/CustomExamplesQuery.php
-
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:resolve\\(\\) has no return typehint specified\\.$#"
-			count: 1
-			path: tests/Support/Objects/CustomExamplesQuery.php
-
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:resolve\\(\\) has parameter \\$args with no typehint specified\\.$#"
-			count: 1
-			path: tests/Support/Objects/CustomExamplesQuery.php
-
-		-
-			message: "#^Method Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Objects\\\\CustomExamplesQuery\\:\\:resolve\\(\\) has parameter \\$root with no typehint specified\\.$#"
-			count: 1
-			path: tests/Support/Objects/CustomExamplesQuery.php
-
 		-
 			message: "#^Property Rebing\\\\GraphQL\\\\Tests\\\\Support\\\\Queries\\\\PostNonNullWithSelectFieldsAndModelQuery\\:\\:\\$attributes has no typehint specified\\.$#"
 			count: 1

diff --git a/src/Support/Privacy.php b/src/Support/Privacy.php
@@ -6,15 +6,17 @@
 
 abstract class Privacy
 {
-    public function fire(): bool
+    public function fire(...$args): bool
     {
-        return $this->validate(func_get_args()[0]);
+        return $this->validate(...$args);
     }
 
     /**
-     * @param  array  $queryArgs  Arguments given with the query/mutation
+     * @param array<string, mixed> $queryArgs    Arguments given with the query/mutation
+     * @param mixed                $queryContext Context of the query/mutation
+     *
      * @return bool Return `true` to allow access to the field in question,
      *   `false otherwise
      */
-    abstract public function validate(array $queryArgs): bool;
+    abstract public function validate(array $queryArgs, $queryContext = null): bool;
 }
diff --git a/src/Support/SelectFields.php b/src/Support/SelectFields.php
@@ -173,7 +173,7 @@ protected static function handleFields(
             }
 
             // First check if the field is even accessible
-            $canSelect = static::validateField($fieldObject, $queryArgs);
+            $canSelect = static::validateField($fieldObject, $queryArgs, $ctx);
             if ($canSelect === true) {
                 // Add a query, if it exists
                 $customQuery = $fieldObject->config['query'] ?? null;
@@ -300,13 +300,15 @@ protected static function addFieldToSelect($field, array &$select, ?string $pare
     /**
      * Check the privacy status, if it's given.
      *
-     * @param  FieldDefinition  $fieldObject
-     * @param  array  $queryArgs  Arguments given with the query/mutation
+     * @param FieldDefinition       $fieldObject Validated field
+     * @param array<string, mixed>  $queryArgs   Arguments given with the query/mutation
+     * @param mixed                 $ctx         Query/mutation context
+     *
      * @return bool|null `true`  if selectable
      *                   `false` if not selectable, but allowed
      *                   `null`  if not allowed
      */
-    protected static function validateField(FieldDefinition $fieldObject, array $queryArgs): ?bool
+    protected static function validateField(FieldDefinition $fieldObject, array $queryArgs, $ctx): ?bool
     {
         $selectable = true;
 
@@ -321,16 +323,17 @@ protected static function validateField(FieldDefinition $fieldObject, array $que
             switch ($privacyClass) {
                 // If privacy given as a closure
                 case is_callable($privacyClass):
-                    if (false === call_user_func($privacyClass, $queryArgs)) {
+                    if (false === call_user_func($privacyClass, $queryArgs, $ctx)) {
                         $selectable = null;
                     }
 
                     break;
 
                 // If Privacy class given
                 case is_string($privacyClass):
+                    /** @var \Rebing\GraphQL\Support\Privacy $instance */
                     $instance = app($privacyClass);
-                    if (false === call_user_func([$instance, 'fire'], $queryArgs)) {
+                    if (false === call_user_func([$instance, 'fire'], $queryArgs, $ctx)) {
                         $selectable = null;
                     }
 

diff --git a/tests/Database/SelectFields/ValidateFieldTests/PostType.php b/tests/Database/SelectFields/ValidateFieldTests/PostType.php
@@ -104,6 +104,24 @@ public function fields(): array
                 'type' => Type::string(),
                 'privacy' => true,
             ],
+            'title_privacy_closure_query_context' => [
+                'alias' => 'title',
+                'type' => Type::string(),
+                'privacy' => static function (array $queryArgs, $queryContext): bool {
+                    $expectedQueryContext = [
+                        'arg_from_context_true' => true,
+                        'arg_from_context_false' => false,
+                    ];
+                    Assert::assertSame($expectedQueryContext, $queryContext);
+
+                    return true;
+                },
+            ],
+            'title_privacy_class_query_context' => [
+                'alias' => 'title',
+                'type' => Type::string(),
+                'privacy' => PrivacyQueryContext::class,
+            ],
         ];
     }
 }
diff --git a/tests/Database/SelectFields/ValidateFieldTests/PrivacyAllowed.php b/tests/Database/SelectFields/ValidateFieldTests/PrivacyAllowed.php
@@ -9,11 +9,9 @@
 class PrivacyAllowed extends Privacy
 {
     /**
-     * @param  array  $queryArgs  Arguments given with the query/mutation
-     * @return bool Return `true` to allow access to the field in question,
-     *   `false otherwise
+     * @inheritDoc
      */
-    public function validate(array $queryArgs): bool
+    public function validate(array $queryArgs, $queryContext = null): bool
     {
         return true;
     }

diff --git a/tests/Database/SelectFields/ValidateFieldTests/PrivacyArgs.php b/tests/Database/SelectFields/ValidateFieldTests/PrivacyArgs.php
@@ -10,11 +10,9 @@
 class PrivacyArgs extends Privacy
 {
     /**
-     * @param  array  $queryArgs  Arguments given with the query/mutation
-     * @return bool Return `true` to allow access to the field in question,
-     *   `false otherwise
+     * @inheritDoc
      */
-    public function validate(array $queryArgs): bool
+    public function validate(array $queryArgs, $queryContext = null): bool
     {
         $expectedQueryArgs = [
             'arg_from_query' => true,

diff --git a/tests/Database/SelectFields/ValidateFieldTests/PrivacyDenied.php b/tests/Database/SelectFields/ValidateFieldTests/PrivacyDenied.php
@@ -9,11 +9,9 @@
 class PrivacyDenied extends Privacy
 {
     /**
-     * @param  array  $queryArgs Arguments given with the query/mutation
-     * @return bool Return `true` to allow access to the field in question,
-     *   `false otherwise
+     * @inheritDoc
      */
-    public function validate(array $queryArgs): bool
+    public function validate(array $queryArgs, $queryContext = null): bool
     {
         return false;
     }

diff --git a/tests/Database/SelectFields/ValidateFieldTests/PrivacyQueryContext.php b/tests/Database/SelectFields/ValidateFieldTests/PrivacyQueryContext.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rebing\GraphQL\Tests\Database\SelectFields\ValidateFieldTests;
+
+use PHPUnit\Framework\Assert;
+use Rebing\GraphQL\Support\Privacy;
+
+class PrivacyQueryContext extends Privacy
+{
+    /**
+     * @inheritDoc
+     */
+    public function validate(array $queryArgs, $queryContext = null): bool
+    {
+        $expectedQueryContext = [
+            'arg_from_context_true' => true,
+            'arg_from_context_false' => false,
+        ];
+        Assert::assertSame($expectedQueryContext, $queryContext);
+
+        return true;
+    }
+}