PoweredLocal / vrata

API gateway implemented in PHP and Lumen
MIT License
471 stars 116 forks source link

App\Http\Request leads to conflict when dispatching internal routes #38

Open renky opened 4 years ago

renky commented 4 years ago

Hi there

I'm using plenty of your vrata-gateway-code in my own api-gateway and your code helped me very much on my way. But now I figured out a problem when using laravel passport with a proxy request. This means, I use an additional Controller (LoginController) that accepts post-params username and password and then does an internal request to Passport oauth/token to get the token - like it is shown in this article (sorry - German): https://medium.com/@albrecht.mauersberger/spa-mit-vue-js-vuetify-laravel-lumen-und-passport-ii-83f7491297a6

But to reduce it - here the code of LoginControllers login function


public function login(Request $request)
    {
        $this->validate($request, [
            'username' => 'required',
            'password' => 'required',
        ]);

        $tokenRequest = Request::create(
            env('APP_URL') . 'api/oauth/token',
            'POST'
        );

        $tokenRequest->merge([
            'username' => $request->username,
            'password' => $request->password,
            'grant_type' => 'password',
            'client_id' => $this->client->id,
            'client_secret' => $this->client->secret,
            'scope' => '*'
        ]);

        return app()->dispatch($tokenRequest);
    }

there you see that it is dispatched an internal request to passport. And there the problem starts with the App\Http\Request-Class -> the dispatcher uses make('request') internally and now gets a App\Http\Request-Class what doesn't fit to this interal request and returns the originally that came from outside... - in short: my internal passport-request doesn't work any more... It took me several hours of debugging to find out, why my request didn't work - so maybe this helps somebody other...

I finally found a solution and I'd even suggest to include it to your project even if there is no internal request used... this solution totally removes the App\Http\Request-Class from your project. (I didn't fork, because I don't use your complete project... so I just share my solution here)

First change in Middleware: instead of attaching the Route with the function attachRoute we can use attributes of the original Request-Class - here my new middleware:

<?php

namespace App\Http\Middleware;

use App\Routing\RouteRegistry;
use Closure;
use Illuminate\Http\Request;

class HelperMiddleware
{
    /**
     * Handle an incoming request.
     *
     * @param Request $request
     * @param  \Closure  $next
     * @param string $id
     * @return mixed
     */
    public function handle(Request $request, Closure $next, $id)
    {
        $request->attributes->add([
            'route' => app()->make(RouteRegistry::class)->getRoute($id)
        ]);

        return $next($request);
    }
}

now the route-object is added to the Illuminate Requests attributes.

Next changes in GatewayController also replace App\Http\Request with Illuminate\Http\Request and then use this: $request->attributes->get('route') instead of $request->getRoute()

To receive all route-params you don't need an own Route-Resolver - it can be done with the standard-Request-Class: $request->route()[2] this always returns the params in Lumen (route() function always returns the route-array). so I replaced all calls of $request->getRouteParams() with $request->route()[2]

So now all functions except "route()" of the App\Http\Request-Class aren't used any more... and thats now the reason why I don't open a Merge-Request, because in MY application it seems like this functionality is also not necessary - so I just removed it, and the route()-Function of the base-class works for me... But it might be, that this doesn't work in your whole application - I'm not quite sure...

The additional benefit: I could remove the whole part of binding the own request-class in AppServiceProvider, no own RouteResolver, no own prepareRequest-Call, all the base-functionallity runs smooth. I currently use no trustedproxys - but to get this running again, I'd include package fideloper/proxy

I gues this solution could also work for you and increase performance additionally. I'm looking forward for comments