dingo / api

A RESTful API package for the Laravel and Lumen frameworks.
BSD 3-Clause "New" or "Revised" License
9.33k stars 1.25k forks source link

Internal request doesn't send data #548

Closed mentos1386 closed 9 years ago

mentos1386 commented 9 years ago

I am sending POST data (username and password) to /proxy where i add some more data to array and make Internal request to /oauth/access_token with new array.

$request = app('request')->only('username', 'password');

$credentials = [
    'grant_type'    => 'password',
    'username'      => $request['username'],
    'password'      => $request['password'],
    'client_id'     => env('OAUTH_CLIENT_ID'),
    'client_secret' => env('OAUTH_CLIENT_SECRET')
];

try{
    $response = $dispatcher->post('oauth/access_token', $credentials);
}catch (InternalHttpException $e)
{
    return response()->json($e->getResponse()->original);
}

But the problem is that data /oauth/access_token gets is only username and password from request to /proxy, and not new array.

  +request: ParameterBag {#130
    #parameters: array:2 [
      "username" => "Username"
      "password" => "Password"
    ]

What did I do wrong?

lightvision commented 9 years ago

I have same issue with internal request failing to get the response thanks to Transformer:

Argument 1 passed to RoboticAccounting\Api\v1\Http\Transformers\ClientGroupTransformer::transform() must be an instance of RoboticAccounting\ClientGroup, null given, called in /home/vagrant/projects/hdcrmwithdingoapi.local/vendor/league/fractal/src/Scope.php on line 307 and defined 

The code in the ClientGroupTransformer:

<?php

namespace RoboticAccounting\Api\v1\Http\Transformers;

use League\Fractal\TransformerAbstract;
use RoboticAccounting\ClientGroup;

/**
 * ClientGroupTransformer
 *
 * @version 1.0.0
 * @author marius ionel <webmaster@grupnet.ro>
 */
class ClientGroupTransformer extends TransformerAbstract{

    public function transform(ClientGroup $client_group){

        return [
            //'id'=>(int) $client_group->id,
            'client_group'=>$client_group->name
        ];
    }
}
jasonlewis commented 9 years ago

@lightvision, I think that might be something else. If this is an include then you'll need to ensure that the parent has a relation, otherwise it returns null instead of (in your case) an instance of RoboticAccounting\ClientGroup.

@mentos1386 are you retrieving the request input in your OAuth route or is it being handled by a separate package? I just did a quick test and I can send data internally. Just try a simple route to see if you can send internal data, but let me know how you're retrieving the data on the other end.

lightvision commented 9 years ago

@jasonlewis You are right. It's not dingo issue while is a weird one, because the only wrong code was in a seeder, where an index was duplicated although the database wasn't migrated alt all.

mentos1386 commented 9 years ago

@jasonlewis I am sending request to function Authorizer::issueAccessToken() which of uses league/oauth2-server.

I belive the problem is, that Oauth2-server is using symfony/HttpFundation/Request. And this only gets:

  +request: ParameterBag {#121
    #parameters: array:2 [
      "username" => "Dejon00@Kiehn.com"
      "password" => "password"
    ]
  }

as illuminate/http/Request or Dingo\Api\Http\Request gets:

  +request: ParameterBag {#194
    #parameters: array:5 [
      "grant_type" => "password"
      "username" => "Dejon00@Kiehn.com"
      "password" => "password"
      "client_id" => "1"
      "client_secret" => "secret"
    ]
  }

Why is this difference between these to? And how can i fix it? Should i change the oauth2-server to use illiminate/request?

jasonlewis commented 9 years ago

Are you using the bridge package (lucadegasperi/oauth2-server-laravel)?

mentos1386 commented 9 years ago

Yeah. I’m sending request to it. https://github.com/lucadegasperi/oauth2-server-laravel/blob/master/src/Authorizer.php

jasonlewis commented 9 years ago

Let me set something up locally to see if I can get to the bottom of it. The bound request instance in the container should be set on the authorizer when it's updated. So it should be getting the updated request instance. I'll see what I can uncover.

