Wrong directive order firing after PR #1666

[webard]

What about:

extend type Query @guard {
    getProduct(id: ID! @eq): Product
        @find
        @can(ability:"get product")
}

?

First should be fired @guard to check if user is authenticated, but after this change first fired directive is @can

Originally posted by @webard in #1666 (comment)

[spawnia]

That is an unfortunate side effect of the change. The mechanism for applying @guard in your example works by appending it to the individual fields, so what you wrote is equivalent to:

extend type Query {
    getProduct(id: ID! @eq): Product
        @find
        @can(ability:"get product")
        @guard
}

Given this definition, I would not expect @guard to apply before @can - after all field middleware should resolve in order of definition. Lighthouse is not smart about @can or @guard, they are just like any other field middleware.

[webard]

As I though, but more logical is prepending that directives. When I add directive to group of fields (this is how I treat schema from example, like group of fields), I expect that directive will run before rest of them It is defined before actually. Only middleware directives have meaningful order, so adding them before @find, @create should not break anything.

[LastDragon-ru]

@spawnia added the question label 20 days ago

From my point of view, this is a bug, not Question. Even worse: in these cases @validator is always run before @guard that is definitely wrong :(

authSignUp(
        a: String!
        b: String!
    ): Boolean! @guard @validator

authSignUp(
        a: String!
        b: String!
    ): Boolean! @validator @guard

[spawnia]

I added proper documentation for the lexical ordering of field middleware and the behaviour of applying @guard to types in #1734.

@LastDragon-ru @validator is not a field middleware, it merely provides data for the @validate directive. Validation belongs to global field middleware, which is executed before all other field middleware directives.

Perhaps we should special case @guard and run it first when it is present.

[spawnia]

Fixed in https://github.com/nuwave/lighthouse/releases/tag/v5.3.0

[wimski]

@spawnia

Fixed in https://github.com/nuwave/lighthouse/releases/tag/v5.3.0

Is the following fixed in that release?

Perhaps we should special case @guard and run it first when it is present.

Because I'm using 5.3.0 and validation is still run before guarding, which to me also seems wrong. If you're not authenticated, then just stop right there. Input validation is redundant when the user is not even allowed to access the endpoint.

[spawnia]

@wimski There is no special casing for @guard yet. Can you open up an issue for it?

[wimski]

@wimski There is no special casing for @guard yet. Can you open up an issue for it?

#1780