Adam Tamas has posted comments on this change. ( http://gerrit.cloudera.org:8080/16199 )
Change subject: IMPALA-7001: Fix Privilege inconsistency between SHOW TABLES and SHOW FUNCTIONS ...................................................................... Patch Set 4: > Hi Adam, thanks for working on this patch! > > The patch looks good to me since you have implemented what Fredy > had suggested at https://issues.apache.org/jira/browse/IMPALA-7001. > I only have two minor comments regarding the test and the commit > message. > > Specifically, after your patch, a user granted only the privilege > of CREATE on a specified database, e.g., functional, would be able > to execute a statement like "SHOW FUNCTIONS IN functional", since > according to > https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/Privilege.java > and > https://github.com/apache/impala/blob/3a6022ce80ca1cedb629400b18caaf0d1f54137c/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L431-L453, > such a statement would succeed as long as the user is granted any > privilege in the set {ALL, OWNER, ALTER, DROP, CREATE, INSERT, > SELECT, REFRESH}. > > Before your patch, in order for the statement above to succeed, a > user has to be granted any privilege in the set {INSERT, SELECT, > REFRESH}. Thus I think it would be good to add one more test case > in > https://github.com/apache/impala/blob/master/tests/authorization/test_ranger.py, > where we 1) grant the privilege of CREATE to a user (as > admin_client), and 2) execute a statement like "SHOW FUNCTIONS IN > unique_database" to verify there is no exception thrown. > > On the other hand, I think it may also be good to provide more > detail of the difference before and after the patch. For instance, > we could mention that a user granted only the privilege of CREATE > is now able to execute that SQL statement above after this patch, > making it easier for the user to manage the functions it creates. Hi and thank you for the review, I updated it based on the suggestions. -- To view, visit http://gerrit.cloudera.org:8080/16199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9ae7546c206daaf98ecc3de449069027c43c6e1a Gerrit-Change-Number: 16199 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Tamas <[email protected]> Gerrit-Reviewer: Adam Tamas <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Thu, 16 Jul 2020 16:29:55 +0000 Gerrit-HasComments: No
