diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f732a1e87..14babfba8d 100644 --- 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 index 1ec30095d5..e86c2429eb 100644 --- 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 index ca708a82e2..5f1ff2b909 100644 --- 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 + * + * @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 index 0144d43804..8a8b4c7119 100644 --- 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 index 17a8474abb..5de10e3d7e 100644 --- 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']; + } }