Skip to content

Commit

Permalink
fix(plugins): consistent error responses upon Admin API auth failures (
Browse files Browse the repository at this point in the history
…#12429)

* fix(plugins): consistent error responses upon Admin API auth failures

* fix(basic-auth): update error message

(cherry picked from commit 60ea714)
  • Loading branch information
sumimakito committed Jan 30, 2024
1 parent b0889c3 commit 354b2e0
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "Enhance error responses for authentication failures in the Admin API"
type: bugfix
scope: Plugin
2 changes: 1 addition & 1 deletion kong/plugins/basic-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ end


local function fail_authentication()
return false, { status = 401, message = "Invalid authentication credentials" }
return false, { status = 401, message = "Unauthorized" }
end


Expand Down
2 changes: 1 addition & 1 deletion kong/plugins/key-auth/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ local _realm = 'Key realm="' .. _KONG._NAME .. '"'

local ERR_DUPLICATE_API_KEY = { status = 401, message = "Duplicate API key found" }
local ERR_NO_API_KEY = { status = 401, message = "No API key found in request" }
local ERR_INVALID_AUTH_CRED = { status = 401, message = "Invalid authentication credentials" }
local ERR_INVALID_AUTH_CRED = { status = 401, message = "Unauthorized" }
local ERR_INVALID_PLUGIN_CONF = { status = 500, message = "Invalid plugin configuration" }
local ERR_UNEXPECTED = { status = 500, message = "An unexpected error occurred" }

Expand Down
2 changes: 1 addition & 1 deletion kong/plugins/ldap-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ local function do_authentication(conf)
end

if not is_authorized then
return false, {status = 401, message = "Invalid authentication credentials" }
return false, {status = 401, message = "Unauthorized" }
end

if conf.hide_credentials then
Expand Down
2 changes: 1 addition & 1 deletion spec/02-integration/02-cmd/03-reload_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ describe("key-auth plugin invalidation on dbless reload #off", function()
})
local body = res:read_body()
proxy_client:close()
return body ~= [[{"message":"Invalid authentication credentials"}]]
return body ~= [[{"message":"Unauthorized"}]]
end, 5)

admin_client = assert(helpers.admin_client())
Expand Down
10 changes: 5 additions & 5 deletions spec/03-plugins/09-key-auth/02-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)
it("handles duplicated key in querystring", function()
local res = assert(proxy_client:send {
Expand Down Expand Up @@ -365,7 +365,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)

-- lua-multipart doesn't currently handle duplicates at all.
Expand Down Expand Up @@ -461,7 +461,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)
end)

Expand Down Expand Up @@ -521,7 +521,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)

res = assert(proxy_client:send {
method = "GET",
Expand All @@ -534,7 +534,7 @@ for _, strategy in helpers.each_strategy() do
body = assert.res_status(401, res)
json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)
end)

Expand Down
10 changes: 5 additions & 5 deletions spec/03-plugins/10-basic-auth/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)

it("returns 401 Unauthorized on invalid credentials in Proxy-Authorization", function()
Expand All @@ -190,7 +190,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)

it("returns 401 Unauthorized on password only", function()
Expand All @@ -205,7 +205,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)

it("returns 401 Unauthorized on username only", function()
Expand All @@ -220,7 +220,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)

it("rejects gRPC call without credentials", function()
Expand Down Expand Up @@ -295,7 +295,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)

it("authenticates valid credentials in Proxy-Authorization", function()
Expand Down
2 changes: 1 addition & 1 deletion spec/03-plugins/10-basic-auth/05-declarative_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.matches("Unauthorized", json.message)
end)
end)

Expand Down
6 changes: 3 additions & 3 deletions spec/03-plugins/20-ldap-auth/01-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
})
assert.response(res).has.status(401)
local json = assert.response(res).has.jsonbody()
assert.equal("Invalid authentication credentials", json.message)
assert.equal("Unauthorized", json.message)
end)
it("returns 'invalid credentials' when credential value is in wrong format in proxy-authorization header", function()
local res = assert(proxy_client:send {
Expand All @@ -250,7 +250,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
})
assert.response(res).has.status(401)
local json = assert.response(res).has.jsonbody()
assert.equal("Invalid authentication credentials", json.message)
assert.equal("Unauthorized", json.message)
end)
it("returns 'invalid credentials' when credential value is missing in authorization header", function()
local res = assert(proxy_client:send {
Expand All @@ -263,7 +263,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
})
assert.response(res).has.status(401)
local json = assert.response(res).has.jsonbody()
assert.equal("Invalid authentication credentials", json.message)
assert.equal("Unauthorized", json.message)
end)
it("passes if credential is valid in post request", function()
local res = assert(proxy_client:send {
Expand Down

0 comments on commit 354b2e0

Please sign in to comment.