Skip to content

Commit

Permalink
fix(admin): reject AdminAPI call with empty tags (#13723)
Browse files Browse the repository at this point in the history
(cherry picked from commit d1069e6)
  • Loading branch information
nowNick authored and kevwilliams committed Oct 30, 2024
1 parent 36a6a3d commit 1538517
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 5 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/fix-admin-api-for-empty-tags.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Fix for querying admin API entities with empty tags
type: bugfix
scope: Admin API
13 changes: 11 additions & 2 deletions kong/api/endpoints.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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]
Expand Down Expand Up @@ -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]

Expand Down
6 changes: 5 additions & 1 deletion kong/api/routes/consumers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 11 additions & 2 deletions kong/api/routes/upstreams.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions spec/02-integration/04-admin_api/03-consumers_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
10 changes: 10 additions & 0 deletions spec/02-integration/04-admin_api/07-upstreams_routes_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
20 changes: 20 additions & 0 deletions spec/02-integration/04-admin_api/14-tags_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 1538517

Please sign in to comment.