help request: about the field allowed_by_methods of the plugin consumer-restriction cause unexpected behavior

[ronething]

Description

I want to restrict a user to access the route only by a certain method(like GET) and deny other users to access the route via set allowed_by_methods, configured as follows

# create consumer baz
curl http://127.0.0.1:9180/apisix/admin/consumers \
-H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "username": "baz",
    "plugins": {
        "basic-auth": {
            "username": "baz",
            "password": "foo"
        }
    }
}'

# create consumer hello
curl http://127.0.0.1:9180/apisix/admin/consumers \
-H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "username": "hello",
    "plugins": {
        "basic-auth": {
            "username": "hello",
            "password": "world"
        }
    }
}'

# access user baz by GET method with the field `allowed_by_methods`
curl http://127.0.0.1:9180/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/get",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:8080": 1
        }
    },
    "plugins": {
        "basic-auth": {},
        "consumer-restriction": {
            "allowed_by_methods":[{
                "user": "baz",
                "methods": ["GET"]
            }]
        }
    }
}'

1、restrict a user to access the route only by a certain method(like GET) is ok

$ curl -i -ubaz:foo http://127.0.0.1:9080/get -X GET 
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 265
Connection: keep-alive
Date: Mon, 06 Feb 2023 03:26:03 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Server: APISIX/3.1.0

{
  "args": {}, 
  "headers": {
    "Accept": "*/*", 
    "Authorization": "Basic YmF6OmZvbw==", 
    "Host": "127.0.0.1:9080", 
    "User-Agent": "curl/7.79.1", 
    "X-Forwarded-Host": "127.0.0.1"
  }, 
  "origin": "127.0.0.1", 
  "url": "http://127.0.0.1/get"
}

$ curl -i -ubaz:foo http://127.0.0.1:9080/get -X POST
HTTP/1.1 403 Forbidden
Date: Mon, 06 Feb 2023 03:22:57 GMT
Content-Type: text/plain; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Server: APISIX/3.1.0

{"message":"The consumer_name is forbidden."}

2、Deny other users to access this route, this does not work

$ curl -i -uhello:world http://127.0.0.1:9080/get   

HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 269
Connection: keep-alive
Date: Mon, 06 Feb 2023 03:22:31 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Server: APISIX/3.1.0

{
  "args": {}, 
  "headers": {
    "Accept": "*/*", 
    "Authorization": "Basic aGVsbG86d29ybGQ=", 
    "Host": "127.0.0.1:9080", 
    "User-Agent": "curl/7.79.1", 
    "X-Forwarded-Host": "127.0.0.1"
  }, 
  "origin": "127.0.0.1", 
  "url": "http://127.0.0.1/get"
}

If this is a bug, i would like to help and fix it

Environment

• APISIX version (run apisix version): 3.1.0
• Operating system (run uname -a):
• OpenResty / Nginx version (run openresty -V or nginx -V):
• etcd version, if relevant (run curl http://127.0.0.1:9090/v1/server_info):
• APISIX Dashboard version, if relevant:
• Plugin runner version, for issues related to plugin runners:
• LuaRocks version, for installation issues (run luarocks --version):

[mscb402]

The allowed_by_methods is to restrict what method the user accesses. So this means this is not for forbidding users to access routers.
If you want to ban particular users to access routers. You could config the whitelist or blacklist.
Like this

{
    "uri": "/get",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:8080": 1
        }
    },
    "plugins": {
        "basic-auth": {},
        "consumer-restriction": {
            "whitelist":[ "baz" ],
            "allowed_by_methods":[{
                "user": "baz",
                "methods": ["GET"]
            }]
        }
    }
}

[ronething]

@mscb402 yes, but if use whitelist, the allowed_by_methods does not work. for example the POST method is allowed like below. just ignore 405 error code, it's the resp code that httpbin server return.

$ curl -i -ubaz:foo http://127.0.0.1:9080/get -X POST
HTTP/1.1 405 METHOD NOT ALLOWED
Content-Type: text/html; charset=utf-8
Content-Length: 178
Connection: keep-alive
Date: Mon, 06 Feb 2023 07:37:04 GMT
Allow: OPTIONS, GET, HEAD
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Server: APISIX/3.1.0

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
<title>405 Method Not Allowed</title>
<h1>Method Not Allowed</h1>
<p>The method is not allowed for the requested URL.</p>

