Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(basic-auth): add missing www-authenticate headers #11795

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",
kikito marked this conversation as resolved.
Show resolved Hide resolved
}
},
{
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
Loading