Skip to content

Don't conceal errors in resolvers for canFind, canModel and canQuery directives #2686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions src/Auth/BaseCanDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Nuwave\Lighthouse\Auth;

use Illuminate\Contracts\Auth\Access\Gate;
use Nuwave\Lighthouse\Auth\Contracts\ResolvesDuringAuthorization;
use Nuwave\Lighthouse\Exceptions\AuthorizationException;
use Nuwave\Lighthouse\Execution\ResolveInfo;
use Nuwave\Lighthouse\Schema\Directives\BaseDirective;
Expand Down Expand Up @@ -86,9 +87,18 @@ public function handleField(FieldValue $fieldValue): void
$gate = $this->gate->forUser($context->user());
$checkArguments = $this->buildCheckArguments($args);
$authorizeModel = fn (mixed $model) => $this->authorizeModel($gate, $ability, $model, $checkArguments);
$hasResolved = false;
$trackedResolver = function () use (&$hasResolved, $resolver) {
$hasResolved = true;

return $resolver(...func_get_args());
};

try {
return $this->authorizeRequest($root, $args, $context, $resolveInfo, $resolver, $authorizeModel);
$resolved = $this->authorizeRequest($root, $args, $context, $resolveInfo, $trackedResolver, $authorizeModel);
if ($hasResolved) {
return $resolved;
}
} catch (\Throwable $throwable) {
$action = $this->directiveArgValue('action');
if ($action === 'EXCEPTION_NOT_AUTHORIZED') {
Expand All @@ -101,11 +111,18 @@ public function handleField(FieldValue $fieldValue): void

throw $throwable;
}

// Try to resolve the field outside the authorization try-catch block to avoid catching resolver exceptions.
return $resolver($root, $args, $context, $resolveInfo);
});
}

/**
* Authorizes request and resolves the field.
* Authorizes request and optionally resolves the field.
*
* This method is called inside the authorization try-catch block.
* Resolving the field here is optional, it will be done outside the try-catch block if not resolved here.
* Resolving inside this method means any exceptions thrown by the resolver will be caught and handled according to the `action` argument.
*
* @phpstan-import-type Resolver from \Nuwave\Lighthouse\Schema\Values\FieldValue as Resolver
*
Expand Down
2 changes: 1 addition & 1 deletion src/Auth/CanFindDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ protected function authorizeRequest(mixed $root, array $args, GraphQLContext $co
$authorize($model);
}

return $resolver($root, $args, $context, $resolveInfo);
return null;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Auth/CanModelDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ protected function authorizeRequest(mixed $root, array $args, GraphQLContext $co
{
$authorize($this->getModelClass());

return $resolver($root, $args, $context, $resolveInfo);
return null;
}
}
2 changes: 1 addition & 1 deletion src/Auth/CanQueryDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ protected function authorizeRequest(mixed $root, array $args, GraphQLContext $co
$authorize($model);
}

return $resolver($root, $args, $context, $resolveInfo);
return null;
}
}
2 changes: 1 addition & 1 deletion src/Auth/CanRootDirective.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ protected function authorizeRequest(mixed $root, array $args, GraphQLContext $co
{
$authorize($root);

return $resolver($root, $args, $context, $resolveInfo);
return null;
}
}
32 changes: 32 additions & 0 deletions tests/Integration/Auth/CanFindDirectiveDBTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Tests\Utils\Models\Post;
use Tests\Utils\Models\Task;
use Tests\Utils\Models\User;
use Tests\Utils\Mutations\ThrowWhenInvoked;
use Tests\Utils\Policies\UserPolicy;

final class CanFindDirectiveDBTest extends DBTestCase
Expand Down Expand Up @@ -210,6 +211,37 @@ public function testFailsToFindSpecificModelConcealException(): void
]);
}

public function testDoesntConcealResolverException(): void
{
$admin = new User();
$admin->name = UserPolicy::ADMIN;
$this->be($admin);

$user = factory(User::class)->create();
assert($user instanceof User);

$this->schema = /** @lang GraphQL */ '
type Mutation {
throwWhenInvoked(id: ID!): User
@canFind(ability: "view", find: "id", action: EXCEPTION_NOT_AUTHORIZED)
}

type User {
name: String!
}
' . self::PLACEHOLDER_QUERY;

$this->graphQL(/** @lang GraphQL */ '
mutation ($id: ID!) {
throwWhenInvoked(id: $id) {
name
}
}
', [
'id' => $user->getKey(),
])->assertGraphQLErrorMessage(ThrowWhenInvoked::ERROR_MESSAGE);
}

