fix(ldap): add missing www-authenticate headers

When server returns 401 Unauthorized response it should
return WWW-Authenticate header as well with proper challenge.
Not all ldap-auth 401 responses had this header.

Fix: #7772
KAG-321

changelog/unreleased/kong/ldap_www_authenticate.yml

@@ -0,0 +1,3 @@
+message: "**ldap-auth**: Add WWW-Authenticate headers to all 401 responses."
+type: bugfix
+scope: Plugin

kong/clustering/compat/removed_fields.lua

@@ -150,6 +150,9 @@ return {
     },
     aws_lambda = {
       "empty_arrays_mode",
-    }
+    },
+    ldap_auth = {
+      "realm",
+    },
   },
 }

kong/plugins/ldap-auth/access.lua

@@ -232,25 +232,29 @@ local function set_consumer(consumer, credential)
   end
 end
 
+local function unauthorized(message, authorization_scheme)
+  return {
+    status = 401,
+    message = message,
+    headers = { ["WWW-Authenticate"] = authorization_scheme }
+  }
+end
 
 local function do_authentication(conf)
   local authorization_value = kong.request.get_header(AUTHORIZATION)
   local proxy_authorization_value = kong.request.get_header(PROXY_AUTHORIZATION)
 
+  local scheme = conf.header_type
+  if scheme == "ldap" then
+    -- ensure backwards compatibility (see GH PR #3656)
+    -- TODO: provide migration to capitalize older configurations
+    scheme = upper(scheme)
+  end
+
+  local www_auth_content = conf.realm and fmt('%s realm="%s"', scheme, conf.realm) or scheme
   -- If both headers are missing, return 401
   if not (authorization_value or proxy_authorization_value) then
-    local scheme = conf.header_type
-    if scheme == "ldap" then
-      -- ensure backwards compatibility (see GH PR #3656)
-      -- TODO: provide migration to capitalize older configurations
-      scheme = upper(scheme)
-    end
-
-    return false, {
-      status = 401,
-      message = "Unauthorized",
-      headers = { ["WWW-Authenticate"] = scheme .. ' realm="kong"' }
-    }
+    return false, unauthorized("Unauthorized", www_auth_content)
   end
 
   local is_authorized, credential
@@ -263,7 +267,7 @@ local function do_authentication(conf)
   end
 
   if not is_authorized then
-    return false, {status = 401, message = "Unauthorized" }
+    return false, unauthorized("Unauthorized", www_auth_content)
   end
 
   if conf.hide_credentials then
@@ -277,30 +281,50 @@ local function do_authentication(conf)
 end
 
 
-function _M.execute(conf)
-  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.
+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
+
+  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
 
+  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 _M.execute(conf)
+  if conf.anonymous then
+    return logical_OR_authentication(conf)
+  else
+    return logical_AND_authentication(conf)
   end
 end
 

kong/plugins/ldap-auth/schema.lua

@@ -24,6 +24,7 @@ return {
           { keepalive = { description = "An optional value in milliseconds that defines how long an idle connection to LDAP server will live before being closed.", type = "number", default = 60000 }, },
           { anonymous = { description = "An optional string (consumer UUID or username) value to use as an “anonymous” consumer if authentication fails. If empty (default null), the request fails with an authentication failure `4xx`.", type = "string" }, },
           { header_type = { description = "An optional string to use as part of the Authorization header",  type = "string", default = "ldap" }, },
+          { realm = { description = "When authentication fails the plugin sends `WWW-Authenticate` header with `realm` attribute value.", type = "string", required = false }, },
         },
         entity_checks = {
           { conditional = {

spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua

@@ -659,6 +659,23 @@ describe("CP/DP config compat transformations #" .. strategy, function()
         -- cleanup
         admin.plugins:remove({ id = key_auth.id })
       end)
+
+      it("[ldap-auth] removes realm for versions below 3.8", function()
+        local ldap_auth = admin.plugins:insert {
+          name = "ldap-auth",
+          config = {
+            ldap_host = "localhost",
+            base_dn = "test",
+            attribute = "test",
+            realm = "test",
+          }
+        }
+        local expected_ldap_auth_prior_38 = cycle_aware_deep_copy(ldap_auth)
+        expected_ldap_auth_prior_38.config.realm = nil
+        do_assert(utils.uuid(), "3.7.0", expected_ldap_auth_prior_38)
+        -- cleanup
+        admin.plugins:remove({ id = ldap_auth.id })
+      end)
     end)
 
     describe("compatibility test for response-transformer plugin", function()

spec/03-plugins/20-ldap-auth/01-access_spec.lua

@@ -74,6 +74,10 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             hosts = { "ldap7.test" },
           }
 
+          local route8 = bp.routes:insert {
+            hosts = { "ldap8.test" },
+          }
+
           assert(bp.routes:insert {
             protocols = { "grpc" },
             paths = { "/hello.HelloService/" },
@@ -110,6 +114,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
               attribute        = "uid",
               hide_credentials = true,
               cache_ttl        = 2,
+              realm            = "test-ldap",
             }
           }
 
@@ -177,6 +182,20 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           }
 
+          bp.plugins:insert {
+            route = { id = route8.id },
+            name     = "ldap-auth",
+            config   = {
+              ldap_host = ldap_host_aws,
+              ldap_port = 389,
+              start_tls = ldap_strategy.start_tls,
+              base_dn   = "ou=scientists,dc=ldap,dc=mashape,dc=com",
+              attribute = "uid",
+              header_type = "Basic",
+              realm = "test-ldap",
+            }
+          }
+
           assert(helpers.start_kong({
             database   = strategy,
             nginx_conf = "spec/fixtures/custom_nginx.template",
@@ -211,8 +230,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
-          local value = assert.response(res).has.header("www-authenticate")
-          assert.are.equal('LDAP realm="kong"', value)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
           local json = assert.response(res).has.jsonbody()
           assert.equal("Unauthorized", json.message)
         end)
@@ -249,6 +267,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
           local json = assert.response(res).has.jsonbody()
           assert.equal("Unauthorized", json.message)
         end)
@@ -262,6 +281,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
           local json = assert.response(res).has.jsonbody()
           assert.equal("Unauthorized", json.message)
         end)
@@ -291,7 +311,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
         end)
 
         it("fails if credential type is invalid in post request", function()
-          local r = assert(proxy_client:send {
+          local res = assert(proxy_client:send {
             method = "POST",
             path = "/request",
             body = {},
@@ -301,7 +321,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do
               ["content-type"] = "application/x-www-form-urlencoded",
             }
           })
-          assert.response(r).has.status(401)
+          assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
         end)
         it("passes if credential is valid and starts with space in post request", function()
           local res = assert(proxy_client:send {
@@ -349,6 +370,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
         end)
         it("authorization fails with correct status with wrong very long password", function()
           local res = assert(proxy_client:send {
@@ -360,6 +382,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
         end)
         it("authorization fails if credential has multiple encoded usernames or passwords separated by ':' in get request", function()
           local res = assert(proxy_client:send {
@@ -371,6 +394,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
         end)
         it("does not pass if credential is invalid in get request", function()
           local res = assert(proxy_client:send {
@@ -382,6 +406,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
             }
           })
           assert.response(res).has.status(401)
+          assert.equal('LDAP', res.headers["WWW-Authenticate"])
         end)
         it("does not hide credential sent along with authorization header to upstream server", function()
           local res = assert(proxy_client:send {
@@ -408,6 +433,18 @@ for _, ldap_strategy in pairs(ldap_strategies) do
           assert.response(res).has.status(200)
           assert.request(res).has.no.header("authorization")
         end)
+        it("does not pass if credential is invalid in get request and passes www-authenticate realm information", function()
+          local res = assert(proxy_client:send {
+            method  = "GET",
+            path    = "/request",
+            headers = {
+              host          = "ldap2.test",
+              authorization = "ldap " .. ngx.encode_base64("einstein:wrong_password")
+            }
+          })
+          assert.response(res).has.status(401)
+          assert.equal('LDAP realm="test-ldap"', res.headers["WWW-Authenticate"])
+        end)
         it("passes if custom credential type is given in post request", function()
           local r = assert(proxy_client:send {
             method = "POST",
@@ -432,12 +469,27 @@ for _, ldap_strategy in pairs(ldap_strategies) do
           assert.response(res).has.status(401)
 
           local value = assert.response(res).has.header("www-authenticate")
-          assert.equal('Basic realm="kong"', value)
+          assert.equal('Basic', value)
+          local json = assert.response(res).has.jsonbody()
+          assert.equal("Unauthorized", json.message)
+        end)
+        it("injects conf.header_type in WWW-Authenticate header and realm if provided", function()
+          local res = assert(proxy_client:send {
+            method  = "GET",
+            path    = "/get",
+            headers = {
+              host  = "ldap8.test",
+            }
+          })
+          assert.response(res).has.status(401)
+
+          local value = assert.response(res).has.header("www-authenticate")
+          assert.equal('Basic realm="test-ldap"', value)
           local json = assert.response(res).has.jsonbody()
           assert.equal("Unauthorized", json.message)
         end)
         it("fails if custom credential type is invalid in post request", function()
-          local r = assert(proxy_client:send {
+          local res = assert(proxy_client:send {
             method = "POST",
             path = "/request",
             body = {},
@@ -447,7 +499,8 @@ for _, ldap_strategy in pairs(ldap_strategies) do
               ["content-type"] = "application/x-www-form-urlencoded",
             }
           })
-          assert.response(r).has.status(401)
+          assert.response(res).has.status(401)
+          assert.equal('Basic', res.headers["WWW-Authenticate"])
         end)
         it("passes if credential is valid in get request using global plugin", function()
           local res = assert(proxy_client:send {
@@ -676,6 +729,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
               }
             })
             assert.response(res).has.status(401)
+            assert.equal('LDAP', res.headers["WWW-Authenticate"])
           end)
 
           it("fails 401, with only the second credential provided", function()
@@ -688,6 +742,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
               }
             })
             assert.response(res).has.status(401)
+            assert.equal('Key', res.headers["WWW-Authenticate"])
           end)
 
           it("fails 401, with no credential provided", function()
@@ -699,6 +754,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
               }
             })
             assert.response(res).has.status(401)
+            assert.equal('Key', res.headers["WWW-Authenticate"])
           end)
 
         end)