Skip to content
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

Disable global middleware for a subset of fields #385

Closed
robsontenorio opened this issue Oct 14, 2018 · 9 comments · Fixed by #395
Closed

Disable global middleware for a subset of fields #385

robsontenorio opened this issue Oct 14, 2018 · 9 comments · Fixed by #395
Labels
enhancement A feature or improvement

Comments

@robsontenorio
Copy link
Contributor

Describe the solution you'd like

Currently i can set a global middleware for AUTH on config.

 'route' => [      
        'middleware' => ['auth:api']
    ],

Because 99% of my requests are protected. But in some cases 1% can be public available for unauthenticated users.

So, right now, if i use the global middleware i cant set another requests to public.
When most of queries are protected, it is easier set the global middleware than manually on each query.

Describe alternatives you've considered

If i choose to make it public, something like this would be useful. So it would override the global middleware.

extend type Query @group(middleware: []){  
   // many fields
}
@robsontenorio robsontenorio changed the title [FEATURE] Disable middleware for custom requests, when using global middleware [FEATURE] Disable global middleware for custom requests Oct 14, 2018
@enzonotario
Copy link
Collaborator

I think that you can use the directive @middleware(checks: ["auth:api"] for that

@robsontenorio
Copy link
Contributor Author

robsontenorio commented Oct 14, 2018

@enzonotario I`m using global middleware ("auth:api") because 99% of my resources are protected (user must be authenticated).

lighthouse.php

 'route' => [      
        'middleware' => ['auth:api']
    ],

I`m proposing some mechanism to disable that global middleware in some cases, when resources are public (another 1%).

These does not disable/override the global middleware:

extend type Query @middleware(checks: []){

}

extend type Query @group(middleware: []){

}

@enzonotario
Copy link
Collaborator

Ahh sorry! I misunderstand you.

Have you tried use the @group directive in the tefinitions of Query and Mutation types, and use @middleware(checks: []) in those field that you want to disable?

I mean, if I go to MiddlewareDirectiveTest and set:

  type Query {
                foo: String! @middleware(checks: ["auth:web", "auth:admin"])
                bar: String!
            }
            type Mutation @group(middleware: ["auth:api"]) {
                foo(bar: String!): String!
                bar(baz: String!): String!
            }

Both foo and bar mutations has the middleware. But if I put:

type Query {
                foo: String! @middleware(checks: ["auth:web", "auth:admin"])
                bar: String!
            }
            type Mutation @group(middleware: ["auth:api"]) {
                foo(bar: String!): String! @middleware(checks: [])
                bar(baz: String!): String!
            }

the middlewares for foo is resetted.

@spawnia
Copy link
Collaborator

spawnia commented Oct 14, 2018

@robsontenorio Thanks for bringing this up. Indeed, middlewares are among the less documented and less polished parts of Lighthouse.

The solution that @enzonotario suggested should work, without having tried it myself.

I plan to do some major rework on middlewares, because of an Issue i have come across, see #363. Your use-case is something that should be considered for the implementation, i added it to a small todo list over there.

@spawnia spawnia added the enhancement A feature or improvement label Oct 14, 2018
@spawnia spawnia changed the title [FEATURE] Disable global middleware for custom requests Disable global middleware for a subset of fields Oct 14, 2018
@robsontenorio
Copy link
Contributor Author

Thanks for your time!

@robsontenorio
Copy link
Contributor Author

robsontenorio commented Oct 15, 2018

@enzonotario It does not work.

How to reproduce

  1. Set this on lighthouse.php
'route' => [      
        'middleware' => ['auth:api']
    ],
  1. These won't work. It won't override the global middleware in any case.
extend type Query @middleware(checks: []){
  // ...
}

extend type Query @group(middleware: []){
  // ...
}

extend type Query{
    users: [User]  @middleware(checks: [])
}

@enzonotario
Copy link
Collaborator

enzonotario commented Oct 15, 2018

Try removing the middleware from the lighthouse.php.

Then, in your schema.graphql you have to put:

type Query @group...
type Mutation @group...

Then, in those fields that you want to reset the middlewares, put @middleware(checks: []).

If you post your schema maybe I can help you a bit more.

@robsontenorio
Copy link
Contributor Author

@enzonotario That does not work.

  1. REMOVED middleware from the lighthouse.php.

  2. THEN ...

schema.graphql

#import ./city.graphql

type Query @group(middleware: ["auth:api"]){
  me: User @auth
}

city.graphql (both wont work)

extend type Query{  
  cities: [City] @middleware(checks: [])
}

# OR

extend type Query @group(middleware: []){
   cities: [City] 
}

Maybe @spawnia middleware rework #363 solve it.

@spawnia
Copy link
Collaborator

spawnia commented Oct 17, 2018

Working on resolving this in #395

Global middleware from the config run the risk of aborting the whole query in case of failure. It runs before the GraphQL execution phase, and as such do not return spec compliant responses for errors.

Middlewares defined through @group should be able to be overwritten in fields with @middleware. Working on a test right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants