fix(admin): reject AdminAPI call with empty tags (#13723)

changelog/unreleased/kong/fix-admin-api-for-empty-tags.yml

@@ -0,0 +1,3 @@
+message: Fix for querying admin API entities with empty tags
+type: bugfix
+scope: Admin API

kong/api/endpoints.lua

@@ -137,7 +137,7 @@ local function handle_error(err_t)
 end
 
 
-local function extract_options(args, schema, context)
+local function extract_options(db, args, schema, context)
   local options = {
     nulls = true,
     pagination = {
@@ -156,6 +156,11 @@ local function extract_options(args, schema, context)
     end
 
     if schema.fields.tags and args.tags ~= nil and context == "page" then
+      if args.tags == null or #args.tags == 0 then
+        local error_message = "cannot be null"
+        return nil, error_message, db[schema.name].errors:invalid_options({tags = error_message})
+      end
+
       local tags = args.tags
       if type(tags) == "table" then
         tags = tags[1]
@@ -207,7 +212,11 @@ local function query_entity(context, self, db, schema, method)
     args = self.args.uri
   end
 
-  local opts = extract_options(args, schema, context)
+  local opts, err, err_t = extract_options(db, args, schema, context)
+  if err then
+    return nil, err, err_t
+  end
+
   local schema_name = schema.name
   local dao = db[schema_name]
 

kong/api/routes/consumers.lua

@@ -20,7 +20,11 @@ return {
 
       -- Search by custom_id: /consumers?custom_id=xxx
       if custom_id then
-        local opts = endpoints.extract_options(args, db.consumers.schema, "select")
+        local opts, _, err_t = endpoints.extract_options(db, args, db.consumers.schema, "select")
+        if err_t then
+          return endpoints.handle_error(err_t)
+        end
+
         local consumer, _, err_t = db.consumers:select_by_custom_id(custom_id, opts)
         if err_t then
           return endpoints.handle_error(err_t)

kong/api/routes/upstreams.lua

@@ -25,7 +25,12 @@ local function set_target_health(self, db, is_healthy)
     target, _, err_t = endpoints.select_entity(self, db, db.targets.schema)
 
   else
-    local opts = endpoints.extract_options(self.args.uri, db.targets.schema, "select")
+    local opts
+    opts, _, err_t = endpoints.extract_options(db, self.args.uri, db.targets.schema, "select")
+    if err_t then
+      return endpoints.handle_error(err_t)
+    end
+
     local upstream_pk = db.upstreams.schema:extract_pk_values(upstream)
     local filter = { target = unescape_uri(self.params.targets) }
     target, _, err_t = db.targets:select_by_upstream_filter(upstream_pk, filter, opts)
@@ -94,7 +99,11 @@ local function target_endpoint(self, db, callback)
     target, _, err_t = endpoints.select_entity(self, db, db.targets.schema)
 
   else
-    local opts = endpoints.extract_options(self.args.uri, db.targets.schema, "select")
+    local opts
+    opts, _, err_t = endpoints.extract_options(db, self.args.uri, db.targets.schema, "select")
+    if err_t then
+      return endpoints.handle_error(err_t)
+    end
     local upstream_pk = db.upstreams.schema:extract_pk_values(upstream)
     local filter = { target = unescape_uri(self.params.targets) }
     target, _, err_t = db.targets:select_by_upstream_filter(upstream_pk, filter, opts)

spec/02-integration/04-admin_api/03-consumers_routes_spec.lua

@@ -282,6 +282,16 @@ describe("Admin API (#" .. strategy .. "): ", function()
         local body = assert.response(res).has.status(200)
         assert.match('"data":%[%]', body)
       end)
+      it("returns bad request for empty tags", function()
+        local res = assert(client:send {
+          method = "GET",
+          path = "/consumers",
+          query = { tags = ngx.null}
+        })
+        res = assert.res_status(400, res)
+        local json = cjson.decode(res)
+        assert.same("invalid option (tags: cannot be null)", json.message)
+      end)
     end)
     it("returns 405 on invalid method", function()
       local methods = {"DELETE", "PATCH"}

spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua

@@ -637,6 +637,16 @@ describe("Admin API: #" .. strategy, function()
         })
         assert.res_status(200, res)
       end)
+      it("returns bad request for empty tags", function()
+        local res = assert(client:send {
+          method = "GET",
+          path = "/upstreams",
+          query = { tags = ngx.null}
+        })
+        res = assert.res_status(400, res)
+        local json = cjson.decode(res)
+        assert.same("invalid option (tags: cannot be null)", json.message)
+      end)
 
       describe("empty results", function()
         lazy_setup(function()

spec/02-integration/04-admin_api/14-tags_spec.lua

@@ -73,6 +73,26 @@ describe("Admin API - tags", function()
         end
       end)
 
+      it("filter by empty tag", function()
+        local res = assert(client:send {
+          method = "GET",
+          path = "/consumers?tags="
+        })
+        local body = assert.res_status(400, res)
+        local json = cjson.decode(body)
+        assert.same("invalid option (tags: cannot be null)", json.message)
+      end)
+
+      it("filter by empty string tag", function()
+        local res = assert(client:send {
+          method = "GET",
+          path = "/consumers?tags=''"
+        })
+        local body = assert.res_status(200, res)
+        local json = cjson.decode(body)
+        assert.equals(0, #json.data)
+      end)
+
       it("filter by multiple tags with AND", function()
         local res = assert(client:send {
           method = "GET",