diff --git a/kong/dao/schemas_validation.lua b/kong/dao/schemas_validation.lua index dcf887db9a2d..4115f9a359b6 100644 --- a/kong/dao/schemas_validation.lua +++ b/kong/dao/schemas_validation.lua @@ -24,7 +24,9 @@ local custom_types_validation = { return parsed_url and parsed_url.path and parsed_url.host and parsed_url.scheme end end, - ["array"] = function(v) return utils.is_array(v) end + ["array"] = function(v) + return utils.is_array(v) + end } local function validate_type(field_type, value) diff --git a/kong/plugins/ip-restriction/schema.lua b/kong/plugins/ip-restriction/schema.lua index 860dc0a907a2..9c02ba493b28 100644 --- a/kong/plugins/ip-restriction/schema.lua +++ b/kong/plugins/ip-restriction/schema.lua @@ -1,6 +1,5 @@ local iputils = require "resty.iputils" local DaoError = require "kong.dao.error" -local utils = require "kong.tools.utils" local constants = require "kong.constants" local function validate_ips(v, t, column) @@ -9,29 +8,33 @@ local function validate_ips(v, t, column) for _, ip in ipairs(v) do local _, err = iputils.parse_cidr(ip) if type(err) == "string" then -- It's an error only if the second variable is a string - return false, err + return false, "cannot parse '"..ip.."': "..err end end - new_fields = { ["_"..column.."_cache"] = iputils.parse_cidrs(v) } + new_fields = {["_"..column.."_cache"] = iputils.parse_cidrs(v)} end return true, nil, new_fields end return { fields = { - whitelist = { type = "array", func = validate_ips }, - blacklist = { type = "array", func = validate_ips }, + whitelist = {type = "array", func = validate_ips}, + blacklist = {type = "array", func = validate_ips}, -- Internal use - _whitelist_cache = { type = "array" }, - _blacklist_cache = { type = "array" } + _whitelist_cache = {type = "array"}, + _blacklist_cache = {type = "array"} }, self_check = function(schema, plugin_t, dao, is_update) - if utils.table_size(plugin_t.whitelist) > 0 and utils.table_size(plugin_t.blacklist) > 0 then - return false, DaoError("You cannot set both a whitelist and a blacklist", constants.DATABASE_ERROR_TYPES.SCHEMA) - elseif utils.table_size(plugin_t.whitelist) == 0 and utils.table_size(plugin_t.blacklist) == 0 then - return false, DaoError("You must set at least a whitelist or blacklist", constants.DATABASE_ERROR_TYPES.SCHEMA) + local wl = type(plugin_t.whitelist) == "table" and plugin_t.whitelist or {} + local bl = type(plugin_t.blacklist) == "table" and plugin_t.blacklist or {} + + if #wl > 0 and #bl > 0 then + return false, DaoError("you cannot set both a whitelist and a blacklist", constants.DATABASE_ERROR_TYPES.SCHEMA) + elseif #wl == 0 and #bl == 0 then + return false, DaoError("you must set at least a whitelist or blacklist", constants.DATABASE_ERROR_TYPES.SCHEMA) end + return true end } diff --git a/spec/plugins/ip-restriction/access_spec.lua b/spec/plugins/ip-restriction/access_spec.lua index 6e80a3fb48ed..93ce28ee55ae 100644 --- a/spec/plugins/ip-restriction/access_spec.lua +++ b/spec/plugins/ip-restriction/access_spec.lua @@ -5,75 +5,66 @@ local cjson = require "cjson" local STUB_GET_URL = spec_helper.STUB_GET_URL describe("IP Restriction", function() - setup(function() spec_helper.prepare_db() spec_helper.insert_fixtures { api = { - { name = "iprestriction1", request_host = "test1.com", upstream_url = "http://mockbin.com" }, - { name = "iprestriction2", request_host = "test2.com", upstream_url = "http://mockbin.com" }, - { name = "iprestriction3", request_host = "test3.com", upstream_url = "http://mockbin.com" }, - { name = "iprestriction4", request_host = "test4.com", upstream_url = "http://mockbin.com" }, - { name = "iprestriction5", request_host = "test5.com", upstream_url = "http://mockbin.com" }, - { name = "iprestriction6", request_host = "test6.com", upstream_url = "http://mockbin.com" } + {name = "iprestriction1", request_host = "test1.com", upstream_url = "http://mockbin.com"}, + {name = "iprestriction2", request_host = "test2.com", upstream_url = "http://mockbin.com"}, + {name = "iprestriction3", request_host = "test3.com", upstream_url = "http://mockbin.com"}, + {name = "iprestriction4", request_host = "test4.com", upstream_url = "http://mockbin.com"}, + {name = "iprestriction5", request_host = "test5.com", upstream_url = "http://mockbin.com"}, + {name = "iprestriction6", request_host = "test6.com", upstream_url = "http://mockbin.com"}, + {name = "iprestriction7", request_host = "test7.com", upstream_url = "http://mockbin.com"} }, plugin = { - { name = "ip-restriction", config = { blacklist = { "127.0.0.1" }}, __api = 1 }, - { name = "ip-restriction", config = { blacklist = { "127.0.0.2" }}, __api = 2 }, - { name = "ip-restriction", config = { whitelist = { "127.0.0.2" }}, __api = 3 }, - { name = "ip-restriction", config = { whitelist = { "127.0.0.1" }}, __api = 4 }, - { name = "ip-restriction", config = { blacklist = { "127.0.0.0/24" }}, __api = 5 }, - { name = "ip-restriction", config = { whitelist = { "127.0.0.1", "127.0.0.2" }}, __api = 6 }, + {name = "ip-restriction", config = {blacklist = {"127.0.0.1"}}, __api = 1}, + {name = "ip-restriction", config = {blacklist = {"127.0.0.2"}}, __api = 2}, + {name = "ip-restriction", config = {whitelist = {"127.0.0.2"}}, __api = 3}, + {name = "ip-restriction", config = {whitelist = {"127.0.0.1"}}, __api = 4}, + {name = "ip-restriction", config = {blacklist = {"127.0.0.0/24"}}, __api = 5}, + {name = "ip-restriction", config = {whitelist = {"127.0.0.1", "127.0.0.2"}}, __api = 6} } } - spec_helper.start_kong() end) - teardown(function() spec_helper.stop_kong() end) - it("should block request when IP is in blacklist", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "test1.com"}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Your IP address is not allowed", body.message) + assert.equal(403, status) + assert.equal("Your IP address is not allowed", body.message) end) - it("should allow request when IP is not in blacklist", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "test2.com"}) local body = cjson.decode(response) - assert.are.equal(200, status) - assert.are.equal("127.0.0.1", body.clientIPAddress) + assert.equal(200, status) + assert.equal("127.0.0.1", body.clientIPAddress) end) - it("should block request when IP is not in whitelist", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "test3.com"}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Your IP address is not allowed", body.message) + assert.equal(403, status) + assert.equal("Your IP address is not allowed", body.message) end) - it("should allow request when IP is in whitelist", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "test4.com"}) local body = cjson.decode(response) - assert.are.equal(200, status) - assert.are.equal("127.0.0.1", body.clientIPAddress) + assert.equal(200, status) + assert.equal("127.0.0.1", body.clientIPAddress) end) - it("should block request when IP is blacklisted with CIDR", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "test5.com"}) local body = cjson.decode(response) - assert.are.equal(403, status) - assert.are.equal("Your IP address is not allowed", body.message) + assert.equal(403, status) + assert.equal("Your IP address is not allowed", body.message) end) - it("should allow request when IP is in whitelist with another IP", function() local response, status = http_client.get(STUB_GET_URL, {}, {host = "test6.com"}) local body = cjson.decode(response) - assert.are.equal(200, status) - assert.are.equal("127.0.0.1", body.clientIPAddress) + assert.equal(200, status) + assert.equal("127.0.0.1", body.clientIPAddress) end) - end) diff --git a/spec/plugins/ip-restriction/api_spec.lua b/spec/plugins/ip-restriction/api_spec.lua new file mode 100644 index 000000000000..9387acc15a9b --- /dev/null +++ b/spec/plugins/ip-restriction/api_spec.lua @@ -0,0 +1,85 @@ +local schemas_validation = require "kong.dao.schemas_validation" +local schema = require "kong.plugins.ip-restriction.schema" + +local v = schemas_validation.validate_entity + +describe("ip-restriction schema", function() + describe("errors", function() + it("whitelist should not accept invalid types", function() + local t = {whitelist = 12} + local ok, err = v(t, schema) + assert.False(ok) + assert.same({whitelist = "whitelist is not a array"}, err) + end) + it("whitelist should not accept invalid IPs", function() + local t = {whitelist = "hello"} + local ok, err = v(t, schema) + assert.False(ok) + assert.same({whitelist = "cannot parse 'hello': Invalid IP"}, err) + + t = {whitelist = {"127.0.0.1", "127.0.0.2", "hello"}} + ok, err = v(t, schema) + assert.False(ok) + assert.same({whitelist = "cannot parse 'hello': Invalid IP"}, err) + end) + it("blacklist should not accept invalid types", function() + local t = {blacklist = 12} + local ok, err = v(t, schema) + assert.False(ok) + assert.same({blacklist = "blacklist is not a array"}, err) + end) + it("blacklist should not accept invalid IPs", function() + local t = {blacklist = "hello"} + local ok, err = v(t, schema) + assert.False(ok) + assert.same({blacklist = "cannot parse 'hello': Invalid IP"}, err) + + t = {blacklist = {"127.0.0.1", "127.0.0.2", "hello"}} + ok, err = v(t, schema) + assert.False(ok) + assert.same({blacklist = "cannot parse 'hello': Invalid IP"}, err) + end) + it("should not accept both a whitelist and a blacklist", function() + local t = {blacklist = {"127.0.0.1"}, whitelist = {"127.0.0.2"}} + local ok, err, self_err = v(t, schema) + assert.False(ok) + assert.falsy(err) + assert.equal("you cannot set both a whitelist and a blacklist", self_err.message) + end) + it("should not accept both empty whitelist and blacklist", function() + local t = {blacklist = {}, whitelist = {}} + local ok, err, self_err = v(t, schema) + assert.False(ok) + assert.falsy(err) + assert.equal("you must set at least a whitelist or blacklist", self_err.message) + end) + end) + describe("ok", function() + it("should accept a valid whitelist", function() + local t = {whitelist = {"127.0.0.1", "127.0.0.2"}} + local ok, err = v(t, schema) + assert.True(ok) + assert.falsy(err) + end) + it("should accept a valid blacklist", function() + local t = {blacklist = {"127.0.0.1", "127.0.0.2"}} + local ok, err = v(t, schema) + assert.True(ok) + assert.falsy(err) + end) + it("should build _whitelist_cache", function() + local t = {whitelist = {"127.0.0.1", "127.0.0.2"}} + local ok, err = v(t, schema) + assert.True(ok) + assert.falsy(err) + assert.is_table(t._whitelist_cache) + end) + it("should build _blacklist_cache", function() + local t = {blacklist = {"127.0.0.1", "127.0.0.2"}} + local ok, err = v(t, schema) + assert.True(ok) + assert.falsy(err) + assert.is_table(t._blacklist_cache) + end) + end) +end)