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, ldap-auth, key-auth, jwt, hmac-auth) status code for unauthorized requests #4238

Merged
merged 2 commits into from
Feb 14, 2019
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
2 changes: 1 addition & 1 deletion kong/plugins/basic-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ local function do_authentication(conf)
end

if not credential or not validate_credentials(credential, given_password) then
return false, { status = 403, message = "Invalid authentication credentials" }
return false, { status = 401, message = "Invalid authentication credentials" }
end

-- Retrieve consumer
Expand Down
10 changes: 5 additions & 5 deletions kong/plugins/hmac-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ local function do_authentication(conf)
if not (validate_clock_skew(X_DATE, conf.clock_skew) or
validate_clock_skew(DATE, conf.clock_skew)) then
return false, {
status = 403,
status = 401,
message = "HMAC signature cannot be verified, a valid date or " ..
"x-date header is required for HMAC Authentication"
}
Expand All @@ -322,26 +322,26 @@ local function do_authentication(conf)
local ok, err = validate_params(hmac_params, conf)
if not ok then
kong.log.debug(err)
return false, { status = 403, message = SIGNATURE_NOT_VALID }
return false, { status = 401, message = SIGNATURE_NOT_VALID }
end

-- validate signature
local credential = load_credential(hmac_params.username)
if not credential then
kong.log.debug("failed to retrieve credential for ", hmac_params.username)
return false, { status = 403, message = SIGNATURE_NOT_VALID }
return false, { status = 401, message = SIGNATURE_NOT_VALID }
end

hmac_params.secret = credential.secret

if not validate_signature(hmac_params) then
return false, { status = 403, message = SIGNATURE_NOT_SAME }
return false, { status = 401, message = SIGNATURE_NOT_SAME }
end

-- If request body validation is enabled, then verify digest.
if conf.validate_request_body and not validate_body() then
kong.log.debug("digest validation failed")
return false, { status = 403, message = SIGNATURE_NOT_SAME }
return false, { status = 401, message = SIGNATURE_NOT_SAME }
end

-- Retrieve consumer
Expand Down
12 changes: 6 additions & 6 deletions kong/plugins/jwt/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ local function do_authentication(conf)
end

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

local algorithm = jwt_secret.algorithm or "HS256"

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

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

if not jwt_secret_value then
return false, { status = 403, message = "Invalid key/secret" }
return false, { status = 401, message = "Invalid key/secret" }
end

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

-- Verify the JWT registered claims
Expand All @@ -207,7 +207,7 @@ local function do_authentication(conf)
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 = 403, errors = errors }
return false, { status = 401, errors = errors }
end
end

Expand All @@ -224,7 +224,7 @@ local function do_authentication(conf)
-- However this should not happen
if not consumer then
return false, {
status = 403,
status = 401,
message = fmt("Could not find consumer for '%s=%s'", conf.key_claim_name, jwt_secret_key)
}
end
Expand Down
4 changes: 2 additions & 2 deletions kong/plugins/key-auth/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ local function do_authentication(conf)
return kong.response.exit(500, "An unexpected error occurred")
end

-- no credential in DB, for this key, it is invalid, HTTP 403
-- no credential in DB, for this key, it is invalid, HTTP 401
if not credential then
return nil, { status = 403, message = "Invalid authentication credentials" }
return nil, { status = 401, message = "Invalid authentication credentials" }
end

-----------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion kong/plugins/ldap-auth/access.lua
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ local function do_authentication(conf)
end

if not is_authorized then
return false, {status = 403, message = "Invalid authentication credentials" }
return false, {status = 401, message = "Invalid authentication credentials" }
end

if conf.hide_credentials then
Expand Down
12 changes: 6 additions & 6 deletions spec/03-plugins/09-key-auth/02-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -193,15 +193,15 @@ for _, strategy in helpers.each_strategy() do
})
assert.res_status(200, res)
end)
it("returns 403 Forbidden on invalid key", function()
it("returns 401 Unauthorized on invalid key", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200?apikey=123",
headers = {
["Host"] = "key-auth1.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)
Expand Down Expand Up @@ -235,7 +235,7 @@ for _, strategy in helpers.each_strategy() do
})
assert.res_status(200, res)
end)
it("returns 403 Forbidden on invalid key", function()
it("returns 401 Unauthorized on invalid key", function()
local res = assert(proxy_client:send {
path = "/status/200",
headers = {
Expand All @@ -246,7 +246,7 @@ for _, strategy in helpers.each_strategy() do
apikey = "123",
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)
Expand Down Expand Up @@ -287,7 +287,7 @@ for _, strategy in helpers.each_strategy() do
})
assert.res_status(200, res)
end)
it("returns 403 Forbidden on invalid key", function()
it("returns 401 Unauthorized on invalid key", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
Expand All @@ -296,7 +296,7 @@ for _, strategy in helpers.each_strategy() do
["apikey"] = "123"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)
Expand Down
6 changes: 3 additions & 3 deletions spec/03-plugins/09-key-auth/03-invalidations_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ for _, strategy in helpers.each_strategy() do
["apikey"] = "kong"
}
})
assert.res_status(403, res)
assert.res_status(401, res)
end)

it("invalidates credentials from cache when deleted", function()
Expand Down Expand Up @@ -147,7 +147,7 @@ for _, strategy in helpers.each_strategy() do
["apikey"] = "kong"
}
})
assert.res_status(403, res)
assert.res_status(401, res)
end)

