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 000000000000..3d1e12b085d7 --- /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 81b2e309a4f5..73902e2e3bc4 100644 --- a/kong/plugins/key-auth/handler.lua +++ b/kong/plugins/key-auth/handler.lua @@ -7,6 +7,7 @@ local type = type local error = error local ipairs = ipairs local tostring = tostring +local fmt = string.format local HEADERS_CONSUMER_ID = constants.HEADERS.CONSUMER_ID @@ -25,14 +26,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 = "Unauthorized" } -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 = "Unauthorized" +local ERR_INVALID_PLUGIN_CONF = "Invalid plugin configuration" +local ERR_UNEXPECTED = "An unexpected error occurred" local function load_credential(key) @@ -99,13 +97,21 @@ 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) 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 www_auth_content = conf.realm and fmt('Key realm="%s"', conf.realm) or 'Key' local headers = kong.request.get_headers() local query = kong.request.get_query() local key @@ -160,14 +166,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 @@ -187,8 +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) @@ -211,36 +215,55 @@ local function do_authentication(conf) return true end +local function set_anonymous_consumer(anonymous) + local consumer_cache_key = kong.db.consumers:cache_key(anonymous) + local consumer, err = kong.cache:get(consumer_cache_key, nil, + kong.client.load_consumer, + anonymous, true) + if err then + return error(err) + end -function KeyAuthHandler:access(conf) - -- check if preflight request and whether it should be authenticated - if not conf.run_on_preflight and kong.request.get_method() == "OPTIONS" then + set_consumer(consumer) +end + + +--- When conf.anonymous is enabled we are in "logical OR" authentication flow. +--- Meaning - either anonymous consumer is enabled or there are multiple auth plugins +--- and we need to passthrough on failed authentication. +local function logical_OR_authentication(conf) + if kong.client.get_credential() then + -- we're already authenticated and in "logical OR" between auth methods -- early exit return end - if conf.anonymous and kong.client.get_credential() then - -- we're already authenticated, and we're configured for using anonymous, - -- hence we're in a logical OR between auth methods and we're already done. - return + local ok, _ = do_authentication(conf) + if not ok then + set_anonymous_consumer(conf.anonymous) end +end +--- When conf.anonymous is not set we are in "logical AND" authentication flow. +--- Meaning - if this authentication fails the request should not be authorized +--- even though other auth plugins might have successfully authorized user. +local function logical_AND_authentication(conf) local ok, err = do_authentication(conf) if not ok then - if conf.anonymous then - -- get anonymous user - local consumer_cache_key = kong.db.consumers:cache_key(conf.anonymous) - local consumer, err = kong.cache:get(consumer_cache_key, nil, - kong.client.load_consumer, - conf.anonymous, true) - if err then - return error(err) - end + return kong.response.error(err.status, err.message, err.headers) + end +end - set_consumer(consumer) - else - return kong.response.error(err.status, err.message, err.headers) - end +function KeyAuthHandler:access(conf) + -- check if preflight request and whether it should be authenticated + if not conf.run_on_preflight and kong.request.get_method() == "OPTIONS" then + return + end + + if conf.anonymous then + return logical_OR_authentication(conf) + else + return logical_AND_authentication(conf) end end diff --git a/kong/plugins/key-auth/schema.lua b/kong/plugins/key-auth/schema.lua index 9af6aa2742fd..0dd51e6cbf0f 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 1dead97596fe..1bfc1efcefab 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 324a86fe4f48..9bdba912e43a 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, @@ -628,6 +630,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, @@ -1144,6 +1147,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 4830ab8ce4d9..49f3f3561a1e 100644 --- a/spec/03-plugins/09-key-auth/02-access_spec.lua +++ b/spec/03-plugins/09-key-auth/02-access_spec.lua @@ -1,6 +1,5 @@ local helpers = require "spec.helpers" local cjson = require "cjson" -local meta = require "kong.meta" local utils = require "kong.tools.utils" local http_mock = require "spec.helpers.http_mock" @@ -97,6 +96,7 @@ for _, strategy in helpers.each_strategy() do route = { id = route2.id }, config = { hide_credentials = true, + realm = "test-realm" }, } @@ -243,6 +243,41 @@ for _, strategy in helpers.each_strategy() do assert.not_nil(json) assert.matches("No API key found in request", json.message) end) + 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.test", + ["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 conifgured realm", function() + local res = assert(proxy_client:send { + method = "GET", + path = "/status/200", + headers = { + ["Host"] = "key-auth2.test", + ["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", @@ -255,6 +290,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 { @@ -265,7 +301,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) @@ -292,6 +328,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Unauthorized", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("handles duplicated key in querystring", function() local res = assert(proxy_client:send { @@ -305,6 +342,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) @@ -366,6 +404,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Unauthorized", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) -- lua-multipart doesn't currently handle duplicates at all. @@ -387,6 +426,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 @@ -403,6 +443,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() @@ -417,6 +458,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() @@ -431,6 +473,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) @@ -522,6 +565,7 @@ for _, strategy in helpers.each_strategy() do local json = cjson.decode(body) assert.not_nil(json) assert.matches("Unauthorized", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) res = assert(proxy_client:send { method = "GET", @@ -535,6 +579,7 @@ for _, strategy in helpers.each_strategy() do json = cjson.decode(body) assert.not_nil(json) assert.matches("Unauthorized", json.message) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end) @@ -665,6 +710,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) @@ -844,6 +890,7 @@ for _, strategy in helpers.each_strategy() do } }) assert.response(res).has.status(401) + assert.equal('Basic realm="service"', res.headers["WWW-Authenticate"]) end) it("fails 401, with only the second credential provided", function() @@ -856,6 +903,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() @@ -1280,6 +1328,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 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 1193c85de01b..765a3a4aa77b 100644 --- a/spec/03-plugins/10-basic-auth/03-access_spec.lua +++ b/spec/03-plugins/10-basic-auth/03-access_spec.lua @@ -1,6 +1,5 @@ local helpers = require "spec.helpers" local cjson = require "cjson" -local meta = require "kong.meta" local utils = require "kong.tools.utils" @@ -578,7 +577,7 @@ for _, strategy in helpers.each_strategy() do } }) assert.response(res).has.status(401) - assert.equal('Key realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"]) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) it("fails 401, with no credential provided", function() @@ -590,7 +589,7 @@ for _, strategy in helpers.each_strategy() do } }) assert.response(res).has.status(401) - assert.equal('Key realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"]) + assert.equal('Key', res.headers["WWW-Authenticate"]) end) end)