ACL allows user to perform any action in endpoints not related to tables, such as /zk, /instances, etc

[rodolphogarrido]

I was testing the ACL feature based on this doc, and everything is working fine regarding table/schema operations (CREATE, READ, UPDATE, DELETE), but I found that if I call any other endpoint that is not related to a table operation using an user that should only have READ access to tables, such as a PUT, DELETE, GET and POST in endpoints like /zk, /instances/, this user is allowed to perform the operation.

Is this the intended behavior or only for table access control? I feel it could lead to a potential problem for our cluster if users decides to call these endpoints without the knowledge of what they are doing (I was even able to delete the cluster path in Zk). In our use case we'll be managing Pinot and having it as a service for other teams and thus we don't intend to let users update any config that isn't related to their own table.

Cluster ACL configs:

Controller ACL conf:

# ACLs
# Enable ACL enforcement in the controller
controller.admin.access.control.factory.class=org.apache.pinot.controller.api.access.BasicAuthAccessControlFactory

# Create users "admin" and "user". Keep in mind we're not enforcing any ACLs yet.
controller.admin.access.control.principals=admin,user
# Set the admin's password to "verysecret"
controller.admin.access.control.principals.admin.password=verysecret
# Set the user's password to "secret" and allow "READ" only
controller.admin.access.control.principals.user.password=secret
controller.admin.access.control.principals.user.permissions=READ
controller.admin.access.control.principals.user.tables=events,events_upsert_full,events_upsert_partial
# Enable the controller to fetch segments by providing the credentials as a token "Basic " + base64encode("admin:verysecret")
controller.segment.fetcher.auth.token="Basic YWRtaW46dmVyeXNlY3JldA=="

Server ACL conf:

# ACLs
# Enable the Server to fetch/upload segments by providing the credentials as a token "Basic " + base64encode("admin:verysecret")
pinot.server.segment.fetcher.auth.token="Basic YWRtaW46dmVyeXNlY3JldA=="
pinot.server.segment.uploader.auth.token="Basic YWRtaW46dmVyeXNlY3JldA=="
pinot.server.instance.auth.token="Basic YWRtaW46dmVyeXNlY3JldA=="

Broker ACL conf:

# Enable ACL enforcement in the Broker
# The factory class property is different for the broker
pinot.broker.access.control.class=org.apache.pinot.broker.broker.BasicAuthAccessControlFactory

# Create the users and password (must be exactly the same as the ones created for the controller)
pinot.broker.access.control.principals=admin,user
pinot.broker.access.control.principals.admin.password=verysecret
pinot.broker.access.control.principals.user.password=secret

# No need to set READ permissions here since broker requests are read-only

# This configuration option allow specification of usernames and passwords as well as optional ACL restrictions on a per-table table basis (access type is always READ)
pinot.broker.access.control.principals.user.tables=events,events_upsert_full,events_upsert_partial

Minion ACL conf:

# ACLs
segment.fetcher.auth.token="Basic YWRtaW46dmVyeXNlY3JldA=="
task.auth.token="Basic YWRtaW46dmVyeXNlY3JldA=="

Example of working endpoint calls (which I expected not to be allowed):

1. Untagging Servers:

   
   instances=("Server_pinot-server-0_8098" "Server_pinot-server-1_8098" "Server_pinot-server-2_8098" "Server_pinot-server-3_8098")

for instance in ${instances[@]}; do curl -X PUT \ "http://localhost:9000/instances/${instance}/updateTags?tags=server_untagged&updateBrokerResource=false" \ -H "accept: application/json" \ -u user:secret done

2. Deleting Zookeeper path for our cluster: 
```sh
curl -X DELETE "http://localhost:9000/zk/delete?path=%2Fpinot-example" -H "accept: application/json" -u user:secret

This obviously destroyed the cluster.

General cluster info

Apache Pinot version: 0.12.1

Thank you very much!

[Jackie-Jiang]

Thanks for reporting the issue! This framework is controller by the Authenticate annotation, and I do see it added for these ZK APIs (In ZookeeperResource introduced in #6507), so I don't know why the check is bypassed. @sajjad-moradi Does it work properly in your environment?

[sajjad-moradi]

It should, but we're using a custom AccessControl in our environment (auth token is not used)! I just debugged and stepped through AuthenticationFilter for PUT /zk/putChildren endpoint, and it hits the right method for authentication: Maybe something has changed for token based authentication 🤷