Kong / kong

🦍 The Cloud-Native API Gateway and AI Gateway.
https://konghq.com/install/#kong-community
Apache License 2.0
39.06k stars 4.79k forks source link

OAuth plugin returns Unexpected error #1710

Closed nevalla closed 7 years ago

nevalla commented 8 years ago

Summary

When trying to do a GET request to an OAuth secured API with Content-Type: application/json header, Kong returns "An unexpected error occurred" with status 500. Don't see any errors on the logs. Other values than application/json work as expected.

Steps To Reproduce

  1. Create an API
  2. Add OAuth plugin
  3. Create GET request with Content-Type: application/json header

    Additional Details & Logs

    • Kong version 0.9.1, 0.9.2
Tieske commented 8 years ago

Does it also happen with 0.9.2?

nevalla commented 8 years ago

Just tested and answer is yes

mleace commented 8 years ago

Trouble is in that Kong tries to parse request body as application/json using CJSON module and it fails (empty body is not valid JSON) - see plugins/oauth2/access.lua, line 96.

IMO, in general Kong should return HTTP status 400 Bad Request when it fails to parse body instead of 500 (as it is a client error, not server's). Empty body is a special case, and I would suggest checking Content-Length header is greater than 0 before trying to parse the body - so that empty-body request will pass regardless of Content-Type.

edwardjrp commented 8 years ago

Kong version kong/0.9.1.

I got into the same issue. Content-Type can be sent on any type of request (i.e. POST, GET,PUT, etc) thats the normal behavior for a RESTful api. This just identify that everything being sent is JSON format.

The code is assuming that if content type is application/json then is a request with a body.

On GET request there is no body so when the Content-Type is set it will get a nil because there is not body in it, plugins/oauth2/access.lua:97: bad argument #1 to 'decode' (string expected, got nil).

A possibility for improvement is accepting application/json content-type regardless and ignore body request params for the http methods that don't have one like (GET, COPY, HEAD, PURGE and UNLOCK)

eddmann commented 7 years ago

We have encountered this problem to and I have published a PR which should hopefully address the issue: https://github.com/Mashape/kong/pull/1853

As a temporary measure we have added a custom nginx config. which includes the following before kong does it's access invocation.

location / {
    // ...

    lua_need_request_body on;

    access_by_lua_block {
        local content_type = ngx.req.get_headers()["Content-Type"]
        local is_json_request = content_type and string.find(string.lower(content_type), "application/json", nil, true)
        if is_json_request and ngx.req.get_body_data() == nil then
            ngx.req.set_body_data("{}")
        end

        kong.access()
    }

    // ...
}