Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

change(ua-restriction): allowlist and denylist can't be enabled at the same time #9841

Merged
merged 7 commits into from
Jul 24, 2023
Merged

change(ua-restriction): allowlist and denylist can't be enabled at the same time #9841

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
869a8cc
fix: allowlist and denylist can't be enabled at the same time
jiangfucheng Jul 15, 2023
8c78cc9
update docs
jiangfucheng Jul 15, 2023
36e5457
fix test
jiangfucheng Jul 15, 2023
30e4950
fix
jiangfucheng Jul 17, 2023
90a0975
fix
jiangfucheng Jul 17, 2023
258dfd3
fix
jiangfucheng Jul 18, 2023
7205f46
fix
jiangfucheng Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 45 additions & 36 deletions apisix/plugins/ua-restriction.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -55,20 +56,8 @@ local schema = {
},
},
oneOf = {
{
required = {"allowlist"}
},
{
required = {"denylist"}
},
{
["not"] = {
anyOf = {
{required = {"allowlist"}},
{required = {"denylist"}}
}
}
}
{required = {"allowlist"}},
{required = {"denylist"}}
}
}

Expand All @@ -81,27 +70,53 @@ 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
end
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
jiangfucheng marked this conversation as resolved.
Show resolved Hide resolved
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
jiangfucheng marked this conversation as resolved.
Show resolved Hide resolved
end
end
return true
end

return true
if type(user_agents) == "table" then
for _, v in ipairs(denylist) do
jiangfucheng marked this conversation as resolved.
Show resolved Hide resolved
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


Expand Down Expand Up @@ -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
Expand Down
185 changes: 45 additions & 140 deletions t/plugin/ua-restriction.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,136 +340,7 @@ User-Agent:foo/bar
hello world



=== TEST 18: bypass_missing test, using default, reset conf(part1)
Copy link
Contributor

Choose a reason for hiding this comment

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

The bypass_missing should work when allowlist or denylist exists and no user agent, the test case is needed

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

--- 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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -576,7 +447,7 @@ passed



=== TEST 27: test custom message
=== TEST 21: test custom message
--- request
GET /hello
--- more_headers
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -681,7 +552,7 @@ hello world



=== TEST 32: set disable=true
=== TEST 26: set disable=true
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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"}