From 869a8cc2d17c19762bbc262eb0b23df2dbd661dd Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Sat, 15 Jul 2023 21:57:26 +0800 Subject: [PATCH 1/7] fix: allowlist and denylist can't be enabled at the same time --- apisix/plugins/ua-restriction.lua | 23 +++++----- t/plugin/ua-restriction.t | 74 +++---------------------------- 2 files changed, 18 insertions(+), 79 deletions(-) diff --git a/apisix/plugins/ua-restriction.lua b/apisix/plugins/ua-restriction.lua index ec74e7592115..606313d96df6 100644 --- a/apisix/plugins/ua-restriction.lua +++ b/apisix/plugins/ua-restriction.lua @@ -22,10 +22,6 @@ local type = type local str_strip = stringx.strip local re_find = ngx.re.find -local MATCH_NONE = 0 -local MATCH_ALLOW = 1 -local MATCH_DENY = 2 - local lrucache_useragent = core.lrucache.new({ ttl = 300, count = 4096 }) local schema = { @@ -74,20 +70,22 @@ local function match_user_agent(user_agent, conf) if conf.allowlist then for _, rule in ipairs(conf.allowlist) do if re_find(user_agent, rule, "jo") then - return MATCH_ALLOW + return true end end + return false end if conf.denylist then for _, rule in ipairs(conf.denylist) do if re_find(user_agent, rule, "jo") then - return MATCH_DENY + return false end end + return true end - return MATCH_NONE + return true end function _M.check_schema(conf) @@ -97,6 +95,10 @@ function _M.check_schema(conf) return false, err end + if conf.allowlist and conf.denylist then + return false, "allowlist and denylist can't be enabled at the same time." + end + if conf.allowlist then for _, re_rule in ipairs(conf.allowlist) do ok, err = re_compile(re_rule, "j") @@ -128,12 +130,13 @@ function _M.access(conf, ctx) return 403, { message = conf.message } end end - local match = MATCH_NONE + local match + if type(user_agent) == "table" then for _, v in ipairs(user_agent) do if type(v) == "string" then match = lrucache_useragent(v, conf, match_user_agent, v, conf) - if match > MATCH_ALLOW then + if not match then break end end @@ -142,7 +145,7 @@ function _M.access(conf, ctx) match = lrucache_useragent(user_agent, conf, match_user_agent, user_agent, conf) end - if match > MATCH_ALLOW then + if not match then return 403, { message = conf.message } end end diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index 0e8a9544bd34..40deef7c3f0d 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -38,13 +38,12 @@ run_tests; __DATA__ -=== TEST 1: set allowlist, denylist, bypass_missing and user-defined message +=== TEST 1: set both allowlist and denylist --- config location /t { content_by_lua_block { local plugin = require("apisix.plugins.ua-restriction") local conf = { - bypass_missing = true, allowlist = { "my-bot1", "my-bot2" @@ -53,18 +52,18 @@ __DATA__ "my-bot1", "my-bot2" }, - message = "User-Agent Forbidden", } local ok, err = plugin.check_schema(conf) if not ok then ngx.say(err) + return end ngx.say(require("toolkit.json").encode(conf)) } } --- response_body -{"allowlist":["my-bot1","my-bot2"],"bypass_missing":true,"denylist":["my-bot1","my-bot2"],"message":"User-Agent Forbidden"} +allowlist and denylist can't be enabled at the same time. @@ -302,72 +301,9 @@ hello world GET /hello --- more_headers User-Agent:foo/bar ---- error_code: 200 ---- response_body -hello world - - - -=== TEST 15: set config: user-agent in both allowlist and denylist ---- config - location /t { - content_by_lua_block { - local t = require("lib.test_admin").test - local code, body = t('/apisix/admin/routes/1', - ngx.HTTP_PUT, - [[{ - "uri": "/hello", - "upstream": { - "type": "roundrobin", - "nodes": { - "127.0.0.1:1980": 1 - } - }, - "plugins": { - "ua-restriction": { - "allowlist": [ - "foo/bar", - "(Baiduspider)/(\\d+)\\.(\\d+)" - ], - "denylist": [ - "foo/bar", - "(Baiduspider)/(\\d+)\\.(\\d+)" - ] - } - } - }]] - ) - - if code >= 300 then - ngx.status = code - end - ngx.say(body) - } - } ---- response_body -passed - - - -=== TEST 16: hit route and user-agent in both allowlist and denylist, pass(part 1) ---- request -GET /hello ---- more_headers -User-Agent:foo/bar ---- error_code: 200 ---- response_body -hello world - - - -=== TEST 17: hit route and user-agent in both allowlist and denylist, pass(part 2) ---- request -GET /hello ---- more_headers -User-Agent:Baiduspider/1.0 ---- error_code: 200 +--- error_code: 403 --- response_body -hello world +{"message":"Not allowed"} From 8c78cc9e9cc2ab94fefb75a114c6628b0abcbf30 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Sat, 15 Jul 2023 22:04:52 +0800 Subject: [PATCH 2/7] update docs --- docs/en/latest/plugins/ua-restriction.md | 2 +- docs/zh/latest/plugins/ua-restriction.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/latest/plugins/ua-restriction.md b/docs/en/latest/plugins/ua-restriction.md index 84bd2cb14303..701b39531545 100644 --- a/docs/en/latest/plugins/ua-restriction.md +++ b/docs/en/latest/plugins/ua-restriction.md @@ -43,7 +43,7 @@ A common scenario is to set crawler rules. `User-Agent` is the identity of the c :::note -Both `allowlist` and `denylist` can be used on their own. If they are used together, the `allowlist` matches before the `denylist`. +Both `allowlist` and `denylist` can't be used at the same time. ::: diff --git a/docs/zh/latest/plugins/ua-restriction.md b/docs/zh/latest/plugins/ua-restriction.md index 3f31e092c2ae..d8e21b62f1f7 100644 --- a/docs/zh/latest/plugins/ua-restriction.md +++ b/docs/zh/latest/plugins/ua-restriction.md @@ -43,7 +43,7 @@ description: 本文介绍了 Apache APISIX ua-restriction 插件的使用方法 :::note -`allowlist` 和 `denylist` 可以同时启用。同时启用时,插件会根据 `User-Agent` 先检查 `allowlist`,再检查 `denylist`。 +`allowlist` 和 `denylist` 不可以同时启用。 ::: From 36e5457a64eec098f1ebee5a33549862cfd5c7ea Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Sat, 15 Jul 2023 22:08:20 +0800 Subject: [PATCH 3/7] fix test --- t/plugin/ua-restriction.t | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index 40deef7c3f0d..ca7f8aa7489a 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -307,7 +307,7 @@ User-Agent:foo/bar -=== TEST 18: bypass_missing test, using default, reset conf(part1) +=== TEST 15: bypass_missing test, using default, reset conf(part1) --- config location /t { content_by_lua_block { @@ -340,7 +340,7 @@ passed -=== TEST 19: bypass_missing test, using default, send request without User-Agent(part2) +=== TEST 16: bypass_missing test, using default, send request without User-Agent(part2) --- request GET /hello --- error_code: 403 @@ -349,7 +349,7 @@ GET /hello -=== TEST 20: bypass_missing test, set to true(part1) +=== TEST 17: bypass_missing test, set to true(part1) --- config location /t { content_by_lua_block { @@ -383,7 +383,7 @@ passed -=== TEST 21: bypass_missing test, set to true, send request without User-Agent(part2) +=== TEST 18: bypass_missing test, set to true, send request without User-Agent(part2) --- request GET /hello --- error_code: 200 @@ -392,7 +392,7 @@ hello world -=== TEST 22: bypass_missing test, set to false(part1) +=== TEST 19: bypass_missing test, set to false(part1) --- config location /t { content_by_lua_block { @@ -426,7 +426,7 @@ passed -=== TEST 23: bypass_missing test, set to false, send request without User-Agent(part2) +=== TEST 20: bypass_missing test, set to false, send request without User-Agent(part2) --- request GET /hello --- error_code: 403 @@ -435,7 +435,7 @@ GET /hello -=== TEST 24: message that do not reach the minimum range +=== TEST 21: message that do not reach the minimum range --- config location /t { content_by_lua_block { @@ -466,7 +466,7 @@ qr/string too short, expected at least 1, got 0/ -=== TEST 25: exceeds the maximum limit of message +=== TEST 22: exceeds the maximum limit of message --- config location /t { content_by_lua_block { @@ -504,7 +504,7 @@ qr/string too long, expected at most 1024, got 1025/ -=== TEST 26: set custom message +=== TEST 23: set custom message --- config location /t { content_by_lua_block { @@ -542,7 +542,7 @@ passed -=== TEST 27: test custom message +=== TEST 24: test custom message --- request GET /hello --- more_headers @@ -553,7 +553,7 @@ User-Agent:Baiduspider/1.0 -=== TEST 28: test remove ua-restriction, add denylist(part 1) +=== TEST 25: test remove ua-restriction, add denylist(part 1) --- config location /enable { content_by_lua_block { @@ -592,7 +592,7 @@ passed -=== TEST 29: test remove ua-restriction, fail(part 2) +=== TEST 26: test remove ua-restriction, fail(part 2) --- request GET /hello --- more_headers @@ -603,7 +603,7 @@ User-Agent:Baiduspider/1.0 -=== TEST 30: test remove ua-restriction, remove plugin(part 3) +=== TEST 27: test remove ua-restriction, remove plugin(part 3) --- config location /disable { content_by_lua_block { @@ -637,7 +637,7 @@ passed -=== TEST 31: test remove ua-restriction, check spider User-Agent(part 4) +=== TEST 28: test remove ua-restriction, check spider User-Agent(part 4) --- request GET /hello --- more_headers @@ -647,7 +647,7 @@ hello world -=== TEST 32: set disable=true +=== TEST 29: set disable=true --- config location /t { content_by_lua_block { @@ -680,7 +680,7 @@ passed -=== TEST 33: the element in allowlist is null +=== TEST 30: the element in allowlist is null --- config location /t { content_by_lua_block { @@ -707,7 +707,7 @@ done -=== TEST 34: the element in denylist is null +=== TEST 31: the element in denylist is null --- config location /t { content_by_lua_block { From 30e4950fec656d2b5360e8c5fcfe5587f21a1088 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Mon, 17 Jul 2023 21:10:40 +0800 Subject: [PATCH 4/7] fix --- apisix/plugins/ua-restriction.lua | 4 +- t/plugin/ua-restriction.t | 80 ++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/apisix/plugins/ua-restriction.lua b/apisix/plugins/ua-restriction.lua index 606313d96df6..cc861106f1d0 100644 --- a/apisix/plugins/ua-restriction.lua +++ b/apisix/plugins/ua-restriction.lua @@ -88,6 +88,7 @@ local function match_user_agent(user_agent, conf) return true end + function _M.check_schema(conf) local ok, err = core.schema.check(schema, conf) @@ -120,6 +121,7 @@ function _M.check_schema(conf) return true end + function _M.access(conf, ctx) local user_agent = core.request.header(ctx, "User-Agent") @@ -136,7 +138,7 @@ function _M.access(conf, ctx) for _, v in ipairs(user_agent) do if type(v) == "string" then match = lrucache_useragent(v, conf, match_user_agent, v, conf) - if not match then + if (conf.denylist and not match) or (conf.allowlist and match) then break end end diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index ca7f8aa7489a..aa5275b615a7 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -215,7 +215,19 @@ User-Agent:my-bot2 -=== TEST 9: hit route and user-agent match denylist regex +=== TEST 9: hit route and user-agent in denylist with reverse order multiple user-agent +--- request +GET /hello +--- more_headers +User-Agent:my-bot2 +User-Agent:my-bot1 +--- error_code: 403 +--- response_body +{"message":"Not allowed"} + + + +=== TEST 10: hit route and user-agent match denylist regex --- request GET /hello --- more_headers @@ -226,7 +238,7 @@ User-Agent:Baiduspider/3.0 -=== TEST 10: hit route and user-agent not in denylist +=== TEST 11: hit route and user-agent not in denylist --- request GET /hello --- more_headers @@ -237,7 +249,7 @@ hello world -=== TEST 11: set allowlist +=== TEST 12: set allowlist --- config location /t { content_by_lua_block { @@ -274,7 +286,7 @@ passed -=== TEST 12: hit route and user-agent in allowlist +=== TEST 13: hit route and user-agent in allowlist --- request GET /hello --- more_headers @@ -285,7 +297,7 @@ hello world -=== TEST 13: hit route and user-agent match allowlist regex +=== TEST 14: hit route and user-agent match allowlist regex --- request GET /hello --- more_headers @@ -296,7 +308,7 @@ hello world -=== TEST 14: hit route and user-agent not in allowlist +=== TEST 15: hit route and user-agent not in allowlist --- request GET /hello --- more_headers @@ -307,7 +319,29 @@ User-Agent:foo/bar -=== TEST 15: bypass_missing test, using default, reset conf(part1) +=== TEST 16: hit route and user-agent in allowlist with multiple user-agent +--- request +GET /hello +--- more_headers +User-Agent:foo/bar +User-Agent:my-bot1 +--- response_body +hello world + + + +=== TEST 17: hit route and user-agent in allowlist with reverse order multiple user-agent +--- request +GET /hello +--- more_headers +User-Agent:my-bot1 +User-Agent:foo/bar +--- response_body +hello world + + + +=== TEST 18: bypass_missing test, using default, reset conf(part1) --- config location /t { content_by_lua_block { @@ -340,7 +374,7 @@ passed -=== TEST 16: bypass_missing test, using default, send request without User-Agent(part2) +=== TEST 19: bypass_missing test, using default, send request without User-Agent(part2) --- request GET /hello --- error_code: 403 @@ -349,7 +383,7 @@ GET /hello -=== TEST 17: bypass_missing test, set to true(part1) +=== TEST 20: bypass_missing test, set to true(part1) --- config location /t { content_by_lua_block { @@ -383,7 +417,7 @@ passed -=== TEST 18: bypass_missing test, set to true, send request without User-Agent(part2) +=== TEST 21: bypass_missing test, set to true, send request without User-Agent(part2) --- request GET /hello --- error_code: 200 @@ -392,7 +426,7 @@ hello world -=== TEST 19: bypass_missing test, set to false(part1) +=== TEST 22: bypass_missing test, set to false(part1) --- config location /t { content_by_lua_block { @@ -426,7 +460,7 @@ passed -=== TEST 20: bypass_missing test, set to false, send request without User-Agent(part2) +=== TEST 23: bypass_missing test, set to false, send request without User-Agent(part2) --- request GET /hello --- error_code: 403 @@ -435,7 +469,7 @@ GET /hello -=== TEST 21: message that do not reach the minimum range +=== TEST 24: message that do not reach the minimum range --- config location /t { content_by_lua_block { @@ -466,7 +500,7 @@ qr/string too short, expected at least 1, got 0/ -=== TEST 22: exceeds the maximum limit of message +=== TEST 25: exceeds the maximum limit of message --- config location /t { content_by_lua_block { @@ -504,7 +538,7 @@ qr/string too long, expected at most 1024, got 1025/ -=== TEST 23: set custom message +=== TEST 26: set custom message --- config location /t { content_by_lua_block { @@ -542,7 +576,7 @@ passed -=== TEST 24: test custom message +=== TEST 27: test custom message --- request GET /hello --- more_headers @@ -553,7 +587,7 @@ User-Agent:Baiduspider/1.0 -=== TEST 25: test remove ua-restriction, add denylist(part 1) +=== TEST 28: test remove ua-restriction, add denylist(part 1) --- config location /enable { content_by_lua_block { @@ -592,7 +626,7 @@ passed -=== TEST 26: test remove ua-restriction, fail(part 2) +=== TEST 29: test remove ua-restriction, fail(part 2) --- request GET /hello --- more_headers @@ -603,7 +637,7 @@ User-Agent:Baiduspider/1.0 -=== TEST 27: test remove ua-restriction, remove plugin(part 3) +=== TEST 30: test remove ua-restriction, remove plugin(part 3) --- config location /disable { content_by_lua_block { @@ -637,7 +671,7 @@ passed -=== TEST 28: test remove ua-restriction, check spider User-Agent(part 4) +=== TEST 31: test remove ua-restriction, check spider User-Agent(part 4) --- request GET /hello --- more_headers @@ -647,7 +681,7 @@ hello world -=== TEST 29: set disable=true +=== TEST 32: set disable=true --- config location /t { content_by_lua_block { @@ -680,7 +714,7 @@ passed -=== TEST 30: the element in allowlist is null +=== TEST 33: the element in allowlist is null --- config location /t { content_by_lua_block { @@ -707,7 +741,7 @@ done -=== TEST 31: the element in denylist is null +=== TEST 34: the element in denylist is null --- config location /t { content_by_lua_block { From 90a097586c05426909d4bc7bd4e8b347e5e61e30 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Mon, 17 Jul 2023 21:27:22 +0800 Subject: [PATCH 5/7] fix --- apisix/plugins/ua-restriction.lua | 20 ++++++++++++++++---- t/plugin/ua-restriction.t | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/apisix/plugins/ua-restriction.lua b/apisix/plugins/ua-restriction.lua index cc861106f1d0..37b7bc4f9dc8 100644 --- a/apisix/plugins/ua-restriction.lua +++ b/apisix/plugins/ua-restriction.lua @@ -54,6 +54,22 @@ local schema = { default = "Not allowed" }, }, + oneOf = { + { + required = {"allowlist"} + }, + { + required = {"denylist"} + }, + { + ["not"] = { + anyOf = { + {required = {"allowlist"}}, + {required = {"denylist"}} + } + } + } + } } local plugin_name = "ua-restriction" @@ -96,10 +112,6 @@ function _M.check_schema(conf) return false, err end - if conf.allowlist and conf.denylist then - return false, "allowlist and denylist can't be enabled at the same time." - end - if conf.allowlist then for _, re_rule in ipairs(conf.allowlist) do ok, err = re_compile(re_rule, "j") diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index aa5275b615a7..7c0cdd8b9108 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -63,7 +63,7 @@ __DATA__ } } --- response_body -allowlist and denylist can't be enabled at the same time. +value should match only one schema, but matches both schemas 1 and 2 From 258dfd3dd1e0f684b291d3d8bb4bbf58a6dbcaba Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Tue, 18 Jul 2023 21:05:46 +0800 Subject: [PATCH 6/7] fix --- apisix/plugins/ua-restriction.lua | 81 +++++++------ t/plugin/ua-restriction.t | 185 ++++++++---------------------- 2 files changed, 90 insertions(+), 176 deletions(-) diff --git a/apisix/plugins/ua-restriction.lua b/apisix/plugins/ua-restriction.lua index 37b7bc4f9dc8..2d80059802fb 100644 --- a/apisix/plugins/ua-restriction.lua +++ b/apisix/plugins/ua-restriction.lua @@ -22,7 +22,8 @@ local type = type local str_strip = stringx.strip local re_find = ngx.re.find -local lrucache_useragent = core.lrucache.new({ ttl = 300, count = 4096 }) +local lrucache_allow = core.lrucache.new({ ttl = 300, count = 4096 }) +local lrucache_deny = core.lrucache.new({ ttl = 300, count = 4096 }) local schema = { type = "object", @@ -55,20 +56,8 @@ local schema = { }, }, oneOf = { - { - required = {"allowlist"} - }, - { - required = {"denylist"} - }, - { - ["not"] = { - anyOf = { - {required = {"allowlist"}}, - {required = {"denylist"}} - } - } - } + {required = {"allowlist"}}, + {required = {"denylist"}} } } @@ -81,10 +70,11 @@ local _M = { schema = schema, } -local function match_user_agent(user_agent, conf) - user_agent = str_strip(user_agent) - if conf.allowlist then - for _, rule in ipairs(conf.allowlist) do +local function check_with_allow_list(user_agents, allowlist) + local check = function (user_agent) + user_agent = str_strip(user_agent) + + for _, rule in ipairs(allowlist) do if re_find(user_agent, rule, "jo") then return true end @@ -92,16 +82,41 @@ local function match_user_agent(user_agent, conf) return false end - if conf.denylist then - for _, rule in ipairs(conf.denylist) do + if type(user_agents) == "table" then + for _, v in ipairs(allowlist) do + if lrucache_allow(v, allowlist, check, v) then + return true + end + end + return false + else + return lrucache_allow(user_agents, allowlist, check, user_agents) + end +end + + +local function check_with_deny_list(user_agents, denylist) + local check = function (user_agent) + user_agent = str_strip(user_agent) + + for _, rule in ipairs(denylist) do if re_find(user_agent, rule, "jo") then - return false + return false end end return true end - return true + if type(user_agents) == "table" then + for _, v in ipairs(denylist) do + if lrucache_allow(v, denylist, check, v) then + return false + end + end + return true + else + return lrucache_allow(user_agents, denylist, check, user_agents) + end end @@ -144,22 +159,16 @@ function _M.access(conf, ctx) return 403, { message = conf.message } end end - local match - - if type(user_agent) == "table" then - for _, v in ipairs(user_agent) do - if type(v) == "string" then - match = lrucache_useragent(v, conf, match_user_agent, v, conf) - if (conf.denylist and not match) or (conf.allowlist and match) then - break - end - end - end + + local is_passed + + if conf.allowlist then + is_passed = check_with_allow_list(user_agent, conf.allowlist) else - match = lrucache_useragent(user_agent, conf, match_user_agent, user_agent, conf) + is_passed = check_with_deny_list(user_agent, conf.denylist) end - if not match then + if not is_passed then return 403, { message = conf.message } end end diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index 7c0cdd8b9108..b4a3d93bb11e 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -340,136 +340,7 @@ User-Agent:foo/bar hello world - -=== TEST 18: bypass_missing test, using default, reset conf(part1) ---- config - location /t { - content_by_lua_block { - local t = require("lib.test_admin").test - local code, body = t('/apisix/admin/routes/1', - ngx.HTTP_PUT, - [[{ - "uri": "/hello", - "upstream": { - "type": "roundrobin", - "nodes": { - "127.0.0.1:1980": 1 - } - }, - "plugins": { - "ua-restriction": { - } - } - }]] - ) - - if code >= 300 then - ngx.status = code - end - ngx.say(body) - } - } ---- response_body -passed - - - -=== TEST 19: bypass_missing test, using default, send request without User-Agent(part2) ---- request -GET /hello ---- error_code: 403 ---- response_body -{"message":"Not allowed"} - - - -=== TEST 20: bypass_missing test, set to true(part1) ---- config - location /t { - content_by_lua_block { - local t = require("lib.test_admin").test - local code, body = t('/apisix/admin/routes/1', - ngx.HTTP_PUT, - [[{ - "uri": "/hello", - "upstream": { - "type": "roundrobin", - "nodes": { - "127.0.0.1:1980": 1 - } - }, - "plugins": { - "ua-restriction": { - "bypass_missing": true - } - } - }]] - ) - - if code >= 300 then - ngx.status = code - end - ngx.say(body) - } - } ---- response_body -passed - - - -=== TEST 21: bypass_missing test, set to true, send request without User-Agent(part2) ---- request -GET /hello ---- error_code: 200 ---- response_body -hello world - - - -=== TEST 22: bypass_missing test, set to false(part1) ---- config - location /t { - content_by_lua_block { - local t = require("lib.test_admin").test - local code, body = t('/apisix/admin/routes/1', - ngx.HTTP_PUT, - [[{ - "uri": "/hello", - "upstream": { - "type": "roundrobin", - "nodes": { - "127.0.0.1:1980": 1 - } - }, - "plugins": { - "ua-restriction": { - "bypass_missing": false - } - } - }]] - ) - - if code >= 300 then - ngx.status = code - end - ngx.say(body) - } - } ---- response_body -passed - - - -=== TEST 23: bypass_missing test, set to false, send request without User-Agent(part2) ---- request -GET /hello ---- error_code: 403 ---- response_body -{"message":"Not allowed"} - - - -=== TEST 24: message that do not reach the minimum range +=== TEST 18: message that do not reach the minimum range --- config location /t { content_by_lua_block { @@ -500,7 +371,7 @@ qr/string too short, expected at least 1, got 0/ -=== TEST 25: exceeds the maximum limit of message +=== TEST 19: exceeds the maximum limit of message --- config location /t { content_by_lua_block { @@ -538,7 +409,7 @@ qr/string too long, expected at most 1024, got 1025/ -=== TEST 26: set custom message +=== TEST 20: set custom message --- config location /t { content_by_lua_block { @@ -576,7 +447,7 @@ passed -=== TEST 27: test custom message +=== TEST 21: test custom message --- request GET /hello --- more_headers @@ -587,7 +458,7 @@ User-Agent:Baiduspider/1.0 -=== TEST 28: test remove ua-restriction, add denylist(part 1) +=== TEST 22: test remove ua-restriction, add denylist(part 1) --- config location /enable { content_by_lua_block { @@ -626,7 +497,7 @@ passed -=== TEST 29: test remove ua-restriction, fail(part 2) +=== TEST 23: test remove ua-restriction, fail(part 2) --- request GET /hello --- more_headers @@ -637,7 +508,7 @@ User-Agent:Baiduspider/1.0 -=== TEST 30: test remove ua-restriction, remove plugin(part 3) +=== TEST 24: test remove ua-restriction, remove plugin(part 3) --- config location /disable { content_by_lua_block { @@ -671,7 +542,7 @@ passed -=== TEST 31: test remove ua-restriction, check spider User-Agent(part 4) +=== TEST 25: test remove ua-restriction, check spider User-Agent(part 4) --- request GET /hello --- more_headers @@ -681,7 +552,7 @@ hello world -=== TEST 32: set disable=true +=== TEST 26: set disable=true --- config location /t { content_by_lua_block { @@ -714,7 +585,7 @@ passed -=== TEST 33: the element in allowlist is null +=== TEST 27: the element in allowlist is null --- config location /t { content_by_lua_block { @@ -741,7 +612,7 @@ done -=== TEST 34: the element in denylist is null +=== TEST 28: the element in denylist is null --- config location /t { content_by_lua_block { @@ -765,3 +636,37 @@ done --- response_body property "denylist" validation failed: wrong type: expected array, got table done + + + +=== TEST 29: test both allowlist and denylist are not exist +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:1980": 1 + } + }, + "plugins": { + "ua-restriction": { + } + } + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.print(body) + } + } +--- error_code: 400 +--- response_body +{"error_msg":"failed to check the configuration of plugin ua-restriction err: value should match only one schema, but matches none"} From 7205f46e3820b144e0b7b09e623a248dff1b6842 Mon Sep 17 00:00:00 2001 From: jiangfucheng Date: Wed, 19 Jul 2023 01:24:48 +0800 Subject: [PATCH 7/7] fix --- apisix/plugins/ua-restriction.lua | 10 ++-- t/plugin/ua-restriction.t | 89 +++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/apisix/plugins/ua-restriction.lua b/apisix/plugins/ua-restriction.lua index 2d80059802fb..577dc2b67cbb 100644 --- a/apisix/plugins/ua-restriction.lua +++ b/apisix/plugins/ua-restriction.lua @@ -83,7 +83,7 @@ local function check_with_allow_list(user_agents, allowlist) end if type(user_agents) == "table" then - for _, v in ipairs(allowlist) do + for _, v in ipairs(user_agents) do if lrucache_allow(v, allowlist, check, v) then return true end @@ -101,21 +101,21 @@ local function check_with_deny_list(user_agents, denylist) for _, rule in ipairs(denylist) do if re_find(user_agent, rule, "jo") then - return false + return false end end return true end if type(user_agents) == "table" then - for _, v in ipairs(denylist) do - if lrucache_allow(v, denylist, check, v) then + for _, v in ipairs(user_agents) do + if lrucache_deny(v, denylist, check, v) then return false end end return true else - return lrucache_allow(user_agents, denylist, check, user_agents) + return lrucache_deny(user_agents, denylist, check, user_agents) end end diff --git a/t/plugin/ua-restriction.t b/t/plugin/ua-restriction.t index b4a3d93bb11e..56f07b39b13f 100644 --- a/t/plugin/ua-restriction.t +++ b/t/plugin/ua-restriction.t @@ -340,6 +340,7 @@ User-Agent:foo/bar hello world + === TEST 18: message that do not reach the minimum range --- config location /t { @@ -670,3 +671,91 @@ done --- error_code: 400 --- response_body {"error_msg":"failed to check the configuration of plugin ua-restriction err: value should match only one schema, but matches none"} + + + +=== TEST 30: test bypass_missing +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:1980": 1 + } + }, + "plugins": { + "ua-restriction": { + "allowlist": [ + "my-bot1" + ] + } + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 31: hit +--- request +GET /hello +--- error_code: 403 +--- response_body +{"message":"Not allowed"} + + + +=== TEST 32: test bypass_missing with true +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": { + "127.0.0.1:1980": 1 + } + }, + "plugins": { + "ua-restriction": { + "bypass_missing": true, + "denylist": [ + "my-bot1" + ] + } + } + }]] + ) + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 33: hit +--- request +GET /hello +--- response_body +hello world