[GitHub] [pinot] amrishlal commented on pull request #8557: Adding DML definition and parse SQL InsertFile

[GitBox <gi...@apache.org>]

amrishlal commented on PR #8557:
URL: https://github.com/apache/pinot/pull/8557#issuecomment-1103301917

   Looks good, but would be useful to have a few unit and / or integration tests. Also, `AccessControl.hasAccess(...)` methods are currently being called in BaseBrokerREquestHandler:
   
   ```
   Line 189:
       // First-stage access control to prevent unauthenticated requests from using up resources. Secondary table-level
       // check comes later.
       boolean hasAccess = _accessControlFactory.create().hasAccess(requesterIdentity);
   
   Line 295:
       // Second-stage table-level access control
       boolean hasTableAccess = _accessControlFactory.create().hasAccess(requesterIdentity, serverBrokerRequest);
   ```
   
   I am wondering if something similar can be done for all DML/DDL statements as well to avoid either accidental or malicious usecases where a user who normally runs SELECT statements ends up running an INSERT statement with valid S3 credential to insert bad data into a Pinot table (?).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org