jasonlewis commented 9 years ago

All looks fine on my end. The request is being updated for me.

Can you open up League\OAuth2\Server\AuthorizationServer and down the bottom in the issueAccessToken method dump out $this->getRequest(). Just at the top.

Should look like this:

public function issueAccessToken()
{
    dd($this->getRequest());

    $grantType = $this->getRequest()->request->get('grant_type');
    if (is_null($grantType)) {
        throw new Exception\InvalidRequestException('grant_type');
    }

    // Ensure grant type is one that is recognised and is enabled
    if (!in_array($grantType, array_keys($this->grantTypes))) {
        throw new Exception\UnsupportedGrantTypeException($grantType);
    }

    // Complete the flow
    return $this->getGrantType($grantType)->completeFlow();
}

You should get an instance of Dingo\Api\Http\InternalRequest. That's what I'm seeing. If you're not... then there's something else wrong.

What version of the OAuth2 package are you using? 5.0.*@dev?

mentos1386 commented 9 years ago
Request {#146
  #json: null
  #userResolver: null
  #routeResolver: Closure {#126
    class: "Laravel\Lumen\Application"
    this: Application {#3 …}
    file: "/home/vagrant/Projects/Threads/ThreadsAPI/vendor/laravel/lumen-framework/src/Application.php"
    line: "680 to 682"
  }
  +attributes: ParameterBag {#142
    #parameters: []
  }
  +request: ParameterBag {#145
    #parameters: array:2 [
      "username" => "Username"
      "password" => "Password"
    ]
  }
  +query: ParameterBag {#144
    #parameters: []
  }

// server bag removed... // 

 +files: FileBag {#128
    #parameters: []
  }
  +cookies: ParameterBag {#130
    #parameters: []
  }
  +headers: HeaderBag {#133
    #headers: array:11 [
      "content-type" => array:1 [
        0 => "application/x-www-form-urlencoded"
      ]
      "content-length" => array:1 [
        0 => "35"
      ]
      "host" => array:1 [
        0 => "api.threads.app"
      ]
      "connection" => array:1 [
        0 => "keep-alive"
      ]
      "csp" => array:1 [
        0 => "active"
      ]
      "cache-control" => array:1 [
        0 => "no-cache"
      ]
      "origin" => array:1 [
        0 => "chrome-extension://fhbjgbiflinjbdggehcddcbncdddomop"
      ]
      "user-agent" => array:1 [
        0 => "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/43.0.2357.130 Chrome/43.0.2357.130 Safari/537.36"
      ]
      "accept" => array:1 [
        0 => "*/*"
      ]
      "accept-encoding" => array:1 [
        0 => "gzip, deflate"
      ]
      "accept-language" => array:1 [
        0 => "en-US,en;q=0.8"
      ]
    ]
    #cacheControl: array:1 [
      "no-cache" => true
    ]
  }
  #content: null
  #languages: null
  #charsets: null
  #encodings: null
  #acceptableContentTypes: null
  #pathInfo: "/oauth/proxy"
  #requestUri: "/oauth/proxy"
  #baseUrl: ""
  #basePath: null
  #method: "POST"
  #format: null
  #session: null
  #locale: null
  #defaultLocale: "en"
}

Dump from request to /oauth/proxy https://gist.github.com/mentos1386/aa7adf972253822a7c59

Yes I am using 5.0.*@dev

jasonlewis commented 9 years ago

Is this proxy route under the API routes or registered to the Laravel router?

On Mon, 3 Aug 2015 21:59 Tine Jozelj notifications@github.com wrote:

