Skip to content

Commit

Permalink
fix(oauth2) only retrieve token from body upon POST/PUT/PATCH requests
Browse files Browse the repository at this point in the history
* fix in the OAuth2 retrieval access_token path
* regression test

Fix #3055
From #3063
  • Loading branch information
WALL-E authored and thibaultcha committed Dec 15, 2017
1 parent cbbedb7 commit 552360a
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 3 deletions.
16 changes: 13 additions & 3 deletions kong/plugins/oauth2/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ local public_utils = require "kong.tools.public"
local string_find = string.find
local req_get_headers = ngx.req.get_headers
local ngx_set_header = ngx.req.set_header
local ngx_req_get_method = ngx.req.get_method
local ngx_req_get_uri_args = ngx.req.get_uri_args
local check_https = utils.check_https


Expand Down Expand Up @@ -92,10 +94,18 @@ local function get_redirect_uri(client_id)
end

local function retrieve_parameters()
ngx.req.read_body()

-- OAuth2 parameters could be in both the querystring or body
return utils.table_merge(ngx.req.get_uri_args(), public_utils.get_body_args())
local uri_args = ngx_req_get_uri_args()
local method = ngx_req_get_method()

if method == "POST" or method == "PUT" or method == "PATCH" then
ngx.req.read_body()
local body_args = public_utils.get_body_args()

return utils.table_merge(uri_args, body_args)
end

return uri_args
end

local function retrieve_scopes(parameters, conf)
Expand Down
37 changes: 37 additions & 0 deletions spec/03-plugins/26-oauth2/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1725,6 +1725,43 @@ describe("Plugin: oauth2 (access)", function()
})
assert.res_status(200, res)
end)
it("does not throw an error when request has no body", function()
-- Regression test for the following issue:
-- https://github.com/Kong/kong/issues/3055
--
-- We want to make sure we do not attempt to parse a
-- request body if the request isn't supposed to have
-- once in the first place.

-- setup: cleanup logs

local test_error_log_path = helpers.test_conf.nginx_err_logs
os.execute(":> " .. test_error_log_path)

-- TEST: access with a GET request

local token = provision_token()

local res = assert(proxy_ssl_client:send {
method = "GET",
path = "/request?access_token=" .. token.access_token,
headers = {
["Host"] = "oauth2.com"
}
})
assert.res_status(200, res)

-- Assertion: there should be no [error], including no error
-- resulting from an invalid request body parsing that were
-- previously thrown.

local pl_file = require "pl.file"
local logs = pl_file.read(test_error_log_path)

for line in logs:gmatch("[^\r\n]+") do
assert.not_match("[error]", line, nil, true)
end
end)
it("works when a correct access_token is being sent in an authorization header (bearer)", function()
local token = provision_token()

Expand Down

0 comments on commit 552360a

Please sign in to comment.