public function testFailsToFindSpecificModelWithFindOrFailFalse(): void
{
$user = new User();
Expand Down
37 changes: 37 additions & 0 deletions tests/Integration/Auth/CanModelDirectiveDBTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php declare(strict_types=1);

namespace Integration\Auth;

use Tests\DBTestCase;
use Tests\Utils\Models\User;
use Tests\Utils\Mutations\ThrowWhenInvoked;
use Tests\Utils\Policies\UserPolicy;

final class CanModelDirectiveDBTest extends DBTestCase
{
public function testDoesntConcealResolverException(): void
{
$admin = new User();
$admin->name = UserPolicy::ADMIN;
$this->be($admin);

$this->schema = /** @lang GraphQL */ '
type Mutation {
throwWhenInvoked: Task
@canModel(ability: "adminOnly", action: EXCEPTION_NOT_AUTHORIZED)
}

type Task {
name: String!
}
' . self::PLACEHOLDER_QUERY;

$this->graphQL(/** @lang GraphQL */ '
mutation {
throwWhenInvoked {
name
}
}
')->assertGraphQLErrorMessage(ThrowWhenInvoked::ERROR_MESSAGE);
}
}
32 changes: 32 additions & 0 deletions tests/Integration/Auth/CanQueryDirectiveDBTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Tests\Utils\Models\Post;
use Tests\Utils\Models\Task;
use Tests\Utils\Models\User;
use Tests\Utils\Mutations\ThrowWhenInvoked;
use Tests\Utils\Policies\UserPolicy;

final class CanQueryDirectiveDBTest extends DBTestCase
Expand Down Expand Up @@ -83,6 +84,37 @@ public function testFailsToFindSpecificModelWithQuery(): void
]);
}

public function testDoesntConcealResolverException(): void
{
$admin = new User();
$admin->name = UserPolicy::ADMIN;
$this->be($admin);

$user = factory(User::class)->create();
assert($user instanceof User);

$this->schema = /** @lang GraphQL */ '
type Mutation {
throwWhenInvoked(id: ID!): User
@canQuery(ability: "view", action: EXCEPTION_NOT_AUTHORIZED)
}

type User {
name: String!
}
' . self::PLACEHOLDER_QUERY;

$this->graphQL(/** @lang GraphQL */ '
mutation ($id: ID!) {
throwWhenInvoked(id: $id) {
name
}
}
', [
'id' => $user->getKey(),
])->assertGraphQLErrorMessage(ThrowWhenInvoked::ERROR_MESSAGE);
}

public function testHandleMultipleModelsWithQuery(): void
{
$admin = new User();
Expand Down
36 changes: 36 additions & 0 deletions tests/Integration/Auth/CanRootDirectiveDBTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php declare(strict_types=1);

namespace Integration\Auth;

use Tests\DBTestCase;
use Tests\Utils\Models\User;
use Tests\Utils\Mutations\ThrowWhenInvoked;
use Tests\Utils\Policies\UserPolicy;

final class CanRootDirectiveDBTest extends DBTestCase
{
public function testDoesntConcealResolverException(): void
{
$admin = new User();
$admin->name = UserPolicy::ADMIN;
$this->be($admin);

$this->schema = /** @lang GraphQL */ '
type Mutation {
throwWhenInvoked: Task
}

type Task {
name: String! @canRoot(ability: "adminOnly", action: EXCEPTION_NOT_AUTHORIZED)
}
' . self::PLACEHOLDER_QUERY;

$this->graphQL(/** @lang GraphQL */ '
mutation {
throwWhenInvoked {
name
}
}
')->assertGraphQLErrorMessage(ThrowWhenInvoked::ERROR_MESSAGE);
}
}
20 changes: 20 additions & 0 deletions tests/Utils/Mutations/ThrowWhenInvoked.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php declare(strict_types=1);

namespace Tests\Utils\Mutations;

use Nuwave\Lighthouse\Exceptions\AuthorizationException;

final class ThrowWhenInvoked
{
public const ERROR_MESSAGE = 'Custom error message from ThrowWhenInvoked mutation.';

/**
* @param array<string, mixed> $args
*
* @return array<string, mixed>
*/
public function __invoke(mixed $root, array $args): array
{
throw new AuthorizationException(self::ERROR_MESSAGE);
}
}