Request {#146

json: null

userResolver: null

routeResolver: Closure {#126

class: "Laravel\Lumen\Application"
this: Application {#3 …}
file: "/home/vagrant/Projects/Threads/ThreadsAPI/vendor/laravel/lumen-framework/src/Application.php"
line: "680 to 682"

} +attributes: ParameterBag {#142

parameters: []

} +request: ParameterBag {#145

parameters: array:2 [

  "username" => "Username"
  "password" => "Password"
]

} +query: ParameterBag {#144

parameters: []

}

// server bag removed... //

+files: FileBag {#128

parameters: []

} +cookies: ParameterBag {#130

parameters: []

} +headers: HeaderBag {#133

headers: array:11 [

  "content-type" => array:1 [
    0 => "application/x-www-form-urlencoded"
  ]
  "content-length" => array:1 [
    0 => "35"
  ]
  "host" => array:1 [
    0 => "api.threads.app"
  ]
  "connection" => array:1 [
    0 => "keep-alive"
  ]
  "csp" => array:1 [
    0 => "active"
  ]
  "cache-control" => array:1 [
    0 => "no-cache"
  ]
  "origin" => array:1 [
    0 => "chrome-extension://fhbjgbiflinjbdggehcddcbncdddomop"
  ]
  "user-agent" => array:1 [
    0 => "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/43.0.2357.130 Chrome/43.0.2357.130 Safari/537.36"
  ]
  "accept" => array:1 [
    0 => "_/_"
  ]
  "accept-encoding" => array:1 [
    0 => "gzip, deflate"
  ]
  "accept-language" => array:1 [
    0 => "en-US,en;q=0.8"
  ]
]
#cacheControl: array:1 [
  "no-cache" => true
]

}

content: null

languages: null

charsets: null

encodings: null

acceptableContentTypes: null

pathInfo: "/oauth/proxy"

requestUri: "/oauth/proxy"

baseUrl: ""

basePath: null

method: "POST"

format: null

session: null

locale: null

defaultLocale: "en"

}

Dump from request to /oauth/proxy https://gist.github.com/mentos1386/aa7adf972253822a7c59

Yes I am using 5.0.*@dev

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/548#issuecomment-127209649.

mentos1386 commented 9 years ago

Under API routes

$api = app('Dingo\Api\Routing\Router');
/* ... */
        $api->post('oauth/proxy', 'OauthController@proxy');

        $api->post('oauth/access_token', function(){
            return response()->json(Authorizer::issueAccessToken());
        });
jasonlewis commented 9 years ago

Damn, that's the last thing I can think of. Racking my brain here and I can't see anything different between what you've got and what I've got.

As an aside you shouldn't be using the response()->json() method, just return the Authorizer::issueAccessToken() directly. Same same for your internal exception handling in the first post. API will handle the JSON stuff.

jasonlewis commented 9 years ago

Assuming you've got the API configured correctly I can't see how you're getting these results. Double check everything, your prefix or domain, make sure it's all setup correctly.

Inside a standard get API route try the following: dd(app('request')); If the request object is NOT an instance of Dingo\Api\Http\Request then something is misconfigured somewhere or you've got some other code that replaces it.

mentos1386 commented 9 years ago

Everything else works fine. I went over configuration, nothing that could be wrong (I think).

Here is the request from getAPI route with dd(app('request'));

Where could i see if request object is an instance of Dingo\Api\Http\Request ?

My composer.json if any packages are conflicting or wrong version.

    "require": {
        "laravel/lumen-framework": "5.1.*",
        "vlucas/phpdotenv": "~1.0",
        "dingo/api": "0.10.*@dev",
        "lucadegasperi/oauth2-server-laravel": "5.*@dev",
        "webpatser/laravel-uuid": "2.*@dev",
        "illuminate/pagination": "^5.1",
        "vinkla/hashids": "^2.0"
    },
jasonlewis commented 9 years ago

Oh you're using Lumen. I tested Laravel. That could be the difference.

On Mon, 3 Aug 2015 23:02 Tine Jozelj notifications@github.com wrote:

Everything else works fine. I went over configuration, nothing that could be wrong (I think).

Here https://gist.github.com/mentos1386/3cdba4943d0ae8fd5957 is the request from getAPI route with dd(app('request'));

Where could i see if request object is an instance of Dingo\Api\Http\Request ?

My composer.json if any packages are conflicting or wrong version.

