Skip to content

Commit

Permalink
fix(basic-auth): add missing www-authenticate headers (#11795)
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 basic auth 401 responses had this header. It also allows
to configure the protected resource realm via plugin config.

Fix: #7772
KAG-321
  • Loading branch information
nowNick authored Jan 22, 2024
1 parent ef87932 commit b04aa72
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 40 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/basic_www_authenticate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: Add missing WWW-Authenticate headers to 401 response in basic auth plugin.
type: bugfix
scope: Plugin
21 changes: 7 additions & 14 deletions kong/plugins/basic-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ local HEADERS_CREDENTIAL_IDENTIFIER = constants.HEADERS.CREDENTIAL_IDENTIFIER
local HEADERS_ANONYMOUS = constants.HEADERS.ANONYMOUS


local realm = 'Basic realm="' .. _KONG._NAME .. '"'


local _M = {}


Expand Down Expand Up @@ -154,21 +151,17 @@ local function set_consumer(consumer, credential)
end


local function fail_authentication()
return false, { status = 401, message = "Invalid authentication credentials" }
local function unauthorized(message, www_auth_content)
return { status = 401, message = message, headers = { ["WWW-Authenticate"] = www_auth_content } }
end


local function do_authentication(conf)
local www_authenticate = "Basic realm=\"" .. conf.realm .. "\""

-- If both headers are missing, return 401
if not (kong.request.get_header("authorization") or kong.request.get_header("proxy-authorization")) then
return false, {
status = 401,
message = "Unauthorized",
headers = {
["WWW-Authenticate"] = realm
}
}
return false, unauthorized("Unauthorized", www_authenticate)
end

local credential
Expand All @@ -183,12 +176,12 @@ local function do_authentication(conf)
if given_username and given_password then
credential = load_credential_from_db(given_username)
else
return fail_authentication()
return false, unauthorized("Invalid authentication credentials", www_authenticate)
end
end

if not credential or not validate_credentials(credential, given_password) then
return fail_authentication()
return false, unauthorized("Invalid authentication credentials", www_authenticate)
end

-- Retrieve consumer
Expand Down
1 change: 1 addition & 0 deletions kong/plugins/basic-auth/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ return {
fields = {
{ 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 will fail with an authentication failure `4xx`. Please note that this value must refer to the Consumer `id` or `username` attribute, and **not** its `custom_id`.", type = "string" }, },
{ hide_credentials = { description = "An optional boolean value telling the plugin to show or hide the credential from the upstream service. If `true`, the plugin will strip the credential from the request (i.e. the `Authorization` header) before proxying it.", type = "boolean", required = true, default = false }, },
{ 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 = true, default = "service" }, },
}, }, },
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ describe("declarative config: process_auto_fields", function()
protocols = { "grpc", "grpcs", "http", "https" },
config = {
hide_credentials = false,
realm = "service",
}
},
{
Expand Down Expand Up @@ -709,6 +710,7 @@ describe("declarative config: process_auto_fields", function()
protocols = { "grpc", "grpcs", "http", "https" },
config = {
hide_credentials = false,
realm = "service",
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,8 @@ describe("declarative config: flatten", function()
plugins = { {
config = {
anonymous = null,
hide_credentials = false
hide_credentials = false,
realm = "service"
},
consumer = null,
created_at = 1234567890,
Expand Down Expand Up @@ -1088,7 +1089,8 @@ describe("declarative config: flatten", function()
plugins = { {
config = {
anonymous = null,
hide_credentials = false
hide_credentials = false,
realm = "service"
},
consumer = null,
created_at = 1234567890,
Expand Down
64 changes: 40 additions & 24 deletions spec/03-plugins/10-basic-auth/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ for _, strategy in helpers.each_strategy() do
bp.plugins:insert {
name = "basic-auth",
route = { id = route1.id },
config = {
realm = "test-realm",
}
}

bp.plugins:insert {
Expand Down Expand Up @@ -132,33 +135,39 @@ for _, strategy in helpers.each_strategy() do
end)

describe("Unauthorized", function()

it("returns Unauthorized on missing credentials", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
headers = {
["Host"] = "basic-auth1.test"
}
})
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Unauthorized", json.message)
describe("when realm is configured", function()
it("returns Unauthorized on missing credentials", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
headers = {
["Host"] = "basic-auth1.test"
}
})
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Unauthorized", json.message)
assert.equal('Basic realm="test-realm"', res.headers["WWW-Authenticate"])
end)
end)

it("returns WWW-Authenticate header on missing credentials", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
headers = {
["Host"] = "basic-auth1.test"
}
})
assert.res_status(401, res)
assert.equal('Basic realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"])
describe("when realm is default", function()
it("returns Unauthorized on missing credentials", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
headers = {
["Host"] = "basic-auth2.test"
}
})
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Unauthorized", json.message)
assert.equal('Basic realm="service"', res.headers["WWW-Authenticate"])
end)
end)

end)

describe("Unauthorized", function()
Expand All @@ -176,6 +185,7 @@ for _, strategy in helpers.each_strategy() do
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.equal('Basic realm="test-realm"', res.headers["WWW-Authenticate"])
end)

it("returns 401 Unauthorized on invalid credentials in Proxy-Authorization", function()
Expand All @@ -191,6 +201,7 @@ for _, strategy in helpers.each_strategy() do
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.equal('Basic realm="test-realm"', res.headers["WWW-Authenticate"])
end)

it("returns 401 Unauthorized on password only", function()
Expand All @@ -206,6 +217,7 @@ for _, strategy in helpers.each_strategy() do
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.equal('Basic realm="test-realm"', res.headers["WWW-Authenticate"])
end)

it("returns 401 Unauthorized on username only", function()
Expand All @@ -221,6 +233,7 @@ for _, strategy in helpers.each_strategy() do
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.equal('Basic realm="test-realm"', res.headers["WWW-Authenticate"])
end)

it("rejects gRPC call without credentials", function()
Expand Down Expand Up @@ -296,6 +309,7 @@ for _, strategy in helpers.each_strategy() do
local json = cjson.decode(body)
assert.not_nil(json)
assert.matches("Invalid authentication credentials", json.message)
assert.equal('Basic realm="test-realm"', res.headers["WWW-Authenticate"])
end)

it("authenticates valid credentials in Proxy-Authorization", function()
Expand Down Expand Up @@ -564,6 +578,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.response(res).has.status(401)
assert.equal('Key realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"])
end)

it("fails 401, with no credential provided", function()
Expand All @@ -575,6 +590,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.response(res).has.status(401)
assert.equal('Key realm="' .. meta._NAME .. '"', res.headers["WWW-Authenticate"])
end)

end)
Expand Down
1 change: 1 addition & 0 deletions spec/03-plugins/10-basic-auth/05-declarative_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ for _, strategy in helpers.each_strategy() do
name = "basic-auth",
config = {
hide_credentials = true,
realm = "service",
}
}

Expand Down

0 comments on commit b04aa72

Please sign in to comment.