diff --git a/changelog/unreleased/kong/key_auth_www_authenticate.yml b/changelog/unreleased/kong/key_auth_www_authenticate.yml new file mode 100644 index 00000000000..3d1e12b085d --- /dev/null +++ b/changelog/unreleased/kong/key_auth_www_authenticate.yml @@ -0,0 +1,3 @@ +message: Add WWW-Authenticate headers to all 401 response in key auth plugin. +type: bugfix +scope: Plugin diff --git a/kong/plugins/key-auth/handler.lua b/kong/plugins/key-auth/handler.lua index 0c711cca133..246418f5b08 100644 --- a/kong/plugins/key-auth/handler.lua +++ b/kong/plugins/key-auth/handler.lua @@ -25,14 +25,11 @@ local KeyAuthHandler = { local EMPTY = {} -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_PLUGIN_CONF = { status = 500, message = "Invalid plugin configuration" } -local ERR_UNEXPECTED = { status = 500, message = "An unexpected error occurred" } +local ERR_DUPLICATE_API_KEY = "Duplicate API key found" +local ERR_NO_API_KEY = "No API key found in request" +local ERR_INVALID_AUTH_CRED = "Invalid authentication credentials" +local ERR_INVALID_PLUGIN_CONF = "Invalid plugin configuration" +local ERR_UNEXPECTED = "An unexpected error occurred" local function load_credential(key) @@ -99,11 +96,19 @@ local function get_body() return body end +local function server_error(message) + return { status = 500, message = message } +end + +local function unauthorized(message, www_auth_content) + return { status = 401, message = message, headers = { ["WWW-Authenticate"] = www_auth_content } } +end local function do_authentication(conf) + local www_auth_content = conf.realm ~= nil and 'Key realm="' .. conf.realm .. '"' or 'Key' if type(conf.key_names) ~= "table" then kong.log.err("no conf.key_names set, aborting plugin execution") - return nil, ERR_INVALID_PLUGIN_CONF + return nil, server_error(ERR_INVALID_PLUGIN_CONF) end local headers = kong.request.get_headers() @@ -160,14 +165,13 @@ local function do_authentication(conf) elseif type(v) == "table" then -- duplicate API key - return nil, ERR_DUPLICATE_API_KEY + return nil, unauthorized(ERR_DUPLICATE_API_KEY, www_auth_content) end end -- this request is missing an API key, HTTP 401 if not key or key == "" then - kong.response.set_header("WWW-Authenticate", _realm) - return nil, ERR_NO_API_KEY + return nil, unauthorized(ERR_NO_API_KEY, www_auth_content) end -- retrieve our consumer linked to this API key @@ -188,7 +192,7 @@ local function do_authentication(conf) -- no credential in DB, for this key, it is invalid, HTTP 401 if not credential or hit_level == 4 then - return nil, ERR_INVALID_AUTH_CRED + return nil, unauthorized(ERR_INVALID_AUTH_CRED, www_auth_content) end ----------------------------------------- @@ -203,7 +207,7 @@ local function do_authentication(conf) credential.consumer.id) if err then kong.log.err(err) - return nil, ERR_UNEXPECTED + return nil, server_error(ERR_UNEXPECTED) end set_consumer(consumer, credential) diff --git a/kong/plugins/key-auth/schema.lua b/kong/plugins/key-auth/schema.lua index 9af6aa2742f..0dd51e6cbf0 100644 --- a/kong/plugins/key-auth/schema.lua +++ b/kong/plugins/key-auth/schema.lua @@ -20,6 +20,7 @@ return { { key_in_query = { description = "If enabled (default), the plugin reads the query parameter in the request and tries to find the key in it.", type = "boolean", required = true, default = true }, }, { key_in_body = { description = "If enabled, the plugin reads the request body. Supported MIME types: `application/www-form-urlencoded`, `application/json`, and `multipart/form-data`.", type = "boolean", required = true, default = false }, }, { run_on_preflight = { description = "A boolean value that indicates whether the plugin should run (and try to authenticate) on `OPTIONS` preflight requests. If set to `false`, then `OPTIONS` requests are always allowed.", type = "boolean", required = true, default = true }, }, + { realm = { description = "When authentication or authorization fails, or there is an unexpected error, the plugin sends an `WWW-Authenticate` header with the `realm` attribute value.", type = "string", required = false }, }, }, }, }, }, diff --git a/spec/01-unit/01-db/01-schema/07-plugins_spec.lua b/spec/01-unit/01-db/01-schema/07-plugins_spec.lua index 1dead97596f..1bfc1efcefa 100644 --- a/spec/01-unit/01-db/01-schema/07-plugins_spec.lua +++ b/spec/01-unit/01-db/01-schema/07-plugins_spec.lua @@ -121,6 +121,7 @@ describe("plugins", function() key_names = { "apikey" }, hide_credentials = false, anonymous = ngx.null, + realm = ngx.null, key_in_header = true, key_in_query = true, key_in_body = false, diff --git a/spec/01-unit/01-db/01-schema/11-declarative_config/03-flatten_spec.lua b/spec/01-unit/01-db/01-schema/11-declarative_config/03-flatten_spec.lua index 632062e9960..0d02710e673 100644 --- a/spec/01-unit/01-db/01-schema/11-declarative_config/03-flatten_spec.lua +++ b/spec/01-unit/01-db/01-schema/11-declarative_config/03-flatten_spec.lua @@ -320,6 +320,7 @@ describe("declarative config: flatten", function() config = { anonymous = null, hide_credentials = false, + realm = null, key_in_header = true, key_in_query = true, key_in_body = false, @@ -430,6 +431,7 @@ describe("declarative config: flatten", function() config = { anonymous = null, hide_credentials = false, + realm = null, key_in_header = true, key_in_query = true, key_in_body = false, @@ -627,6 +629,7 @@ describe("declarative config: flatten", function() config = { anonymous = null, hide_credentials = false, + realm = null, key_in_header = true, key_in_query = true, key_in_body = false, @@ -1142,6 +1145,7 @@ describe("declarative config: flatten", function() config = { anonymous = null, hide_credentials = false, + realm = null, key_in_header = true, key_in_query = true, key_in_body = false, 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 8135569a1f8..015d1a92218 100644 --- a/spec/03-plugins/09-key-auth/02-access_spec.lua +++ b/spec/03-plugins/09-key-auth/02-access_spec.lua @@ -92,6 +92,7 @@ for _, strategy in helpers.each_strategy() do route = { id = route2.id }, config = { hide_credentials = true, + realm = "test-realm" }, } @@ -223,20 +224,41 @@ for _, strategy in helpers.each_strategy() do assert.not_nil(json) assert.matches("No API key found in request", json.message) end) - it("returns Unauthorized on empty key header", function() - local res = assert(proxy_client:send { - method = "GET", - path = "/status/200", - headers = { - ["Host"] = "key-auth1.com", - ["apikey"] = "", - } - }) - local body = assert.res_status(401, res) - local json = cjson.decode(body) - assert.not_nil(json) - assert.matches("No API key found in request", json.message) + describe("when realm is not configured", function() + it("returns Unauthorized on empty key header with no realm", function() + local res = assert(proxy_client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "key-auth1.com", + ["apikey"] = "", + } + }) + local body = assert.res_status(401, res) + local json = cjson.decode(body) + assert.not_nil(json) + assert.matches("No API key found in request", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) + end) + end) + describe("when realm is configured", function() + it("returns Unauthorized on empty key header with no realm", function() + local res = assert(proxy_client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "key-auth2.com", + ["apikey"] = "", + } + }) + local body = assert.res_status(401, res) + local json = cjson.decode(body) + assert.not_nil(json) + assert.matches("No API key found in request", json.message) + assert.equal('Key realm="test-realm"', res.headers["WWW-Authenticate"]) + end) end) + it("returns Unauthorized on empty key querystring", function() local res = assert(proxy_client:send { method = "GET", @@ -249,6 +271,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("No API key found in request", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("returns WWW-Authenticate header on missing credentials", function() local res = assert(proxy_client:send { @@ -259,7 +282,7 @@ for _, strategy in helpers.each_strategy() do } }) res:read_body() - assert.equal('Key realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"]) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -286,6 +309,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Invalid authentication credentials", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("handles duplicated key in querystring", function() local res = assert(proxy_client:send { @@ -299,6 +323,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Duplicate API key found", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -360,6 +385,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Invalid authentication credentials", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) -- lua-multipart doesn't currently handle duplicates at all. @@ -381,6 +407,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Duplicate API key found", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end @@ -397,6 +424,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Duplicate API key found", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("does not identify apikey[] as api keys", function() @@ -411,6 +439,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("No API key found in request", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("does not identify apikey[1] as api keys", function() @@ -425,6 +454,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("No API key found in request", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end end) @@ -456,6 +486,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Invalid authentication credentials", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -516,6 +547,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Invalid authentication credentials", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) res = assert(proxy_client:send { method = "GET", @@ -529,6 +561,7 @@ for _, strategy in helpers.each_strategy() do json = cjson.decode(body) assert.not_nil(json) assert.matches("Invalid authentication credentials", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -659,6 +692,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("No API key found in request", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -838,6 +872,7 @@ for _, strategy in helpers.each_strategy() do } }) assert.response(res).has.status(401) + assert.equal('Basic realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"]) end) it("fails 401, with only the second credential provided", function() @@ -850,6 +885,7 @@ for _, strategy in helpers.each_strategy() do } }) assert.response(res).has.status(401) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("fails 401, with no credential provided", function() @@ -861,6 +897,7 @@ for _, strategy in helpers.each_strategy() do } }) assert.response(res).has.status(401) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -1274,6 +1311,9 @@ for _, strategy in helpers.each_strategy() do }, }) assert.res_status(test[5], res) + if test[5] == 401 then + assert.equal('Key', res.headers["WWW-Authenticate"]) + end proxy_client:close() end) end