[monkeyDluffy6017]

allowed_by_methods can only be restricted for a specific user and has no effect on other users

Could you explain the behavior you expect?

[mscb402]

I know what happens. http://127.0.0.1:9080/get is a httpbin upstream. And httpbin.org/get is only accepted for GET methods.

[mscb402]

So please use httpbin.org/post as your upstream when you want to test POST request.

[ronething]

So please use httpbin.org/post as your upstream when you want to test POST request.

@mscb402 maybe it's not the point, but you can test like below. After create consumer baz and hello

$ curl http://127.0.0.1:9180/apisix/admin/routes/1 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/*",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "127.0.0.1:8080": 1
        }
    },
    "plugins": {
        "basic-auth": {},
        "consumer-restriction": {
            "whitelist":[ "baz" ],
            "allowed_by_methods":[{
                "user": "baz",
                "methods": ["GET"]
            }]
        }
    }
}'

{"key":"/apisix/routes/1","value":{"priority":0,"plugins":{"basic-auth":{"hide_credentials":false},"consumer-restriction":{"whitelist":["baz"],"type":"consumer_name","rejected_code":403,"allowed_by_methods":[{"user":"baz","methods":["GET"]}]}},"upstream":{"type":"roundrobin","hash_on":"vars","scheme":"http","nodes":{"127.0.0.1:8080":1},"pass_host":"pass"},"uri":"/*","status":1,"id":"1","update_time":1675670501,"create_time":1674875420}}

$ curl -i -ubaz:foo http://127.0.0.1:9080/get -X GET
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 265
Connection: keep-alive
Date: Mon, 06 Feb 2023 08:01:47 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Server: APISIX/3.1.0

{
  "args": {}, 
  "headers": {
    "Accept": "*/*", 
    "Authorization": "Basic YmF6OmZvbw==", 
    "Host": "127.0.0.1:9080", 
    "User-Agent": "curl/7.79.1", 
    "X-Forwarded-Host": "127.0.0.1"
  }, 
  "origin": "127.0.0.1", 
  "url": "http://127.0.0.1/get"
}

$ curl -i -ubaz:foo http://127.0.0.1:9080/post -X POST
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 329
Connection: keep-alive
Date: Mon, 06 Feb 2023 08:01:57 GMT
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Server: APISIX/3.1.0

{
  "args": {}, 
  "data": "", 
  "files": {}, 
  "form": {}, 
  "headers": {
    "Accept": "*/*", 
    "Authorization": "Basic YmF6OmZvbw==", 
    "Host": "127.0.0.1:9080", 
    "User-Agent": "curl/7.79.1", 
    "X-Forwarded-Host": "127.0.0.1"
  }, 
  "json": null, 
  "origin": "127.0.0.1", 
  "url": "http://127.0.0.1/post"
}

you will see POST method is allowed but we don't set it in the allowed_by_methods

[mscb402]

Yes, you are right

if conf.allowed_by_methods and #conf.allowed_by_methods > 0 and not whitelisted then
        if not is_method_allowed(conf.allowed_by_methods, method, value) then
            block = true
        end
    end

At APISIX lua code. When whitelisted is set, the allowed_by_methods will ineffective

[monkeyDluffy6017]

https://github.com/apache/apisix/blob/master/apisix/plugins/consumer-restriction.lua#L151
The allowed_by_methods whitelist are mutually exclusive

#3691

[tokers]

@ronething As it stands now, we may not have any ways to do this (we cannot use a global rule to setup a consumer whitelist, as the global rules run before the route match).

[ronething]

we cannot use a global rule to setup a consumer whitelist, as the global rules run before the route match

@tokers I mean one route but not any route. And i wonder if the allowed_by_methods of the consumer-restriction plugin has a bug or not.

And the allow already has the meaning of whitelist. refer: #3691 (comment)

if yes, i will change PR status to ready for review #8795

[tokers]

Emmm. Not sure if this is a deliberated feature.

[github-actions]

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 dev@apisix.apache.org list. Thank you for your contributions.

[github-actions]

This issue has been closed due to lack of activity. If you think that is incorrect, or the issue requires additional review, you can revive the issue at any time.