fix(key-auth): retain order of query arguments when hiding the creden…

…tials

Fixes #12758 reported by @battlebyte.

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
(cherry picked from commit b3e065e)
Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>

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

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)

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

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()