it("invalidated credentials from cache when updated", function()
Expand Down Expand Up @@ -202,7 +202,7 @@ for _, strategy in helpers.each_strategy() do
["apikey"] = "kong"
}
})
assert.res_status(403, res)
assert.res_status(401, res)

res = assert(proxy_client:send {
method = "GET",
Expand Down
22 changes: 11 additions & 11 deletions spec/03-plugins/10-basic-auth/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ for _, strategy in helpers.each_strategy() do

end)

describe("Forbidden", function()
describe("Unauthorized", function()

it("returns 403 Forbidden on invalid credentials in Authorization", function()
it("returns 401 Unauthorized on invalid credentials in Authorization", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
Expand All @@ -145,12 +145,12 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth1.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)

it("returns 403 Forbidden on invalid credentials in Proxy-Authorization", function()
it("returns 401 Unauthorized on invalid credentials in Proxy-Authorization", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
Expand All @@ -159,12 +159,12 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth1.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)

it("returns 403 Forbidden on password only", function()
it("returns 401 Unauthorized on password only", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
Expand All @@ -173,12 +173,12 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth1.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)

it("returns 403 Forbidden on username only", function()
it("returns 401 Unauthorized on username only", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
Expand All @@ -187,7 +187,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth1.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)
Expand Down Expand Up @@ -230,7 +230,7 @@ for _, strategy in helpers.each_strategy() do
assert.equal("bob", body.headers["x-consumer-username"])
end)

it("returns 403 for valid Base64 encoding", function()
it("returns 401 for valid Base64 encoding", function()
local res = assert(proxy_client:send {
method = "GET",
path = "/status/200",
Expand All @@ -239,7 +239,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth1.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid authentication credentials" }, json)
end)
Expand Down
6 changes: 3 additions & 3 deletions spec/03-plugins/10-basic-auth/04-invalidations_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth.com"
}
})
assert.res_status(403, res)
assert.res_status(401, res)
end)

it("invalidates credentials from cache when deleted", function()
Expand Down Expand Up @@ -168,7 +168,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth.com"
}
})
assert.res_status(403, res)
assert.res_status(401, res)
end)

it("invalidated credentials from cache when updated", function()
Expand Down Expand Up @@ -225,7 +225,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "basic-auth.com"
}
})
assert.res_status(403, res)
assert.res_status(401, res)

res = assert(proxy_client:send {
method = "GET",
Expand Down
20 changes: 10 additions & 10 deletions spec/03-plugins/16-jwt/03-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ for _, strategy in helpers.each_strategy() do
local json = cjson.decode(body)
assert.same({ message = "No mandatory 'iss' in claims" }, json)
end)
it("returns 403 Forbidden if the iss does not match a credential", function()
it("returns 401 Unauthorized if the iss does not match a credential", function()
PAYLOAD.iss = "123456789"
local jwt = jwt_encoder.encode(PAYLOAD, jwt_secret.secret)
local authorization = "Bearer " .. jwt
Expand All @@ -227,11 +227,11 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "jwt1.com",
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "No credentials found for given 'iss'" }, json)
end)
it("returns 403 Forbidden if the signature is invalid", function()
it("returns 401 Unauthorized if the signature is invalid", function()
PAYLOAD.iss = jwt_secret.key
local jwt = jwt_encoder.encode(PAYLOAD, "foo")
local authorization = "Bearer " .. jwt
Expand All @@ -243,11 +243,11 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "jwt1.com",
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid signature" }, json)
end)
it("returns 403 Forbidden if the alg does not match the credential", function()
it("returns 401 Unauthorized if the alg does not match the credential", function()
local header = {typ = "JWT", alg = 'RS256'}
local jwt = jwt_encoder.encode(PAYLOAD, jwt_secret.secret, 'HS256', header)
local authorization = "Bearer " .. jwt
Expand All @@ -259,7 +259,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "jwt1.com",
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "Invalid algorithm" }, json)
end)
Expand All @@ -284,7 +284,7 @@ for _, strategy in helpers.each_strategy() do
local body = assert.res_status(401, res)
assert.equal([[{"message":"Unauthorized"}]], body)
end)
it("returns 403 if the token exceeds the maximum allowed expiration limit", function()
it("returns 401 if the token exceeds the maximum allowed expiration limit", function()
local payload = {
iss = jwt_secret.key,
exp = os.time() + 3600,
Expand All @@ -298,7 +298,7 @@ for _, strategy in helpers.each_strategy() do
["Host"] = "jwt11.com"
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
assert.equal('{"exp":"exceeds maximum allowed expiration"}', body)
end)
it("accepts a JWT token within the maximum allowed expiration limit", function()
Expand Down Expand Up @@ -423,7 +423,7 @@ for _, strategy in helpers.each_strategy() do
})
assert.res_status(200, res)
end)
it("returns 403 if the JWT found in the cookie does not match a credential", function()
it("returns 401 if the JWT found in the cookie does not match a credential", function()
PAYLOAD.iss = "incorrect-issuer"
local jwt = jwt_encoder.encode(PAYLOAD, jwt_secret.secret)
local res = assert(proxy_client:send {
Expand All @@ -434,7 +434,7 @@ for _, strategy in helpers.each_strategy() do
["Cookie"] = "silly=" .. jwt .. "; path=/;domain=.jwt9.com",
}
})
local body = assert.res_status(403, res)
local body = assert.res_status(401, res)
local json = cjson.decode(body)
assert.same({ message = "No credentials found for given 'iss'" }, json)
end)
Expand Down
Loading