fix(key-auth): add missing www-authenticate headers

When serve returns 401 Unauthorized response it should
return WWW-Authenticate header as well with proper challenge.
Not all key-auth 401 responses had this header.
This commit also adds an option to configure the realm
for protected resource. By default it is empty therefore it
is not displayed but it can be configured to
be present in www_authenticate header.

Fix: #7772
KAG-321

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

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,44 +207,63 @@ 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)
 
   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
 

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 }, },
         },
     }, },
   },

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,

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,

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