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

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 fails the plugin sends `WWW-Authenticate` header with `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