nuwave / lighthouse

A framework for serving GraphQL from Laravel
https://lighthouse-php.com
MIT License
3.36k stars 437 forks source link

Can't get @group directive to work #134

Closed EslamNasser closed 6 years ago

EslamNasser commented 6 years ago

Lighthouse version: 2.1-beta.5 for lumen support

Hello, i'm using lumen with lighthouse and everything is working perfectly except for the @group directive as i want to apply an auth middleware to a group of queries. Every time i try to execute one of the queries in that group it goes straight to the resolver without passing by the middleware first.

I made sure that the middleware is working as when i apply the middleware in the lighthouse config it works perfectly and all the graphql requests pass by the middleware first before being handled by the graphql controller.

This is my schema.graphql:

scalar DateTime @scalar(class: "DateTime")

type User{
   id: ID!,
   username: String,
   age: Int,
   notes: String,
   created_at: DateTime!,
   email: String,
   mobileNumber: String
}

type LoginResult{
   userid: String!,
   token: String!,
   msg: String
}

input LoginInput{
   username: String!,
   password: String! @bcrypt
}

type Query{
   login(mobileNumber: String!, password: String!): LoginResult! @field(resolver: "App\\GraphQL\\Queries\\Query@Login")
}

extend type Query @group(middleware: "jwt"){
   test: String! @field(resolver: "App\\GraphQL\\Queries\\Query@testQuery")
   getUsers: [User!]! @field(resolver: "App\\GraphQL\\Queries\\Query@GetUsers")
   getUserInfo: User! @field(resolver: "App\\GraphQL\\Queries\\Query@getUserInfo")
}

type Mutation{
   addUser(username: String!, age: Int!, notes: String, password: String! @bcrypt, mobileNumber:String!, email: String!): User! @create(model: "App\\Models\\User")
}

What could be the problem here?

Thanks a lot for an amazing project btw

kikoseijo commented 6 years ago

Try to add also the others, like web, did you removed all middleware in the configuration?

EslamNasser commented 6 years ago

Yes i stopped all the middlewares in the configurations

This is my lighthouse.php:

<?php

return [
        /*
    |--------------------------------------------------------------------------
    | LightHouse endpoint & middleware
    |--------------------------------------------------------------------------
    |
    | Setup this values as required,
    | default route endpoints its yourdomain.com/graphql
    | setup middleware here for all request,
    | setup more endpoints, ej: pointing to the controller value inside your route file
    |
    */
    'route_name' => 'graphql',

    'route' => [
        'prefix' => '',
        //'middleware' => ['jwt'],    // [ 'loghttp']
    ],

    /*
    |--------------------------------------------------------------------------
    | Directive registry
    |--------------------------------------------------------------------------
    |
    | This package allows you to create your own server-side directives. Change
    | these values to register the directory that will hold all of your
    | custom directives.
    |
    */
    'directives' => [__DIR__.'/../app/Http/GraphQL/Directives'],

    /*
    |--------------------------------------------------------------------------
    | Namespace registry
    |--------------------------------------------------------------------------
    |
    | This package provides a set of commands to make it easy for you to
    | create new parts in your GraphQL schema. Change these values to
    | match the namespaces you'd like each piece to be created in.
    |
    */
    'namespaces' => [
        'models' => 'App\\Models',
        'mutations' => 'App\\GraphQL\\Mutations',
        'queries' => 'App\\GraphQL\\Queries',
        'scalars' => 'App\\GraphQL\\Scalars',
    ],

     /*
     |--------------------------------------------------------------------------
     | GraphQL Controller
     |--------------------------------------------------------------------------
     |
     | Specify which controller (and method) you want to handle GraphQL requests.
     |
     */
    'controller' => 'Nuwave\Lighthouse\Support\Http\Controllers\GraphQLController@query',

    /*
    |--------------------------------------------------------------------------
    | Schema Cache
    |--------------------------------------------------------------------------
    |
    | Specify where the GraphQL schema should be cached.
    |
    */
    'cache' => null,

    /*
    |--------------------------------------------------------------------------
    | Global ID
    |--------------------------------------------------------------------------
    |
    | When creating a GraphQL type that is Relay compliant, provide a named field
    | for the Node identifier.
    |
    */
    'global_id_field' => '_relayId',

    /*
    |--------------------------------------------------------------------------
    | Schema declaration
    |--------------------------------------------------------------------------
    |
    | This is a path that points to where your GraphQL schema is located
    | relative to the app path. You should define your entire GraphQL
    | schema in this file (additional files may be imported).
    |
    */
    'schema' => [
        'register' => base_path('App/GraphQL/schema.graphql'),
    ],
];

