Add injectArgs option to @can directive to pass along client defi…

…ned arguments to the policy check (#1043)
nuwave · Nov 14, 2019 · 09a0df9 · 09a0df9
1 parent b52e364
commit 09a0df9
Show file tree

Hide file tree

Showing 5 changed files with 177 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Add `syncWithoutDetaching` option for BelongsToMany and MorphToMany relationships https://github.com/nuwave/lighthouse/pull/1031
+- Add `injectArgs` option to `@can` directive to pass along client defined
+  arguments to the policy check https://github.com/nuwave/lighthouse/pull/1043
 
 ### Changed
 

diff --git a/docs/master/api-reference/directives.md b/docs/master/api-reference/directives.md
@@ -459,6 +459,9 @@ class PostPolicy
 ```graphql
 """
 Check a Laravel Policy to ensure the current user is authorized to access a field.
+
+When `injectArgs` and `args` are used together, the client given
+arguments will be passed before the static args.
 """
 directive @can(
   """
@@ -471,18 +474,26 @@ directive @can(
   instance against which the permissions should be checked.
   """
   find: String
-  
+
   """
-  Additional arguments that are passed to `Gate::check`. 
+  Pass along the client given input data as arguments to `Gate::check`. 
   """
-  args: [String!]
+  injectArgs: Boolean = false
+
+  """
+  Statically defined arguments that are passed to `Gate::check`.
+  
+  You may pass pass arbitrary GraphQL literals,
+  e.g.: [1, 2, 3] or { foo: "bar" }
+  """
+  args: Mixed
 ) on FIELD_DEFINITION
 ```
 
 ### Examples
 
 In `find` parameter you may specify an input argument which is used to find a specific model
-instance by primary key against which the permissions should be checked.
+instance by primary key against which the permissions should be checked:
 
 ```graphql
 type Query {
@@ -500,7 +511,7 @@ class PostPolicy
 }
 ```
 
-It also works with soft deleted models in combination with `@softDeletes` directive.
+It also works with soft deleted models in combination with `@softDeletes` directive:
 
 ```graphql
 type Query {
@@ -518,7 +529,7 @@ type Mutation {
 }
 ```
 
-You can pass additional arguments to the policy checks by specifying them as `args`.
+You can pass additional arguments to the policy checks by specifying them as `args`:
 
 ```graphql
 type Mutation {
@@ -527,6 +538,18 @@ type Mutation {
 }
 ```
 
+You can pass along the client given input data as arguments to the policy checks
+with the `injectArgs` argument:
+
+```graphql
+type Mutation {
+    createPost(input: PostInput): Post
+        @can(ability: "create", injectArgs: "true")
+}
+```
+
+Now you will have access to `PostInput` values in the policy. 
+
 Starting from Laravel 5.7, [authorization of guest users](https://laravel.com/docs/authorization#guest-users) is supported.
 Because of this, Lighthouse does **not** validate that the user is authenticated before passing it along to the policy.
 

diff --git a/src/Schema/Directives/CanDirective.php b/src/Schema/Directives/CanDirective.php
@@ -7,6 +7,7 @@
 use Illuminate\Contracts\Auth\Access\Gate;
 use Illuminate\Database\Eloquent\Model;
 use Nuwave\Lighthouse\Exceptions\AuthorizationException;
+use Nuwave\Lighthouse\Execution\Arguments\ArgumentSet;
 use Nuwave\Lighthouse\Schema\Values\FieldValue;
 use Nuwave\Lighthouse\SoftDeletes\TrashedDirective;
 use Nuwave\Lighthouse\Support\Contracts\DefinedDirective;
@@ -46,6 +47,9 @@ public static function definition(): string
         return /* @lang GraphQL */ <<<'SDL'
 """
 Check a Laravel Policy to ensure the current user is authorized to access a field.
+
+When `injectArgs` and `args` are used together, the client given
+arguments will be passed before the static args.
 """
 directive @can(
   """
@@ -58,11 +62,19 @@ public static function definition(): string
   instance against which the permissions should be checked.
   """
   find: String
-  
+
+  """
+  Pass along the client given input data as arguments to `Gate::check`. 
   """
-  Additional arguments that are passed to `Gate::check`. 
+  injectArgs: Boolean = false
+
+  """
+  Statically defined arguments that are passed to `Gate::check`.
+  
+  You may pass pass arbitrary GraphQL literals,
+  e.g.: [1, 2, 3] or { foo: "bar" }
   """
-  args: [String!]
+  args: Mixed
 ) on FIELD_DEFINITION
 SDL;
     }
@@ -81,58 +93,66 @@ public function handleField(FieldValue $fieldValue, Closure $next): FieldValue
         return $next(
             $fieldValue->setResolver(
                 function ($root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo) use ($previousResolver) {
-                    if ($find = $this->directiveArgValue('find')) {
-                        $modelOrModels = $resolveInfo
-                            ->argumentSet
-                            ->enhanceBuilder(
-                                $this->getModelClass()::query(),
-                                [],
-                                function (Directive $directive): bool {
-                                    return $directive instanceof TrashedDirective;
-                                }
-                            )
-                            ->findOrFail($args[$find]);
-
-                        if ($modelOrModels instanceof Model) {
-                            $modelOrModels = [$modelOrModels];
-                        }
-
-                        /** @var \Illuminate\Database\Eloquent\Model $model */
-                        foreach ($modelOrModels as $model) {
-                            $this->authorize($context->user(), $model);
-                        }
-                    } else {
-                        $this->authorize($context->user(), $this->getModelClass());
+                    $gate = $this->gate->forUser($context->user());
+                    $ability = $this->directiveArgValue('ability');
+                    $checkArguments = $this->buildCheckArguments($args);
+
+                    foreach ($this->modelsToCheck($resolveInfo->argumentSet, $args) as $model) {
+                        $this->authorize($gate, $ability, $model, $checkArguments);
                     }
 
-                    return call_user_func_array($previousResolver, func_get_args());
+                    return $previousResolver($root, $args, $context, $resolveInfo);
                 }
             )
         );
     }
 
     /**
-     * @param  \Illuminate\Contracts\Auth\Authenticatable|null  $user
+     * @param  \Nuwave\Lighthouse\Execution\Arguments\ArgumentSet  $argumentSet
+     * @param  array  $args
+     * @return iterable<Model|string>
+     *
+     * @throws \Nuwave\Lighthouse\Exceptions\DefinitionException
+     */
+    protected function modelsToCheck(ArgumentSet $argumentSet, array $args): iterable
+    {
+        if ($find = $this->directiveArgValue('find')) {
+            $modelOrModels = $argumentSet
+                ->enhanceBuilder(
+                    $this->getModelClass()::query(),
+                    [],
+                    function (Directive $directive): bool {
+                        return $directive instanceof TrashedDirective;
+                    }
+                )
+                ->findOrFail($args[$find]);
+
+            if ($modelOrModels instanceof Model) {
+                $modelOrModels = [$modelOrModels];
+            }
+
+            return $modelOrModels;
+        }
+
+        return [$this->getModelClass()];
+    }
+
+    /**
+     * @param  \Illuminate\Contracts\Auth\Access\Gate  $gate
+     * @param  string|string[]  $ability
      * @param  string|\Illuminate\Database\Eloquent\Model  $model
+     * @param  array  $arguments
      * @return void
      *
      * @throws \Nuwave\Lighthouse\Exceptions\AuthorizationException
      */
-    protected function authorize($user, $model): void
+    protected function authorize(Gate $gate, $ability, $model, array $arguments): void
     {
         // The signature of the second argument `$arguments` of `Gate::check`
         // should be [modelClassName, additionalArg, additionalArg...]
-        $arguments = $this->getAdditionalArguments();
         array_unshift($arguments, $model);
 
-        $can = $this->gate
-            ->forUser($user)
-            ->check(
-                $this->directiveArgValue('ability'),
-                $arguments
-            );
-
-        if (! $can) {
+        if (! $gate->check($ability, $arguments)) {
             throw new AuthorizationException(
                 "You are not authorized to access {$this->definitionNode->name->value}"
             );
@@ -142,10 +162,22 @@ protected function authorize($user, $model): void
     /**
      * Additional arguments that are passed to `Gate::check`.
      *
+     * @param  array  $args
      * @return mixed[]
      */
-    protected function getAdditionalArguments(): array
+    protected function buildCheckArguments(array $args): array
     {
-        return (array) $this->directiveArgValue('args');
+        $checkArguments = [];
+
+        // The injected args come before the static args
+        if ($this->directiveArgValue('injectArgs')) {
+            $checkArguments [] = $args;
+        }
+
+        if ($this->directiveHasArgument('args')) {
+            $checkArguments [] = $this->directiveArgValue('args');
+        }
+
+        return $checkArguments;
     }
 }
diff --git a/tests/Unit/Schema/Directives/CanDirectiveTest.php b/tests/Unit/Schema/Directives/CanDirectiveTest.php
@@ -156,6 +156,70 @@ public function testProcessesTheArgsArgument(): void
         ')->assertErrorCategory(AuthorizationException::CATEGORY);
     }
 
+    public function testInjectArgsPassesClientArgumentToPolicy(): void
+    {
+        $this->be(new User);
+        $this->schema = /* @lang GraphQL */'
+        type Query {
+            user(foo: String): User!
+                @can(ability:"injectArgs", injectArgs: true)
+                @field(resolver: "'.$this->qualifyTestResolver('resolveUser').'")
+        }
+        
+        type User {
+            name: String
+        }
+        ';
+
+        $this->graphQL(/* @lang GraphQL */ '
+        {
+            user(foo: "bar"){
+                name
+            }
+        }
+        ')->assertJson([
+            'data' => [
+                'user' => [
+                    'name' => 'foo',
+                ],
+            ],
+        ]);
+    }
+
+    public function testInjectedArgsAndStaticArgs(): void
+    {
+        $this->be(new User);
+        $this->schema = /* @lang GraphQL */'
+        type Query {
+            user(foo: String): User!
+                @can(
+                    ability: "argsWithInjectedArgs"
+                    args: { foo: "static" }
+                    injectArgs: true
+                )
+                @field(resolver: "'.$this->qualifyTestResolver('resolveUser').'")
+        }
+        
+        type User {
+            name: String
+        }
+        ';
+
+        $this->graphQL(/* @lang GraphQL */ '
+        {
+            user(foo: "dynamic"){
+                name
+            }
+        }
+        ')->assertJson([
+            'data' => [
+                'user' => [
+                    'name' => 'foo',
+                ],
+            ],
+        ]);
+    }
+
     public function resolveUser(): User
     {
         $user = new User;

diff --git a/tests/Utils/Policies/UserPolicy.php b/tests/Utils/Policies/UserPolicy.php
@@ -32,4 +32,15 @@ public function dependingOnArg($viewer, bool $pass): bool
     {
         return $pass;
     }
+
+    public function injectArgs($user, array $injectedArgs): bool
+    {
+        return $injectedArgs === ['foo' => 'bar'];
+    }
+
+    public function argsWithInjectedArgs($user, array $injectedArgs, array $staticArgs): bool
+    {
+        return $injectedArgs === ['foo' => 'dynamic']
+            && $staticArgs === ['foo' => 'static'];
+    }
 }