From 4c572650f03bf4ad77b34cabd7a58e6b41bfc4e9 Mon Sep 17 00:00:00 2001
From: Aapo Talvensaari <aapo.talvensaari@gmail.com>
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 <aapo.talvensaari@gmail.com>
---
 .../kong/fix-key-auth-retain-query-order.yml  |  3 ++
 kong/plugins/key-auth/handler.lua             |  3 +-
 .../03-plugins/09-key-auth/02-access_spec.lua | 42 +++++++++++++++++++
 3 files changed, 46 insertions(+), 2 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/plugins/key-auth/handler.lua b/kong/plugins/key-auth/handler.lua
index 73902e2e3bc4..89786aa7520a 100644
--- a/kong/plugins/key-auth/handler.lua
+++ b/kong/plugins/key-auth/handler.lua
@@ -143,8 +143,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 6c63dbde3edb..a1a7b3a925ec 100644
--- a/spec/03-plugins/09-key-auth/02-access_spec.lua
+++ b/spec/03-plugins/09-key-auth/02-access_spec.lua
@@ -712,6 +712,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()