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(oauth2): add missing www-authenticate headers #11833

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
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
Loading