feat:As a user, I don't think allowlist and denylist of ua-restriction can be enabled at the same time

[840963657]

Description

I found other restriction plugin has a logic：Either one of whitelist or blacklist attribute must be specified。 But in ua-restriction, both allowlist and denylist can be used on their own. If User-Agent is neither on the allowlist nor the denylist , the processing logic is the same as that of the allowlist . So why do we still use the allowlist ？

[tokers]

Fair enough. But if we make allowlist and denylist exclusive, then this is a broken change. We need to evaluate it.

[tzssangglass]

if allowlist and denylist are both on, ua-restriction only uses allowlist, and User-Agents that are not in allowlist are rejected. How about this?

[spacewander]

Look like if the allowlist is matched, the denylist will be skipped? The match rule is via regex, so it is possible for a UA to match both allowlist and denylist.

[840963657]

Another problem: If I only configure allowlist , User-Agent is not on allowlist will also be released.

this code in apisix/plugins/ua-restriction.lua（apisix 2.13）

` local MATCH_NONE = 0 local MATCH_ALLOW = 1 local MATCH_DENY = 2 ......

if match > MATCH_ALLOW then return 403, { message = conf.message } end `

I looked at the code, If User-Agent on allowlist, the value of match becomes MATCH_ALLOW. If User-Agent not on allowlist, the value of match is MATCH_NONE. In both cases, User-Agent will be released. So I think the allowlist is meaningless.

[spacewander]

So we need an allowlist-only mode. When only allowlist is given, MATCH_NONE will be rejected. @840963657 Would you like to work on it?

[github-actions[bot]]

This issue has been marked as stale due to 350 days of inactivity. It will be closed in 2 weeks if no further activity occurs. If this issue is still relevant, please simply write any comment. Even if closed, you can still revive the issue at any time or discuss it on the [email protected] list. Thank you for your contributions.

[monkeyDluffy6017]

I think the allowlist and denylist should be exclusive, it they exist at the same time, it is very difficult to understand

[bzp2010]

I think it could go either way:

• Follow some sort of prioritization, e.g., allowlist over denylist. Match requests against each rule in order, and when any of the rules are satisfied, stop matching and either reject or pass them
• Use exclusion mode to allow only one type of allowlist or denylist to be configured

The second is simpler and easier to understand. I'm not sure there is a need for the first way of this plugin.

[monkeyDluffy6017]

@bzp2010 The first one looks meanless, if you set a allowlist, all you want is for the allowlist to pass, why do you set a blacklist at all? to block the one in the allowlist?

[jiangfucheng]

I think it could go either way:

• Follow some sort of prioritization, e.g., allowlist over denylist. Match requests against each rule in order, and when any of the rules are satisfied, stop matching and either reject or pass them
• Use exclusion mode to allow only one type of allowlist or denylist to be configured

The second is simpler and easier to understand. I'm not sure there is a need for the first way of this plugin.

1.I also agree with second way. 2.For first way, If the request not in both allowlist and denylist, what should we do? passing or rejecting seem not reasonable.

[monkeyDluffy6017]

I think we can work on this now

[jiangfucheng]

I think we can work on this now

Ok, I will raise a new PR for it.