Skip to content

Commit

Permalink
feat(key-auth) skip authenticating preflight OPTIONS requests
Browse files Browse the repository at this point in the history
fixes #1292, #1535
  • Loading branch information
Tieske committed Aug 31, 2017
1 parent c5c13ca commit a52e506
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 4 deletions.
10 changes: 10 additions & 0 deletions kong/plugins/key-auth/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ local clear_header = ngx.req.clear_header
local ngx_req_read_body = ngx.req.read_body
local ngx_req_set_body_data = ngx.req.set_body_data
local ngx_encode_args = ngx.encode_args
local get_method = ngx.req.get_method
local type = type

local _realm = 'Key realm="' .. _KONG._NAME .. '"'
Expand Down Expand Up @@ -155,6 +156,15 @@ end
function KeyAuthHandler:access(conf)
KeyAuthHandler.super.access(self)

-- check if preflight request and whether it should be authenticated
if conf.run_on_preflight == false and get_method() == "OPTIONS" then
-- FIXME: the above `== false` test is because existing entries in the db will
-- have `nil` and hence will by default start passing the preflight request
-- This should be fixed by a migration to update the actual entries
-- in the datastore
return
end

if ngx.ctx.authenticated_credential and conf.anonymous ~= "" then
-- we're already authenticated, and we're configured for using anonymous,
-- hence we're in a logical OR between auth methods and we're already done.
Expand Down
4 changes: 4 additions & 0 deletions kong/plugins/key-auth/schema.lua
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,9 @@ return {
type = "boolean",
default = false,
},
run_on_preflight = {
type = "boolean",
default = true,
},
}
}
8 changes: 7 additions & 1 deletion spec/01-unit/007-entities_schemas_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,13 @@ describe("Entities Schemas", function()
-- Insert key-auth, whose config has some default values that should be set
local plugin = {name = "key-auth", api_id = "stub"}
local valid = validate_entity(plugin, plugins_schema, {dao = dao_stub})
assert.same({key_names = {"apikey"}, hide_credentials = false, anonymous = "", key_in_body = false}, plugin.config)
assert.same({
key_names = {"apikey"},
hide_credentials = false,
anonymous = "",
key_in_body = false,
run_on_preflight = true,
}, plugin.config)
assert.is_true(valid)
end)
it("should be valid if no value is specified for a subfield and if the config schema has default as empty array", function()
Expand Down
18 changes: 15 additions & 3 deletions spec/02-integration/03-dao/04-constraints_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ helpers.for_each_dao(function(kong_config)
assert.falsy(err)
assert.is_table(plugin)
assert.equal(api_fixture.id, plugin.api_id)
assert.same({hide_credentials = false, key_names = {"apikey"}, anonymous = "", key_in_body = false,}, plugin.config)
assert.same({
run_on_preflight = true,
hide_credentials = false,
key_names = {"apikey"},
anonymous = "",
key_in_body = false,
}, plugin.config)
end)
it("insert a valid plugin bis", function()
plugin_fixture.api_id = api_fixture.id
Expand All @@ -55,8 +61,14 @@ helpers.for_each_dao(function(kong_config)
assert.falsy(err)
assert.is_table(plugin)
assert.equal(api_fixture.id, plugin.api_id)
assert.same({hide_credentials = false, key_names = {"api-key"}, anonymous = "", key_in_body = false}, plugin.config)
end)
assert.same({
run_on_preflight = true,
hide_credentials = false,
key_names = {"api-key"},
anonymous = "",
key_in_body = false,
}, plugin.config)
end)
describe("unique per API/Consumer", function()
it("API/Plugin", function()
plugin_fixture.api_id = api_fixture.id
Expand Down
35 changes: 35 additions & 0 deletions spec/03-plugins/10-key-auth/02-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,19 @@ describe("Plugin: key-auth (access)", function()
},
})

local api7 = assert(helpers.dao.apis:insert {
name = "api-7",
hosts = { "key-auth7.com" },
upstream_url = "http://mockbin.com",
})
assert(helpers.dao.plugins:insert {
name = "key-auth",
api_id = api7.id,
config = {
run_on_preflight = false,
},
})

assert(helpers.start_kong({
nginx_conf = "spec/fixtures/custom_nginx.template",
}))
Expand All @@ -107,6 +120,28 @@ describe("Plugin: key-auth (access)", function()
end)

describe("Unauthorized", function()
it("returns 200 on OPTIONS requests if run_on_preflight is false", function()
local res = assert(client:send {
method = "OPTIONS",
path = "/status/200",
headers = {
["Host"] = "key-auth7.com"
}
})
assert.res_status(200, res)
end)
it("returns Unauthorized on OPTIONS requests if run_on_preflight is true", function()
local res = assert(client:send {
method = "OPTIONS",
path = "/status/200",
headers = {
["Host"] = "key-auth1.com"
}
})
assert.res_status(401, res)
local body = assert.res_status(401, res)
assert.equal([[{"message":"No API key found in request"}]], body)
end)
it("returns Unauthorized on missing credentials", function()
local res = assert(client:send {
method = "GET",
Expand Down

0 comments on commit a52e506

Please sign in to comment.