Skip to content

Commit

Permalink
fix(jwt): add missing www-authenticate headers
Browse files Browse the repository at this point in the history
When serve returns 401 Unauthorized response it should
return WWW-Authenticate header as well with proper challenge.
JWT auth was missing this header.

Fix: #7772
KAG-321
  • Loading branch information
nowNick authored and hanshuebner committed Jun 18, 2024
1 parent 755c07c commit a3f5410
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 33 deletions.
3 changes: 3 additions & 0 deletions changelog/unreleased/kong/jwt_www_authenticate.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
message: "**jwt**: Add WWW-Authenticate headers to 401 responses."
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 @@ -157,5 +157,8 @@ return {
hmac_auth = {
"realm",
},
jwt = {
"realm",
},
},
}
90 changes: 58 additions & 32 deletions kong/plugins/jwt/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -146,38 +146,44 @@ local function set_consumer(consumer, credential, token)
kong.ctx.shared.authenticated_jwt_token = token -- TODO: wrap in a PDK function?
end

local function unauthorized(message, www_auth_content, errors)
return { status = 401, message = message, headers = { ["WWW-Authenticate"] = www_auth_content }, errors = errors }
end


local function do_authentication(conf)
local token, err = retrieve_tokens(conf)
if err then
return error(err)
end

local www_authenticate_base = conf.realm and fmt('Bearer realm="%s"', conf.realm) or 'Bearer'
local www_authenticate_with_error = www_authenticate_base .. ' error="invalid_token"'
local token_type = type(token)
if token_type ~= "string" then
if token_type == "nil" then
return false, { status = 401, message = "Unauthorized" }
return false, unauthorized("Unauthorized", www_authenticate_base)
elseif token_type == "table" then
return false, { status = 401, message = "Multiple tokens provided" }
return false, unauthorized("Multiple tokens provided", www_authenticate_with_error)
else
return false, { status = 401, message = "Unrecognizable token" }
return false, unauthorized("Unrecognizable token", www_authenticate_with_error)
end
end

-- Decode token to find out who the consumer is
local jwt, err = jwt_decoder:new(token)
if err then
return false, { status = 401, message = "Bad token; " .. tostring(err) }
return false, unauthorized("Bad token; " .. tostring(err), www_authenticate_with_error)
end

local claims = jwt.claims
local header = jwt.header

local jwt_secret_key = claims[conf.key_claim_name] or header[conf.key_claim_name]
if not jwt_secret_key then
return false, { status = 401, message = "No mandatory '" .. conf.key_claim_name .. "' in claims" }
return false, unauthorized("No mandatory '" .. conf.key_claim_name .. "' in claims", www_authenticate_with_error)
elseif jwt_secret_key == "" then
return false, { status = 401, message = "Invalid '" .. conf.key_claim_name .. "' in claims" }
return false, unauthorized("Invalid '" .. conf.key_claim_name .. "' in claims", www_authenticate_with_error)
end

-- Retrieve the secret
Expand All @@ -189,14 +195,14 @@ local function do_authentication(conf)
end

if not jwt_secret then
return false, { status = 401, message = "No credentials found for given '" .. conf.key_claim_name .. "'" }
return false, unauthorized("No credentials found for given '" .. conf.key_claim_name .. "'", www_authenticate_with_error)
end

local algorithm = jwt_secret.algorithm or "HS256"

-- Verify "alg"
if jwt.header.alg ~= algorithm then
return false, { status = 401, message = "Invalid algorithm" }
return false, unauthorized("Invalid algorithm", www_authenticate_with_error)
end

local jwt_secret_value = algorithm ~= nil and algorithm:sub(1, 2) == "HS" and
Expand All @@ -207,25 +213,25 @@ local function do_authentication(conf)
end

if not jwt_secret_value then
return false, { status = 401, message = "Invalid key/secret" }
return false, unauthorized("Invalid key/secret", www_authenticate_with_error)
end

-- Now verify the JWT signature
if not jwt:verify_signature(jwt_secret_value) then
return false, { status = 401, message = "Invalid signature" }
return false, unauthorized("Invalid signature", www_authenticate_with_error)
end

-- Verify the JWT registered claims
local ok_claims, errors = jwt:verify_registered_claims(conf.claims_to_verify)
if not ok_claims then
return false, { status = 401, errors = errors }
return false, unauthorized(nil, www_authenticate_with_error, errors)
end

-- Verify the JWT registered claims
if conf.maximum_expiration ~= nil and conf.maximum_expiration > 0 then
local ok, errors = jwt:check_maximum_expiration(conf.maximum_expiration)
if not ok then
return false, { status = 401, errors = errors }
return false, unauthorized(nil, www_authenticate_with_error, errors)
end
end

Expand All @@ -252,35 +258,55 @@ local function do_authentication(conf)
end


function JwtHandler:access(conf)
-- check if preflight request and whether it should be authenticated
if not conf.run_on_preflight and kong.request.get_method() == "OPTIONS" then
return
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

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.
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
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
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
return kong.response.exit(err.status, err.errors or { message = err.message }, err.headers)
end
end

set_consumer(consumer)

