From 8079343780627ff8c90ca04a18d5a0f8aebee7f7 Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Wed, 1 Jul 2015 17:57:08 +0200 Subject: [PATCH] fix(basicauth) return 401 when missing authorization Former-commit-id: 61223048f2bdf8ec7b23757623e7b3e4a1775cb0 --- kong/plugins/basicauth/access.lua | 3 ++ .../integration/proxy/database_cache_spec.lua | 2 +- spec/integration/proxy/resolver_spec.lua | 2 +- spec/plugins/basicauth/access_spec.lua | 41 ++++++++----------- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/kong/plugins/basicauth/access.lua b/kong/plugins/basicauth/access.lua index dbde710c405a..805fb8fe08c5 100644 --- a/kong/plugins/basicauth/access.lua +++ b/kong/plugins/basicauth/access.lua @@ -41,6 +41,9 @@ local function retrieve_credentials(request, conf) username = basic_parts[1] password = basic_parts[2] end + else + ngx.ctx.stop_phases = true + return responses.send_HTTP_UNAUTHORIZED() end if conf.hide_credentials then diff --git a/spec/integration/proxy/database_cache_spec.lua b/spec/integration/proxy/database_cache_spec.lua index ac7195d12c29..89b0e7504539 100644 --- a/spec/integration/proxy/database_cache_spec.lua +++ b/spec/integration/proxy/database_cache_spec.lua @@ -43,7 +43,7 @@ describe("Database cache", function() os.execute("sleep "..tonumber(5)) local _, status = http_client.get(spec_helper.PROXY_URL.."/get", {}, {host = "cache.test"}) - assert.are.equal(403, status) + assert.are.equal(401, status) -- Create a consumer and a key will make it work again local consumer, err = env.dao_factory.consumers:insert { username = "john" } diff --git a/spec/integration/proxy/resolver_spec.lua b/spec/integration/proxy/resolver_spec.lua index d6b9e56eddbf..3b4bd4443b1f 100644 --- a/spec/integration/proxy/resolver_spec.lua +++ b/spec/integration/proxy/resolver_spec.lua @@ -165,7 +165,7 @@ describe("Resolver", function() it("should return the correct Server and no Via header when the request was NOT proxied", function() local _, status, headers = http_client.get(STUB_GET_URL, nil, { host = "mockbin-auth.com"}) - assert.are.equal(403, status) + assert.are.equal(401, status) assert.are.equal(constants.NAME.."/"..constants.VERSION, headers.server) assert.falsy(headers.via) end) diff --git a/spec/plugins/basicauth/access_spec.lua b/spec/plugins/basicauth/access_spec.lua index 1e074b8a0358..b71315dda941 100644 --- a/spec/plugins/basicauth/access_spec.lua +++ b/spec/plugins/basicauth/access_spec.lua @@ -11,16 +11,16 @@ describe("Authentication Plugin", function() spec_helper.prepare_db() spec_helper.insert_fixtures { api = { - { name = "tests basicauth", public_dns = "basicauth.com", target_url = "http://mockbin.com" } + {name = "tests basicauth", public_dns = "basicauth.com", target_url = "http://mockbin.com"} }, consumer = { - { username = "basicauth_tests_consuser" } + {username = "basicauth_tests_consuser"} }, plugin_configuration = { - { name = "basicauth", value = {}, __api = 1 } + {name = "basicauth", value = {}, __api = 1} }, basicauth_credential = { - { username = "username", password = "password", __consumer = 1 } + {username = "username", password = "password", __consumer = 1} } } @@ -36,50 +36,43 @@ describe("Authentication Plugin", function() it("should return invalid credentials when the credential value is wrong", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization = "asd"}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Invalid authentication credentials", body.message) + assert.equal(403, status) + assert.equal("Invalid authentication credentials", body.message) end) it("should not pass when passing only the password", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization = "Basic OmFwaWtleTEyMw=="}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Invalid authentication credentials", body.message) + assert.equal(403, status) + assert.equal("Invalid authentication credentials", body.message) end) it("should not pass when passing only the username", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization = "Basic dXNlcjEyMzo="}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Invalid authentication credentials", body.message) + assert.equal(403, status) + assert.equal("Invalid authentication credentials", body.message) end) - it("should return invalid credentials when the credential parameter name is wrong in GET", function() + it("should reply 401 when authorization is missing", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization123 = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Invalid authentication credentials", body.message) - end) - - it("should return invalid credentials when the credential parameter name is wrong in POST", function() - local response, status = http_client.post(STUB_POST_URL, {}, {host = "basicauth.com", authorization123 = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) - local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Invalid authentication credentials", body.message) + assert.equal(401, status) + assert.equal("Unauthorized", body.message) end) it("should pass with GET", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) - assert.are.equal(200, status) + assert.equal(200, status) local parsed_response = cjson.decode(response) - assert.are.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers.authorization) + assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers.authorization) end) it("should pass with POST", function() local response, status = http_client.post(STUB_POST_URL, {}, {host = "basicauth.com", authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) - assert.are.equal(200, status) + assert.equal(200, status) local parsed_response = cjson.decode(response) - assert.are.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers.authorization) + assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers.authorization) end) end)