nuwave / lighthouse

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

Nuwave\Lighthouse\Schema\Context $request->user() use default guard when I use multi-guards #979

Closed mitoop closed 4 years ago

mitoop commented 5 years ago

Environment

Lighthouse Version: 4.3.0 Laravel Version: 6.0.4


I use multi guards in Laravel. And My Schema demo is

  resolveQuestion(
        data: ResolveQuestionMutationInput!
   ): Boolean
  @middleware(checks: ["auth:app"])
  @field(resolver: "ResolveQuestion@resolve")

  input ResolveQuestionMutationInput {
      id: String!
      other_field: String!
  }

In my situation, I found that the Nuwave\Lighthouse\Schema\Context@construct is always act before the middleware (checks: ["auth:app"]), even I dont use @auth or @can directive in the schema.

class Context implements GraphQLContext
{
    /**
     * An instance of the incoming HTTP request.
     *
     * @var \Illuminate\Http\Request
     */
    public $request;

    /**
     * An instance of the currently authenticated user.
     *
     * @var \Illuminate\Contracts\Auth\Authenticatable|null
     */
    public $user;

    /**
     * Create new context.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return void
     */
    public function __construct(Request $request)
    {
        $this->request = $request;
        $this->user = $request->user(); // here loads user
    }
    ... more

Nuwave\Lighthouse\Schema\Context use \Illuminate\Http\Request@user method to load user, and due to the middleware (checks: ["auth:app"]) act after the method which will result in that the $request->user() will always use default guard. But in fact I wanna use the app guard.

I feel puzzled that why the middleware act after user loaded and why Context always loads user.

enzonotario commented 5 years ago

Hi! try putting that middleware in the config

mitoop commented 5 years ago

Hi @enzonotario. Middleware in the config will runs before the GraphQL execution phase. And the expected guard cant be got.

class Authenticate
{
    /**
     * The authentication factory instance.
     *
     * @var \Illuminate\Contracts\Auth\Factory
     */
    protected $auth;

    /**
     * Create a new middleware instance.
     *
     * @param  \Illuminate\Contracts\Auth\Factory  $auth
     * @return void
     */
    public function __construct(Auth $auth)
    {
        $this->auth = $auth;
    }

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure  $next
     * @param  string[]  ...$guards
     * @return mixed
     *
     * @throws \Illuminate\Auth\AuthenticationException
     */
    public function handle($request, Closure $next, ...$guards)
    {
        $this->authenticate($request, $guards); // $guard will always `[ ]` and at last will use default guard

        return $next($request);
    }
spawnia commented 5 years ago

This seems related to #880 and is yet another testament to how @middleware is flawed.

olivernybroe commented 4 years ago

@spawnia Maybe it is just the naming which is flawed, because it acts like a normal http middleware, but it actually isn't.

spawnia commented 4 years ago

You can create a custom GraphQLContext, see https://github.com/nuwave/lighthouse/pull/1105, and utilize the correct guards.

Additionally, how about we add a config option that allows specifying the default guard that is used within Lighthouse functionality that touches authentication?

spawnia commented 4 years ago

This can now be solved by using the new AttemptAuthenticate middleware, see https://github.com/nuwave/lighthouse/pull/1197