"require": {
    "laravel/lumen-framework": "5.1.*",
    "vlucas/phpdotenv": "~1.0",
    "dingo/api": "0.10.*@dev",
    "lucadegasperi/oauth2-server-laravel": "5.*@dev",
    "webpatser/laravel-uuid": "2.*@dev",
    "illuminate/pagination": "^5.1",
    "vinkla/hashids": "^2.0"
},

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/548#issuecomment-127223399.

jasonlewis commented 9 years ago

Well it works fine for me on Lumen as well. Damn this is annoying.

You can see what type of request it is by hovering over the Request at the top, it should tell you the full namespaced name.

Those packages all look fine to me so I doubt it's a conflict anywhere.

In your oauth/access_token what do you get if you dump the request input?

dd(app('request')->input());

Only other thing I can think of is try some basic routes that shouldn't touch other packages:

$api->version('v1', function($api) {
    $api->get('foo', function () {
        return app('Dingo\Api\Dispatcher')->post('bar', ['name' => 'bob']);
    });

    $api->post('bar', function () {
        return app('request')->input();
    });
});

Expected response and what I'm seeing is:

{
    "name": "bob"
}
mentos1386 commented 9 years ago

I tried the basic rutes, it workes.

{
  "name": "bob"
}

But this isnt using Symfony\Component\HttpFundation\Request that the oauth2-server is using.

Try this, it is how oauth2-server gets request, and it should return same as before:

    $api->get('foo', function () {
        return app('Dingo\Api\Dispatcher')->post('/bar', ['name' => 'bob']);
    });

    $api->post('bar', function () {
        return \Symfony\Component\HttpFoundation\Request::createFromGlobals()->request->all();
    });

But all I got is an empty array, could it be a problem with Symfony\Component\HttpFundation\Request? Look at this https://github.com/dingo/api/issues/548#issuecomment-127185289

mentos1386 commented 9 years ago

In your oauth/access_token what do you get if you dump the request input?

dd(app('request')->input());

I get:

array:5 [
  "grant_type" => "password"
  "username" => "Username"
  "password" => "Password"
  "client_id" => "1"
  "client_secret" => "secret"
]

Which is correct. So the problem must be with Symfony\Component\HttpFundation\Request

jasonlewis commented 9 years ago

Okay another thing you can check for me is to open up League\OAuth2\Server\AbstractServer and find the getRequest method.

Debug in there to see if it's getting the request from the property or if it's building it's own. If it's building its own request then there's something strange happening. You can also try debugging the setRequest method, as this method should be called a couple of times to update the request instance.

mentos1386 commented 9 years ago

setRequest()

Returns

  +request: ParameterBag {#147
    #parameters: array:2 [
      "username" => "Username"
      "password" => "Password"
    ]
  }

And getRequest() returns:

  +request: ParameterBag {#147
    #parameters: array:2 [
      "username" => "Username"
      "password" => "Password"
    ]

Both of them are instance of Dingo\Api\Http\Request

Even app('request')

    public function setRequest($request)
    {
        $this->request = app('request');

        dd($this->request);
        return $this;
    }

that worked on bar->foo route, doesn’t work here.

But weirdly, this works

    public function getRequest()
    {
        if ($this->request === null) {
            $this->request = Request::createFromGlobals();
        }

        return app('request');
    }
  +request: ParameterBag {#195
    #parameters: array:5 [
      "grant_type" => "password"
      "username" => "Username"
      "password" => "Password"
      "client_id" => "1"
      "client_secret" => "secret"
    ]
  }
jasonlewis commented 9 years ago

Hm, I've just pushed an update. Try again without any modification to the files. Hopefully the rebound event is fired properly now.

jasonlewis commented 9 years ago

Actually never mind, a closer inspection leads me to believe the change won't fix anything.

I'm stumped my friend. I cannot fathom why you're not getting an updated request. My advice at this stage would be to try a fresh install with the everything and see if you can replicate it. If you can, make a GitHub repository of everything that I can clone and try out locally.