else
return kong.response.exit(err.status, err.errors or { message = err.message })
end
function JwtHandler:access(conf)
-- check if preflight request and whether it should be authenticated
if not conf.run_on_preflight and kong.request.get_method() == "OPTIONS" then
return
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/jwt/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ return {
elements = { type = "string" },
default = { "authorization" },
}, },
{ realm = { description = "When authentication fails the plugin sends `WWW-Authenticate` header with `realm` attribute value.", type = "string", required = false }, },
},
},
},
Expand Down
14 changes: 14 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 @@ -690,6 +690,20 @@ describe("CP/DP config compat transformations #" .. strategy, function()
-- cleanup
admin.plugins:remove({ id = hmac_auth.id })
end)

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

describe("compatibility test for response-transformer plugin", function()
Expand Down
23 changes: 22 additions & 1 deletion spec/03-plugins/16-jwt/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ for _, strategy in helpers.each_strategy() do
plugins:insert({
name = "jwt",
route = { id = routes[9].id },
config = { cookie_names = { "silly", "crumble" } },
config = {
cookie_names = { "silly", "crumble" },
realm = "test-jwt"
},
})

plugins:insert({
Expand Down Expand Up @@ -298,6 +301,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.res_status(401, res)
assert.equal('Bearer', res.headers["WWW-Authenticate"])
end)
it("returns 401 if the claims do not contain the key to identify a secret", function()
PAYLOAD.iss = nil
Expand All @@ -314,6 +318,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "No mandatory 'iss' in claims" }, json)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("returns 401 if the claims do not contain a valid key to identify a secret", function()
PAYLOAD.iss = ""
Expand All @@ -330,6 +335,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid 'iss' in claims" }, json)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("returns 401 Unauthorized if the iss does not match a credential", function()
PAYLOAD.iss = "123456789"
Expand All @@ -346,6 +352,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "No credentials found for given 'iss'" }, json)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("returns 401 Unauthorized if the signature is invalid", function()
PAYLOAD.iss = jwt_secret.key
Expand All @@ -362,6 +369,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid signature" }, json)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("returns 401 Unauthorized if the alg does not match the credential", function()
local header = {typ = "JWT", alg = 'RS256'}
Expand All @@ -378,6 +386,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid algorithm" }, json)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("returns 200 on OPTIONS requests if run_on_preflight is false", function()
local res = assert(proxy_client:send {
Expand All @@ -399,6 +408,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = assert.res_status(401, res)
assert.equal([[{"message":"Unauthorized"}]], body)
assert.equal('Bearer', res.headers["WWW-Authenticate"])
end)
it("returns 401 if the token exceeds the maximum allowed expiration limit", function()
local payload = {
Expand All @@ -416,6 +426,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = assert.res_status(401, res)
assert.equal('{"exp":"exceeds maximum allowed expiration"}', body)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("accepts a JWT token within the maximum allowed expiration limit", function()
local payload = {
Expand Down Expand Up @@ -456,6 +467,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = cjson.decode(assert.res_status(401, res))
assert.same({ message = "Multiple tokens provided" }, body)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
end)

Expand Down Expand Up @@ -595,6 +607,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "No credentials found for given 'iss'" }, json)
assert.equal('Bearer realm="test-jwt" error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("returns a 401 if the JWT in the cookie is corrupted", function()
PAYLOAD.iss = jwt_secret.key
Expand All @@ -609,6 +622,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = assert.res_status(401, res)
assert.equal([[{"message":"Bad token; invalid JSON"}]], body)
assert.equal('Bearer realm="test-jwt" error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("reports a 200 without cookies but with a JWT token in the Authorization header", function()
PAYLOAD.iss = jwt_secret.key
Expand All @@ -632,6 +646,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.res_status(401, res)
assert.equal('Bearer realm="test-jwt"', res.headers["WWW-Authenticate"])
end)
it("returns 200 without cookies but with a JWT token in the CustomAuthorization header", function()
PAYLOAD.iss = jwt_secret.key
Expand Down Expand Up @@ -1100,6 +1115,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = cjson.decode(assert.res_status(401, res))
assert.same({ nbf="must be a number", exp="must be a number" }, body)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("checks if the fields are valid: `exp` claim", function()
local payload = {
Expand All @@ -1117,6 +1133,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = assert.res_status(401, res)
assert.equal('{"exp":"token expired"}', body)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
it("checks if the fields are valid: `nbf` claim", function()
local payload = {
Expand All @@ -1134,6 +1151,7 @@ for _, strategy in helpers.each_strategy() do
})
local body = assert.res_status(401, res)
assert.equal('{"nbf":"token not valid yet"}', body)
assert.equal('Bearer error="invalid_token"', res.headers["WWW-Authenticate"])
end)
end)

Expand Down Expand Up @@ -1348,6 +1366,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.response(res).has.status(401)
assert.equal('Bearer', res.headers["WWW-Authenticate"])
end)

it("fails 401, with only the second credential provided", function()
Expand All @@ -1360,6 +1379,7 @@ for _, strategy in helpers.each_strategy() do
}
})
assert.response(res).has.status(401)
assert.equal('Key', res.headers["WWW-Authenticate"])
end)

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

end)
Expand Down

1 comment on commit a3f5410

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bazel Build

Docker image available kong/kong:a3f54108623da936048ad413db32f9369b47811a
Artifacts available https://github.com/Kong/kong/actions/runs/9565890344

Please sign in to comment.