Skip to content

Commit

Permalink
fix(plugins) correct status code for unauthorized requests (#4238)
Browse files Browse the repository at this point in the history
Fixes status code for unauthorized requests in basic-auth, ldap-auth, key-auth, jwt, basic-auth, hmac-auth.

The appropriate status code when the request is not authenticated
on an endpoint should be 401. Previously it was 403.

Fix #3713
  • Loading branch information
alaminopu authored and hishamhm committed Feb 14, 2019
1 parent c7e9b6d commit 5000dcd
Show file tree
Hide file tree
Showing 16 changed files with 93 additions and 93 deletions.
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

0 comments on commit 5000dcd

Please sign in to comment.