Edit: Do i have to add the middleware to the route to be able to use it in the directive?

kikoseijo commented 6 years ago

try to add web, i think JWT needs web to work, i have it like this:

'middleware' => ['web','api'],

and in the schemas i add "api:auth" for passport

EslamNasser commented 6 years ago

Ok, forgive my ignorance here as i'm new to lumen but as far as i understand by adding 'middleware' => ['web', 'api], this means i have to have two middlewares web and api registered in my app.php. Correct? If yes i don't have those.

kikoseijo commented 6 years ago

no, pardon me, there is no web on Lumen. It should work as you have it, there might be an error, lets ask for help to the expert on this, @4levels

EslamNasser commented 6 years ago

When i just add api to the list of middlewares in lighthouse.php i get Class api does not exist

kikoseijo commented 6 years ago

api its the one provided by Laravel Passport, you using jwt.

EslamNasser commented 6 years ago

Could it be a problem in this specific release?

kikoseijo commented 6 years ago

Sure, the Beta versions are for testing things out, move to master and let us know if it works,

EslamNasser commented 6 years ago

But master doesn't have lumen support :'(

kikoseijo commented 6 years ago

Master should point to https://github.com/nuwave/lighthouse/releases/latest

and yes, has the lumen spport.

EslamNasser commented 6 years ago

I'm pretty sure that when i tried that version of lighthouse it didn't work for me, will double check it right now tho.

kikoseijo commented 6 years ago

Its only missing couple of functions, but should work out to the box since version v2

4levels commented 6 years ago

Hi @EslamNasser, I have not used the @group middleware so far, I'm currently using a single scope graphql to protect the graphql endpoints in lighthouse_config.php I'll be getting back to this soon since I cannot longer postpone working on the backend (my RN app requires this atm). I'm also very interested in using the @group directive, I'll report back here.

Since my current routing config looks like ["auth:api", "scopes:graphql"] I'll try with that to start with, adding scopes along the way. I am using Passport just like @kikoseijo, I believe this uses jwt tokens as well..

Oh, and I'm using the Lighthouse dev-master branch in my composer and it works well with Lumen. There are some caveats though and the configuration seems very tricky from time to time. I haven't had the chance yet to document how I got this working, make sure search the closed issue since March 2018 as they contain a lot of info (not very structured, but could be helpfull). I have to admit there was a lot of trial and error though..

EslamNasser commented 6 years ago

@4levels Thanks a lot for your response, i just switched to the latest release and i'm getting this error Call to undefined function Nuwave\Lighthouse\Support\Http\Controllers\auth() trying to hookup graphql playground to the backend. I've read before in this issue https://github.com/nuwave/lighthouse/issues/54 that this error is due to the difference between laravel and lumen in handling auth and i followed the fix which lead me to this release https://github.com/nuwave/lighthouse/releases/tag/v2.1-beta.5 which i was using before the current issue pops up.

What do you think i should go for here?

4levels commented 6 years ago

I'm adding a gist right now since there's quite some configuration to share ;-) Hang tight..

EslamNasser commented 6 years ago

Won't go anywhere :D

4levels commented 6 years ago

There youu go ;-) I had to remove all slashes from filenames (not very experienced with gists)..

Regarding the authentication: I'm using a Proxy class inside the app/Auth folder that adds the client id and secret to the request and performs the actual login using guzzle over localhost to prevent them from ever being transmitted over the web..

https://gist.github.com/4levels/4c732fbef1b87a3263998136f5bbe61d

EslamNasser commented 6 years ago

Alright will try this out from my side. @4levels Thanks a lot for the quick responses :)

4levels commented 6 years ago

You're welcome,

I've been in the same situation for days now (if not weeks), eagerly awaiting a reply on an issue (as you pbbly already noticed), glad to be able to help.

Oh, and I forgot to add any of the graphql schema etc etc, I'll update the gist..

4levels commented 6 years ago

Hi @EslamNasser, just so you know, I updated the gist and added some more files, especially the various ServiceProviders were hard to configure.. Hope this gets you going soon!

EslamNasser commented 6 years ago

@4levels @kikoseijo the dev-master version works with lumen but the latest master doesn't work with lumen. I tried the @group(middleware: ) directive ondev-master` and still didn't work for me.

Is there a special config i should have for it to work?

kikoseijo commented 6 years ago

