Skip to content

Commit

Permalink
fix(ldap): add missing www-authenticate headers
Browse files Browse the repository at this point in the history
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. Also this
is removing the realm part from WWW-Authenticate header
as it is a potential phishing attack vector.

Fix: #7772
KAG-321
  • Loading branch information
nowNick committed Oct 23, 2023
1 parent 920ba98 commit e4743e0
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 20 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/ldap_www_authenticate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Add WWW-Authenticate headers to all 401 response in ldap auth plugin.
type: bugfix
scope: Plugin
29 changes: 16 additions & 13 deletions kong/plugins/ldap-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -232,25 +232,28 @@ 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

-- 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", scheme)
end

local is_authorized, credential
Expand All @@ -263,7 +266,7 @@ local function do_authentication(conf)
end

if not is_authorized then
return false, {status = 401, message = "Invalid authentication credentials" }
return false, unauthorized("Invalid authentication credentials", scheme)
end

if conf.hide_credentials then
Expand Down
24 changes: 17 additions & 7 deletions spec/03-plugins/20-ldap-auth/01-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,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)
Expand Down Expand Up @@ -249,6 +248,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("Invalid authentication credentials", json.message)
end)
Expand All @@ -262,6 +262,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("Invalid authentication credentials", json.message)
end)
Expand Down Expand Up @@ -291,7 +292,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 = {},
Expand All @@ -301,7 +302,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 {
Expand Down Expand Up @@ -349,6 +351,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 {
Expand All @@ -360,6 +363,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 {
Expand All @@ -371,6 +375,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 {
Expand All @@ -382,6 +387,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 {
Expand Down Expand Up @@ -432,12 +438,12 @@ 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("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 = {},
Expand All @@ -447,7 +453,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 {
Expand Down Expand Up @@ -676,6 +683,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()
Expand All @@ -688,6 +696,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
}
})
assert.response(res).has.status(401)
assert.equal('Key realm="kong"', res.headers["WWW-Authenticate"])
end)

it("fails 401, with no credential provided", function()
Expand All @@ -699,6 +708,7 @@ for _, ldap_strategy in pairs(ldap_strategies) do
}
})
assert.response(res).has.status(401)
assert.equal('Key realm="kong"', res.headers["WWW-Authenticate"])
end)

end)
Expand Down

0 comments on commit e4743e0

Please sign in to comment.