nuwave / nuwave/lighthouse

Validation is processed before guarding

Open
#1,780 10 comments 13 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
PHP
Stars
3.5k
Forks
468
Avg merge
3h 9m
Merged PRs (30d)
2

Description

Describe the bug
When creating a mutation with both the @guard directive as well as the @validator directive on the input, the validation is processed before the guarding.

Expected behavior/Solution
I expect the guarding to be processed before the validation, because if someone is not authenticated, there's is no point in further handling the request with input validation.

Steps to reproduce

  1. Create a mutation
extend type Mutation @guard {
    createStuff(input: StuffInput!): Stuff! @create
}

type Stuff @model(class: "App\\Models\\Stuff") {
    name: String!
}

input StuffInput @validator {
    name: String!
}
  1. Create a validator
<?php

namespace App\GraphQL\Validators;

use Nuwave\Lighthouse\Validation\Validator;

class StuffInputValidator extends Validator
{
    public function rules(): array
    {
        return [
            'name' => [
                'string',
                'min:24',
            ],
        ];
    }
}
  1. Create a test to check the unauthenticated error
<?php

namespace Tests;

use Illuminate\Contracts\Console\Kernel;
use Illuminate\Foundation\Application;
use Illuminate\Foundation\Testing\TestCase;
use Nuwave\Lighthouse\Testing\MakesGraphQLRequests;

class CreateStuffTest extends TestCase
{
    use MakesGraphQLRequests;

    public function createApplication(): Application
    {
        $app = require dirname(__DIR__) . DIRECTORY_SEPARATOR . 'bootstrap' . DIRECTORY_SEPARATOR . 'app.php';

        $app->make(Kernel::class)->bootstrap();

        return $app;
    }

    /**
     * @test
     */
    public function it_throws_an_error_if_the_user_is_unauthenticated(): void
    {
        $response = $this->graphQL(/** @lang GraphQL */ '
            mutation {
                createStuff(input: {
                    name: "not-long-enough"
                }) {
                    id
                }
            }
        ');
        
        static::assertContains('Unauthenticated.', $response->json('errors.*.message'));
    }
}

This test will fail, because the name attribute is not long enough. It should pass, because a user being unauthenticated should take precedence over validation.

Lighthouse Version
5.3.0

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the reported CreateStuffTest reproduction and the interaction between the @guard directive on Mutation and the @validator directive on StuffInput. Trace how the unauthenticated request is processed, then add regression coverage showing that the unauthenticated error takes precedence over validation and run the relevant test suite.

Written by the indexing model from the issue text.

Assessment

Tech stack
graphql, laravel, php
Domain
api, authentication, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.