From 04200bd9644c1104a7bdf835a6da0c26aa984216 Mon Sep 17 00:00:00 2001 From: Makito <i@maki.to> Date: Tue, 30 Jan 2024 15:16:25 +0800 Subject: [PATCH] fix(plugins): consistent error responses upon Admin API auth failures (#12429) * fix(plugins): consistent error responses upon Admin API auth failures * fix(basic-auth): update error message (cherry picked from commit 60ea714e124ec81bef97031b9d334febcfa9303b) --- .../kong/enhance_admin_api_auth_error_response.yml | 3 +++ kong/plugins/basic-auth/access.lua | 2 +- kong/plugins/key-auth/handler.lua | 2 +- kong/plugins/ldap-auth/access.lua | 2 +- spec/02-integration/02-cmd/03-reload_spec.lua | 2 +- spec/03-plugins/09-key-auth/02-access_spec.lua | 10 +++++----- spec/03-plugins/10-basic-auth/03-access_spec.lua | 10 +++++----- spec/03-plugins/10-basic-auth/05-declarative_spec.lua | 2 +- spec/03-plugins/20-ldap-auth/01-access_spec.lua | 6 +++--- 9 files changed, 21 insertions(+), 18 deletions(-) create mode 100644 changelog/unreleased/kong/enhance_admin_api_auth_error_response.yml diff --git a/changelog/unreleased/kong/enhance_admin_api_auth_error_response.yml b/changelog/unreleased/kong/enhance_admin_api_auth_error_response.yml new file mode 100644 index 00000000000..fb508af5573 --- /dev/null +++ b/changelog/unreleased/kong/enhance_admin_api_auth_error_response.yml @@ -0,0 +1,3 @@ +message: "Enhance error responses for authentication failures in the Admin API" +type: bugfix +scope: Plugin diff --git a/kong/plugins/basic-auth/access.lua b/kong/plugins/basic-auth/access.lua index 8c76b526a53..e7017897524 100644 --- a/kong/plugins/basic-auth/access.lua +++ b/kong/plugins/basic-auth/access.lua @@ -155,7 +155,7 @@ end local function fail_authentication() - return false, { status = 401, message = "Invalid authentication credentials" } + return false, { status = 401, message = "Unauthorized" } end diff --git a/kong/plugins/key-auth/handler.lua b/kong/plugins/key-auth/handler.lua index 0c711cca133..81b2e309a4f 100644 --- a/kong/plugins/key-auth/handler.lua +++ b/kong/plugins/key-auth/handler.lua @@ -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" } diff --git a/kong/plugins/ldap-auth/access.lua b/kong/plugins/ldap-auth/access.lua index c04b6c50276..87a524f9ffe 100644 --- a/kong/plugins/ldap-auth/access.lua +++ b/kong/plugins/ldap-auth/access.lua @@ -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 diff --git a/spec/02-integration/02-cmd/03-reload_spec.lua b/spec/02-integration/02-cmd/03-reload_spec.lua index d05b01387d7..173ff408a4b 100644 --- a/spec/02-integration/02-cmd/03-reload_spec.lua +++ b/spec/02-integration/02-cmd/03-reload_spec.lua @@ -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()) 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 f176e7f246c..5e6d4dcc30d 100644 --- a/spec/03-plugins/09-key-auth/02-access_spec.lua +++ b/spec/03-plugins/09-key-auth/02-access_spec.lua @@ -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 { @@ -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. @@ -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) @@ -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", @@ -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) diff --git a/spec/03-plugins/10-basic-auth/03-access_spec.lua b/spec/03-plugins/10-basic-auth/03-access_spec.lua index acf2c4374d1..4256e4cc049 100644 --- a/spec/03-plugins/10-basic-auth/03-access_spec.lua +++ b/spec/03-plugins/10-basic-auth/03-access_spec.lua @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() diff --git a/spec/03-plugins/10-basic-auth/05-declarative_spec.lua b/spec/03-plugins/10-basic-auth/05-declarative_spec.lua index e73d1eaf503..a2f7af1ce06 100644 --- a/spec/03-plugins/10-basic-auth/05-declarative_spec.lua +++ b/spec/03-plugins/10-basic-auth/05-declarative_spec.lua @@ -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) diff --git a/spec/03-plugins/20-ldap-auth/01-access_spec.lua b/spec/03-plugins/20-ldap-auth/01-access_spec.lua index 9f10529a37a..2a5f48357d5 100644 --- a/spec/03-plugins/20-ldap-auth/01-access_spec.lua +++ b/spec/03-plugins/20-ldap-auth/01-access_spec.lua @@ -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 { @@ -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 { @@ -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 {