Is it a big project? can you upload?

Maybe try on this https://github.com/kikoseijo/lumen-lighthouse-graphql

kikoseijo commented 6 years ago

@EslamNasser what you stuck at?

Been playing with middleware today and you won't get same functionality when added to the route than when inside schema. Let me know if we can help, maybe there is something, but I guess its more of what you expect to happen thats suppose to be happening or something you are not able to make it work.

Anycase, hope you manage to go though and maybe help others with same problem by providing your solution.

Happy coding.!"

EslamNasser commented 6 years ago

@kikoseijo Sorry i got sucked into other things, to answer your 1st question no it's not a big project i just started one to play around with 1st. I'm trying to make the @group(middleware: ) directive to work, as far as i understand when i use this directive any request going to this group of queries/mutations goes through the middleware 1st without the need to apply the middleware on the entire graphql endpoint and this is what i'm trying to achieve here. Let me know if there's another work around for this tho if you know any.

kikoseijo commented 6 years ago

@chrissm79 @EslamNasser , I found the problem.

When a query starts like this: Wont work


query CuponListQuery(
  $first: Int!
  $cursor: String
) {
  ...CuponList_feed
}

fragment CuponList_feed on Query {
  cupons(first: $first, after: $cursor) {

but if it starts likes this: It works

query CuponListQuery {
  cupons(first: 10, after: null) {

middleware gets called.

Problem seems to be on the parse of the query using fragments and will get this type of body when using a Relay <QueryRenderer

We need to work on this. Boss?

kikoseijo commented 6 years ago

@chrissm79 , deeper debugging got me to find out that the problem its here:

On the MiddlewareManager,

$fields = array_pluck(array_get($definition, 'selectionSet.selections', []), 'name.value');

recieves:

["CuponList_feed"]  

Somehow we should be able to convert this fragment to its value to retrieve the query.

Right now im stuck at trying to pull the real query when I find a FragmentSpread .

"selectionSet": {
    "kind": "SelectionSet",
    "loc": {
      "start": 52,
      "end": 75
    },
    "selections": [
      {
        "kind": "FragmentSpread",
        "loc": {
          "start": 56,
          "end": 73
        },
        "name": {
          "kind": "Name",
          "loc": {
            "start": 59,
            "end": 73
          },
          "value": "CuponList_feed"
        },
        "directives": []
      }
    ]
  }
chrissm79 commented 6 years ago

@kikoseijo Thanks for digging into this!! Taking a look and hopefully will get something figured out for this one soon!

chrissm79 commented 6 years ago

@EslamNasser @kikoseijo Okay, just pushed an update that should resolve the issue listed here. Pull down the latest from master and give it a go and let me know if you're still having issues!

@EslamNasser Thanks for reporting this!!! @kikoseijo Thanks for digging into things!!!

kikoseijo commented 6 years ago

@chrissm79 cant see the update, ill wait.

For now what i did was embed in an upper query and gets called.

chrissm79 commented 6 years ago

@kikoseijo Sorry about that, just pushed the update to the repo

kikoseijo commented 6 years ago

@chrissm79 you king! Works for me,

@EslamNasser @mits87 I guess your problems are solved after this patch.

Thanks a lot, Boss!

EslamNasser commented 6 years ago

@chrissm79 is it possible to push the fix on dev-master cause this is what i'm using as it has lumen support, please :D

kikoseijo commented 6 years ago

I believe dev master its the latest commit.

little helper for use with envoy tasks:

@task('link_lighthouse', ['on' => 'local'])
    rm -rf vendor/nuwave/lighthouse
    cd vendor/nuwave/
    ls -la
    ln -s ~/Developer/Laravel/Laravel-Plugins/lighthouse/ ./lighthouse
    ls -la
@endtask
EslamNasser commented 6 years ago

@chrissm79 @kikoseijo I'm still unable to get the @group directive to work, i'm sending the graphql queries using Graphql Background

kikoseijo commented 6 years ago

@EslamNasser many of us are using it and its working. Why don't you try to emulate your middleware in one of the example projects? 4levels or myself have some demo projects you can fork to try to make this tests so, me or anyone can test same conditions you working on to try to find the possible bug if you consider its a bug.

I did found 1 and the cause not long ago and the Boss! did the fix in couple days, so, don´t leave it at the side for long like been.

spawnia commented 6 years ago

Should be resolved through #140. You are now able to add middleware either through @group as you did above, or through the @middleware directive, applied directly to a field.

Closing this for now, feel free to reopen if the issue persists.