diff --git a/kong/plugins/basicauth/access.lua b/kong/plugins/basicauth/access.lua index 8146aafd0071..7559c09fea56 100644 --- a/kong/plugins/basicauth/access.lua +++ b/kong/plugins/basicauth/access.lua @@ -3,6 +3,9 @@ local stringy = require "stringy" local responses = require "kong.tools.responses" local constants = require "kong.constants" +local AUTHORIZATION = "authorization" +local PROXY_AUTHORIZATION = "proxy-authorization" + local _M = {} local function skip_authentication(headers) @@ -18,9 +21,9 @@ end -- @param {table} conf Plugin configuration (value property) -- @return {string} public_key -- @return {string} private_key -local function retrieve_credentials(request, conf) +local function retrieve_credentials(request, header_name, conf) local username, password - local authorization_header = request.get_headers()["authorization"] + local authorization_header = request.get_headers()[header_name] if authorization_header then local iterator, iter_err = ngx.re.gmatch(authorization_header, "\\s*[Bb]asic\\s*(.+)") @@ -43,13 +46,10 @@ local function retrieve_credentials(request, conf) password = basic_parts[2] end end - else - ngx.ctx.stop_phases = true - return responses.send_HTTP_UNAUTHORIZED() end if conf.hide_credentials then - request.clear_header("authorization") + request.clear_header(header_name) end return username, password @@ -70,14 +70,9 @@ local function validate_credentials(credential, username, password) end end -function _M.execute(conf) - if skip_authentication(ngx.req.get_headers()) then return end - - local username, password = retrieve_credentials(ngx.req, conf) +local function load_credential(username) local credential - - -- Make sure we are not sending an empty table to find_by_keys - if username then + if username then credential = cache.get_or_set(cache.basicauth_credential_key(username), function() local credentials, err = dao.basicauth_credentials:find_by_keys { username = username } local result @@ -90,6 +85,30 @@ function _M.execute(conf) end) end + return credential +end + +function _M.execute(conf) + if skip_authentication(ngx.req.get_headers()) then return end + + -- If both headers are missing, return 401 + if not (ngx.req.get_headers()[AUTHORIZATION] or ngx.req.get_headers()[PROXY_AUTHORIZATION]) then + ngx.ctx.stop_phases = true + return responses.send_HTTP_UNAUTHORIZED() + end + + local credential + local username, password = retrieve_credentials(ngx.req, PROXY_AUTHORIZATION, conf) + if username then + credential = load_credential(username) + end + + -- Try with the authorization header + if not credential then + username, password = retrieve_credentials(ngx.req, AUTHORIZATION, conf) + credential = load_credential(username) + end + if not validate_credentials(credential, username, password) then ngx.ctx.stop_phases = true -- interrupt other phases of this request return responses.send_HTTP_FORBIDDEN("Invalid authentication credentials") @@ -104,6 +123,9 @@ function _M.execute(conf) return result end) + local inspect = require "inspect" + print(inspect(ngx.req.get_headers())) + ngx.req.set_header(constants.HEADERS.CONSUMER_ID, consumer.id) ngx.req.set_header(constants.HEADERS.CONSUMER_CUSTOM_ID, consumer.custom_id) ngx.req.set_header(constants.HEADERS.CONSUMER_USERNAME, consumer.username) diff --git a/kong/plugins/keyauth/access.lua b/kong/plugins/keyauth/access.lua index dd82d9165cb9..dd58f60244ef 100644 --- a/kong/plugins/keyauth/access.lua +++ b/kong/plugins/keyauth/access.lua @@ -140,7 +140,7 @@ function _M.execute(conf) -- No key found in the request's headers or parameters if not key_found then ngx.ctx.stop_phases = true - return responses.send_HTTP_UNAUTHORIZED("No API key found in headers or querystring") + return responses.send_HTTP_UNAUTHORIZED("No API Key found in headers, body or querystring") end -- No key found in the DB, this credential is invalid diff --git a/spec/plugins/basicauth/access_spec.lua b/spec/plugins/basicauth/access_spec.lua index b71315dda941..dfc04faea7b7 100644 --- a/spec/plugins/basicauth/access_spec.lua +++ b/spec/plugins/basicauth/access_spec.lua @@ -11,7 +11,7 @@ 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.org"} }, consumer = { {username = "basicauth_tests_consuser"} @@ -33,6 +33,13 @@ describe("Authentication Plugin", function() describe("Basic Authentication", function() + it("should return invalid credentials when the credential is missing", function() + local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com"}) + local body = cjson.decode(response) + assert.equal(401, status) + assert.equal("Unauthorized", body.message) + end) + 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) @@ -40,6 +47,14 @@ describe("Authentication Plugin", function() assert.equal("Invalid authentication credentials", body.message) end) + + it("should return invalid credentials when the credential value is wrong in proxy-authorization", function() + local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", ["proxy-authorization"] = "asd"}) + local body = cjson.decode(response) + 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) @@ -62,12 +77,20 @@ describe("Authentication Plugin", function() end) it("should pass with GET", function() + print(STUB_GET_URL) local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) assert.equal(200, status) local parsed_response = cjson.decode(response) assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers.authorization) end) + it("should pass with GET and proxy-authorization", function() + local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", ["proxy-authorization"] = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) + assert.equal(200, status) + local parsed_response = cjson.decode(response) + assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers["proxy-authorization"]) + end) + it("should pass with POST", function() local response, status = http_client.post(STUB_POST_URL, {}, {host = "basicauth.com", authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) assert.equal(200, status) @@ -75,5 +98,19 @@ describe("Authentication Plugin", function() assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers.authorization) end) + it("should pass with GET and valid authorization and wrong proxy-authorization", function() + local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", ["proxy-authorization"] = "hello", authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) + assert.equal(200, status) + local parsed_response = cjson.decode(response) + assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers["proxy-authorization"]) + end) + + it("should pass with GET and invalid authorization and valid proxy-authorization", function() + local response, status = http_client.get(STUB_GET_URL, {}, {host = "basicauth.com", authorization = "hello", ["proxy-authorization"] = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="}) + assert.equal(200, status) + local parsed_response = cjson.decode(response) + assert.equal("Basic dXNlcm5hbWU6cGFzc3dvcmQ=", parsed_response.headers["proxy-authorization"]) + end) + end) end) diff --git a/spec/plugins/keyauth/access_spec.lua b/spec/plugins/keyauth/access_spec.lua index c3391b2195bf..e6b4b081bc0e 100644 --- a/spec/plugins/keyauth/access_spec.lua +++ b/spec/plugins/keyauth/access_spec.lua @@ -35,6 +35,13 @@ describe("Authentication Plugin", function() describe("Query Authentication", function() + it("should return invalid credentials when the credential is missing", function() + local response, status = http_client.get(STUB_GET_URL, {}, {host = "keyauth1.com"}) + local body = cjson.decode(response) + assert.equal(401, status) + assert.equal("No API Key found in headers, body or querystring", body.message) + end) + it("should return invalid credentials when the credential value is wrong", function() local response, status = http_client.get(STUB_GET_URL, {apikey = "asd"}, {host = "keyauth1.com"}) local body = cjson.decode(response) @@ -46,21 +53,21 @@ describe("Authentication Plugin", function() local response, status = http_client.get(STUB_GET_URL, {apikey123 = "apikey123"}, {host = "keyauth1.com"}) local body = cjson.decode(response) assert.equal(401, status) - assert.equal("No API key found in headers or querystring", body.message) + assert.equal("No API Key found in headers, body or querystring", body.message) end) it("should reply 401 when the credential parameter name is wrong in GET", function() local response, status = http_client.get(STUB_GET_URL, {apikey123 = "apikey123"}, {host = "keyauth1.com"}) local body = cjson.decode(response) assert.equal(401, status) - assert.equal("No API key found in headers or querystring", body.message) + assert.equal("No API Key found in headers, body or querystring", body.message) end) it("should reply 401 when the credential parameter name is wrong in POST", function() local response, status = http_client.post(STUB_POST_URL, {apikey123 = "apikey123"}, {host = "keyauth1.com"}) local body = cjson.decode(response) assert.equal(401, status) - assert.equal("No API key found in headers or querystring", body.message) + assert.equal("No API Key found in headers, body or querystring", body.message) end) it("should pass with GET", function() @@ -81,14 +88,14 @@ describe("Authentication Plugin", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "keyauth1.com", apikey123 = "apikey123"}) local body = cjson.decode(response) assert.equal(401, status) - assert.equal("No API key found in headers or querystring", body.message) + assert.equal("No API Key found in headers, body or querystring", body.message) end) it("should reply 401 when the credential parameter name is wrong in POST header", function() local response, status = http_client.post(STUB_POST_URL, {}, {host = "keyauth1.com", apikey123 = "apikey123"}) local body = cjson.decode(response) assert.equal(401, status) - assert.equal("No API key found in headers or querystring", body.message) + assert.equal("No API Key found in headers, body or querystring", body.message) end) it("should set right headers", function()