Skip to content

Commit

Permalink
fix(oauth2): 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 oauth2 401 responses had this header.

Fix: #7772
KAG-321
  • Loading branch information
nowNick committed Jun 18, 2024
1 parent a3f5410 commit 5b40a68
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 50 deletions.
4 changes: 4 additions & 0 deletions changelog/unreleased/kong/oauth2_www_authenticate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: "**OAuth2**: Add WWW-Authenticate headers to all 401 responses and realm option."
type: bugfix
scope: Plugin

3 changes: 3 additions & 0 deletions kong/clustering/compat/removed_fields.lua
Original file line number Diff line number Diff line change
Expand Up @@ -160,5 +160,8 @@ return {
jwt = {
"realm",
},
oauth2 = {
"realm",
},
},
}
100 changes: 62 additions & 38 deletions kong/plugins/oauth2/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ local secret = require "kong.plugins.oauth2.secret"

local sha256_base64url = require "kong.tools.sha256".sha256_base64url


local fmt = string.format
local kong = kong
local type = type
local next = next
Expand Down Expand Up @@ -811,7 +811,7 @@ local function load_token(access_token)
end


local function retrieve_token(conf, access_token)
local function retrieve_token(conf, access_token, realm)
local token_cache_key = kong.db.oauth2_tokens:cache_key(access_token)
local token, err = kong.cache:get(token_cache_key, nil, load_token, access_token)
if err then
Expand All @@ -827,6 +827,11 @@ local function retrieve_token(conf, access_token)
[ERROR] = "invalid_token",
error_description = "The access token is global, but the current " ..
"plugin is configured without 'global_credentials'",
},
{
["WWW-Authenticate"] = 'Bearer' .. realm .. ' error=' ..
'"invalid_token" error_description=' ..
'"The access token is invalid or has expired"'
})
end

Expand Down Expand Up @@ -951,6 +956,7 @@ end

local function do_authentication(conf)
local access_token = parse_access_token(conf);
local realm = conf.realm and fmt(' realm="%s"', conf.realm) or ''
if not access_token or access_token == "" then
return nil, {
status = 401,
Expand All @@ -959,12 +965,12 @@ local function do_authentication(conf)
error_description = "The access token is missing"
},
headers = {
["WWW-Authenticate"] = 'Bearer realm="service"'
["WWW-Authenticate"] = 'Bearer' .. realm
}
}
end

local token = retrieve_token(conf, access_token)
local token = retrieve_token(conf, access_token, realm)
if not token then
return nil, {
status = 401,
Expand All @@ -973,7 +979,7 @@ local function do_authentication(conf)
error_description = "The access token is invalid or has expired"
},
headers = {
["WWW-Authenticate"] = 'Bearer realm="service" error=' ..
["WWW-Authenticate"] = 'Bearer' .. realm .. ' error=' ..
'"invalid_token" error_description=' ..
'"The access token is invalid or has expired"'
}
Expand All @@ -991,7 +997,7 @@ local function do_authentication(conf)
error_description = "The access token is invalid or has expired"
},
headers = {
["WWW-Authenticate"] = 'Bearer realm="service" error=' ..
["WWW-Authenticate"] = 'Bearer' .. realm .. ' error=' ..
'"invalid_token" error_description=' ..
'"The access token is invalid or has expired"'
}
Expand All @@ -1009,7 +1015,7 @@ local function do_authentication(conf)
error_description = "The access token is invalid or has expired"
},
headers = {
["WWW-Authenticate"] = 'Bearer realm="service" error=' ..
["WWW-Authenticate"] = 'Bearer' .. realm .. ' error=' ..
'"invalid_token" error_description=' ..
'"The access token is invalid or has expired"'
}
Expand Down Expand Up @@ -1043,7 +1049,7 @@ local function do_authentication(conf)
return true
end

local function invalid_oauth2_method(endpoint_name)
local function invalid_oauth2_method(endpoint_name, realm)
return {
status = 405,
message = {
Expand All @@ -1053,7 +1059,7 @@ local function invalid_oauth2_method(endpoint_name)
" is invalid for the " .. endpoint_name .. " endpoint"
},
headers = {
["WWW-Authenticate"] = 'Bearer realm="service" error=' ..
["WWW-Authenticate"] = 'Bearer' .. realm .. ' error=' ..
'"invalid_method" error_description=' ..
'"The HTTP method ' .. kong.request.get_method()
.. ' is invalid for the ' ..
Expand All @@ -1062,13 +1068,54 @@ local function invalid_oauth2_method(endpoint_name)
}
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

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
local clear_header = kong.service.request.clear_header
clear_header("X-Authenticated-Scope")
clear_header("X-Authenticated-UserId")
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
return kong.response.exit(err.status, err.message, err.headers)
end
end

function _M.execute(conf)
local path = kong.request.get_path()
local has_end_slash = string_byte(path, -1) == SLASH

local realm = conf.realm and fmt(' realm="%s"', conf.realm) or ''
if string_find(path, "/oauth2/token", has_end_slash and -14 or -13, true) then
if kong.request.get_method() ~= "POST" then
local err = invalid_oauth2_method("token")
local err = invalid_oauth2_method("token", realm)
return kong.response.exit(err.status, err.message, err.headers)
end

Expand All @@ -1077,40 +1124,17 @@ function _M.execute(conf)

if string_find(path, "/oauth2/authorize", has_end_slash and -18 or -17, true) then
if kong.request.get_method() ~= "POST" then
local err = invalid_oauth2_method("authorization")
local err = invalid_oauth2_method("authorization", realm)
return kong.response.exit(err.status, err.message, err.headers)
end

return authorize(conf)
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.
local clear_header = kong.service.request.clear_header
clear_header("X-Authenticated-Scope")
clear_header("X-Authenticated-UserId")
return
end


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

set_consumer(consumer)

else
return kong.response.exit(err.status, err.message, err.headers)
end
if conf.anonymous then
return logical_OR_authentication(conf)
else
return logical_AND_authentication(conf)
end
end

Expand Down
1 change: 1 addition & 0 deletions kong/plugins/oauth2/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ return {
{ refresh_token_ttl = typedefs.ttl { default = 1209600, required = true }, },
{ reuse_refresh_token = { description = "An optional boolean value that indicates whether an OAuth refresh token is reused when refreshing an access token.", type = "boolean", default = false, required = true }, },
{ pkce = { description = "Specifies a mode of how the Proof Key for Code Exchange (PKCE) should be handled by the plugin.", type = "string", default = "lax", required = false, one_of = { "none", "lax", "strict" } }, },
{ realm = { description = "When authentication fails the plugin sends `WWW-Authenticate` header with `realm` attribute value.", type = "string", required = false }, },
},
custom_validator = validate_flows,
entity_checks = {
Expand Down
15 changes: 15 additions & 0 deletions spec/02-integration/09-hybrid_mode/09-config-compat_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,21 @@ describe("CP/DP config compat transformations #" .. strategy, function()
-- cleanup
admin.plugins:remove({ id = jwt.id })
end)

it("[oauth2] removes realm for versions below 3.8", function()
local oauth2 = admin.plugins:insert {
name = "oauth2",
config = {
enable_password_grant = true,
realm = "test",
}
}
local expected_oauth2_prior_38 = cycle_aware_deep_copy(oauth2)
expected_oauth2_prior_38.config.realm = nil
do_assert(uuid(), "3.7.0", expected_oauth2_prior_38)
-- cleanup
admin.plugins:remove({ id = oauth2.id })
end)
end)

describe("compatibility test for response-transformer plugin", function()
Expand Down
Loading

0 comments on commit 5b40a68

Please sign in to comment.