From 8bf9d39e50d8c3a4686f74c7f66c72693518a096 Mon Sep 17 00:00:00 2001 From: Aapo Talvensaari Date: Wed, 4 Sep 2024 15:43:48 +0300 Subject: [PATCH] fix(key-auth): retain order of query arguments when hiding the credentials Fixes #12758 reported by @battlebyte. Signed-off-by: Aapo Talvensaari (cherry picked from commit b3e065e6c33c54ec3327e2f85f44cf7c4910d491) Signed-off-by: Aapo Talvensaari --- .../kong/fix-key-auth-retain-query-order.yml | 3 ++ kong/pdk/service/request.lua | 2 +- kong/plugins/key-auth/handler.lua | 3 +- .../03-plugins/09-key-auth/02-access_spec.lua | 42 +++++++++++++++++++ 4 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 changelog/unreleased/kong/fix-key-auth-retain-query-order.yml diff --git a/changelog/unreleased/kong/fix-key-auth-retain-query-order.yml b/changelog/unreleased/kong/fix-key-auth-retain-query-order.yml new file mode 100644 index 000000000000..813856f827b6 --- /dev/null +++ b/changelog/unreleased/kong/fix-key-auth-retain-query-order.yml @@ -0,0 +1,3 @@ +message: "**key-auth**: Fixed to retain order of query arguments when hiding the credentials." +type: bugfix +scope: Plugin diff --git a/kong/pdk/service/request.lua b/kong/pdk/service/request.lua index 5b614e48da6c..8f3993875e46 100644 --- a/kong/pdk/service/request.lua +++ b/kong/pdk/service/request.lua @@ -303,7 +303,7 @@ local function new(self) -- @usage -- kong.service.request.clear_query_arg("foo") request.clear_query_arg = function(name) - check_phase(access_and_rewrite) + check_phase(access_and_rewrite_ws) if type(name) ~= "string" then error("query argument name must be a string", 2) diff --git a/kong/plugins/key-auth/handler.lua b/kong/plugins/key-auth/handler.lua index 407be4fa0819..e10d2a9259bf 100644 --- a/kong/plugins/key-auth/handler.lua +++ b/kong/plugins/key-auth/handler.lua @@ -150,8 +150,7 @@ local function do_authentication(conf) key = v if conf.hide_credentials then - query[name] = nil - kong.service.request.set_query(query) + kong.service.request.clear_query_arg(name) kong.service.request.clear_header(name) if conf.key_in_body then diff --git a/spec/03-plugins/09-key-auth/02-access_spec.lua b/spec/03-plugins/09-key-auth/02-access_spec.lua index 5f18f00b2b64..952e65baa6ce 100644 --- a/spec/03-plugins/09-key-auth/02-access_spec.lua +++ b/spec/03-plugins/09-key-auth/02-access_spec.lua @@ -735,6 +735,48 @@ for _, strategy in helpers.each_strategy() do assert.matches("No API key found in request", json.message) assert.equal('Key', res.headers["WWW-Authenticate"]) end) + + it("does not remove apikey and preserves order of query parameters", function() + local res = assert(proxy_client:send { + method = "GET", + path = "/request?c=value1&b=value2&apikey=kong&a=value3", + headers = { + ["Host"] = "key-auth1.test" + } + }) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + + assert.equal("/request?c=value1&b=value2&apikey=kong&a=value3", json.vars.request_uri) + end) + + it("removes apikey and preserves order of query parameters", function() + local res = assert(proxy_client:send{ + method = "GET", + path = "/request?c=value1&b=value2&apikey=kong&a=value3", + headers = { + ["Host"] = "key-auth2.test" + } + }) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + + assert.equal("/request?c=value1&b=value2&a=value3", json.vars.request_uri) + end) + + it("removes apikey in encoded query and preserves order of query parameters", function() + local res = assert(proxy_client:send { + method = "GET", + path = "/request?c=valu%651&b=value2&api%6B%65%79=kong&a=valu%653", + headers = { + ["Host"] = "key-auth2.test" + } + }) + local body = assert.res_status(200, res) + local json = cjson.decode(body) + + assert.equal("/request?c=value1&b=value2&a=value3", json.vars.request_uri) + end) end) describe("config.anonymous", function()