[Bug] Authorization is broken for endpoints which access any resource other than a table

[NihalJain]

While trying Authentication and Authorisation feature of Pinot, I found that for controller, any endpoint which accesses a non-table resource, authorisation does not work as expected using either of the bundled auth options:

1. BasicAuthAccessControlFactory
2. ZkBasicAuthAccessControlFactory

I would consider this a CRITICAL security flaw as it gives a normal user power to do any sort of destructive action in the cluster.

Consider a simple example.

---

Assume we have 3 users in system:

1. admin
2. test
3. user1

---

Now we try to DELETE user1 who is an admin by submitting a request as test who is a normal user

Voila user1 is deleted! :(

---

But the user test was not authorised to do so !!

---

This example is just tip of the iceberg. I plan to work to fix this once the team confirms this is really broken.

[hpvd]

@Jackie-Jiang plz have look

[Jackie-Jiang]

Thanks for reporting this! @soumitra-st Can you help take a look?

[NihalJain]

Thanks for reporting this! @soumitra-st Can you help take a look?

I already have a patch which disallows any user not having appropriate access on ALL tables to do any similar action on cluster level endpoint, which will fix the above issue.

I need some guidance on what is the expected behaviour around the cluster level APIs because as per current logic just providing correct auth is enough to do any action at cluster level.

[soumitra-st]

@NihalJain , Thanks for reporting the issue and highlighting the weakness in BasicAuthAccessControlFactory and ZkBasicAuthAccessControlFactory. Any change in the behavior of these classes will be a backward in-compatible change. We added FineGrainedAccessControl to control AUTHZ at cluster and Table levels. Some of our users have implemented role based access control using this interface.

Can you describe your use case?

[ilamhs]

I have a related PR (#14414) that adds a Http Request object to the *AccessControlFactory and I am realizing any changes in the interface is harder to do. I was wondering if the interface should instead be migrated to boolean hasAccess(AccessControlRequest request) and that request object has all the fields which are required and optional present making any extensions there easier.

Yes, it will be slightly involved to keep it backward compatible in a Deprecated mode till a major version is released. Let me know what you think and how I can proceed with the other PR, thanks!

[soumitra-st]

@ilamhs , have you looked at FineGrainedAccessControl.hasAccess method? This method has access to the HttpHeaders. What information you need to access that is not present in HttpHeaders that requires access to the org.glassfish.grizzly.http.server.Request?

[ilamhs]

I see, that is interesting, I think that could work, I will give it a try soon and report back on the same, thanks @soumitra-st

[soumitra-st]

Check the Actions for cluster and table defined HERE. We have built role-based-access-control using this interface.