lightvision commented 9 years ago

@mentos1386 The @jasonlewis advice is the only good advice. I already mentioned that a bad migration file blows up my internal request even if that file was not migrated, and any way, has nothing to do with dingo/api.

So I had to start over and follow the TDD in order to not get the same weird issues.

mentos1386 commented 9 years ago

@jasonlewis Tried fresh, it works fine now, I have no idea what was messing up before.

And also, It would be good if instead of:

message": "500 Internal Server Error",
  "status_code": 500,
  "debug": {
    "line": 513,
    "file": "\\/home\\/vagrant\\/Projects\\/Hashtag\\/API\\/vendor\\/dingo\\/api\\/src\\/Dispatcher.php",
    "class": "Dingo\\\\Api\\\\Exception\\\\InternalHttpException",
    "trace": [

the dispatcher would return error that requested site returned, easier for debugging. Now all I know is that the requested site returned error, but I don’t know anything about the error itself.

Thanks for all the time.

@lightvision yeah, I should start using TDD as well

jasonlewis commented 9 years ago

Internal requests should rethrow exceptions. Where are you seeing that?

On Tue, 4 Aug 2015 23:41 Tine Jozelj notifications@github.com wrote:

@jasonlewis https://github.com/jasonlewis Tried fresh, it works fine now, I have no idea what was messing up before.

And also, It would be good if instead of:

message": "500 Internal Server Error", "status_code": 500, "debug": { "line": 513, "file": "\/home\/vagrant\/Projects\/Hashtag\/API\/vendor\/dingo\/api\/src\/Dispatcher.php", "class": "Dingo\Api\Exception\InternalHttpException", "trace": [

the dispatcher would return error that requested site returned, easier for debugging. Now all I know is that the requested site returned error, but I don’t know anything about the error itself.

Thanks for all the time.

@lightvision https://github.com/lightvision yeah, I should start using TDD as well

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/548#issuecomment-127616549.

mentos1386 commented 9 years ago

Yes it dose throw exception, the one i provided in previuse comment. But why modifying the message, why not just respond with the error the requested site gave.

For example:

{
  "message": "The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed. Check the \"grant_type\" parameter.",
  "status_code": 500,
  "debug": {
    "line": 266,
    "file": "\\/home\\/vagrant\\/Projects\\/Hashtag\\/API\\/vendor\\/league\\/oauth2-server\\/src\\/AuthorizationServer.php",
    "class": "League\\\\OAuth2\\\\Server\\\\Exception\\\\InvalidRequestException",
    "trace": [

This error gave requested site, but dispatcher modify it to be:

{
  "message": "500 Internal Server Error",
  "status_code": 500,
  "debug": {
    "line": 513,
    "file": "\\/home\\/vagrant\\/Projects\\/Hashtag\\/API\\/vendor\\/dingo\\/api\\/src\\/Dispatcher.php",
    "class": "Dingo\\\\Api\\\\Exception\\\\InternalHttpException",
    "trace": [

It would be easier for debugging, if dispatcher would throw original error.

jasonlewis commented 9 years ago

Can I see your oauth route where the exception is thrown?

The only time this package will intercept and return an InternalHttpException is when your internal request returns an non-successful response. This is so that an exception is thrown for these internal requests.

If you just throw exceptions from your API routes as per normal then the dispatcher will not touch them.

tjmartin69 commented 7 years ago

I was having the same issue. Were you making your request via ajax?

When submitting via ajax it was leaving the Content-Type as application/json so when asking the request for the data it saw it as wanting json so returned the data inside php://input.

To get around this when using the with method I reset the content-type to what I want it to be.

e.g.

if ($this->with !== null) {
    $api->with($this->with);

    if ($this->json === null) {
        $contentType = ($method === 'get') ? 'text/html' : (($this->attach !== null) ? 'multipart/form-data' : 'application/x-www-form-urlencoded');
        $api->header('Content-Type', $contentType);
    }
}

That is just an example as I have a wrapper around the internal request.

Hope this helps everyone.