Skip to content

Commit

Permalink
Adds support for Proxy-Authorization, and closes #460
Browse files Browse the repository at this point in the history
  • Loading branch information
@subnetmarco
subnetmarco committed Aug 13, 2015
1 parent a4cb1e7 commit dbe9310
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 20 deletions.
48 changes: 35 additions & 13 deletions kong/plugins/basicauth/access.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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*(.+)")
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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")
Expand All @@ -104,6 +123,9 @@ function _M.execute(conf)
return result
end)

local inspect = require "inspect"
print(inspect(ngx.req.get_headers()))

This comment has been minimized.

Sign in to view

Copy link
@thibaultcha

thibaultcha Aug 14, 2015

Member

^

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)
Expand Down
2 changes: 1 addition & 1 deletion kong/plugins/keyauth/access.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 38 additions & 1 deletion spec/plugins/basicauth/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand All @@ -33,13 +33,28 @@ 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)
assert.equal(403, status)
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)
Expand All @@ -62,18 +77,40 @@ describe("Authentication Plugin", function()
end)

it("should pass with GET", function()
print(STUB_GET_URL)

This comment has been minimized.

Sign in to view

Copy link
@thibaultcha

thibaultcha Aug 14, 2015

Member

^ bis
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)
local parsed_response = cjson.decode(response)
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)
17 changes: 12 additions & 5 deletions spec/plugins/keyauth/access_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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()
Expand Down

2 comments on commit dbe9310

@thibaultcha
Copy link
Member

Choose a reason for hiding this comment

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

After this some tests are failing:

Failure → spec/plugins/basicauth/access_spec.lua @ 87
Authentication Plugin Basic Authentication should pass with GET and proxy-authorization
spec/plugins/basicauth/access_spec.lua:91: Expected objects to be equal.
Passed in:
(nil)
Expected:
(string) 'Basic dXNlcm5hbWU6cGFzc3dvcmQ='

Failure → spec/plugins/basicauth/access_spec.lua @ 101
Authentication Plugin Basic Authentication should pass with GET and valid authorization and wrong proxy-authorization
spec/plugins/basicauth/access_spec.lua:105: Expected objects to be equal.
Passed in:
(nil)
Expected:
(string) 'Basic dXNlcm5hbWU6cGFzc3dvcmQ='

Failure → spec/plugins/basicauth/access_spec.lua @ 108
Authentication Plugin Basic Authentication should pass with GET and invalid authorization and valid proxy-authorization
spec/plugins/basicauth/access_spec.lua:112: Expected objects to be equal.
Passed in:
(nil)
Expected:
(string) 'Basic dXNlcm5hbWU6cGFzc3dvcmQ='

@subnetmarco
Copy link
Member Author

Choose a reason for hiding this comment

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

Because of a Mockbin bug: Kong/insomnia-mockbin#36 - It works with http://httpbin.org

Once @ahmadnassri fixes the bug, the test will work.

Please sign in to comment.