Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/18684 )
Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers ...................................................................... Patch Set 2: (4 comments) Hi all, I have revised the patch according to Quanlong's comments in the previous iteration. Let me know if you have any other suggestion. Thank you very much for the help! http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1568 PS1, Line 1568: # resource from a role, but we do not have to handle such an exception. We only > Should we hide the table name if the user doesn't have VIEW_METADATA privil Thanks Quanlong! There are only 2 columns in the view 'complex_view' in total. When we do not grant the requesting user the SELECT privilege on the view 'complex_view', an AuthorizationException will be thrown and caught at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L272. Impala would then check whether the requesting user has the SELECT privilege on each column in this view. Since in the test case, we granted the requesting user the SELECT privileges on both columns in the view 'complex_view', in the end BaseAuthorizationChecker#authorizeTableAccess() does not throw an AuthorizationException for this view. Impala's FE then continued to perform the privilege checks for the next table (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L166), which was the table 'functional.alltypesagg'. The requesting user was not granted any privilege on this table and thus an AuthorizationException was thrown. I think this behavior is expected and reasonable. If we only granted the requesting user the SELECT privilege on a single column in the view 'complex_view', then Impala's FE would throw the following error. ERROR: AuthorizationException: User 'non_owner' does not have privileges to execute 'SELECT' on: functional.complex_view http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1569 PS1, Line 1569: need to make sure in > Not sure how the table is picked if there are more than one table fails the Thanks Quanlong! It depends on the order in which 'tablePrivReqs' is iterated at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L165. I will revise the test accordingly to make sure the reported error always corresponds to the same table. http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1572 PS1, Line 1572: pass > Can we add a test after this line for the same query? Just checking the cas Yes. I will add a test after this line in the next patch. http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1582 PS1, Line 1582: impalad_args=LOCAL_CATALOG_IMPALAD_ARGS, > Can we add another test that the alltypesagg table is granted select but on Yes. We can add such a test. In the next patch, I will grant the requesting user the SELECT privilege on the table 'alltypesagg' and add a deny policy to prevent the requesting user from accessing the column 'id'. The error message would look like the following. ERROR: AuthorizationException: User 'non_owner' does not have privileges to execute 'SELECT' on: functional.alltypesagg On a related note, when a deny policy is created via Ranger's REST API, it looks like Ranger would perform some semantic check to make sure no existing policy is associated with the same resource. In our case, it would be the column 'id' of the table 'functional.alltypesagg'. That means we may not always be able to add such a deny policy if there already exists a policy associated with the same column in Ranger. But we can wait and see if the deny policy we choose here could be successfully added later on. -- To view, visit http://gerrit.cloudera.org:8080/18684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad Gerrit-Change-Number: 18684 Gerrit-PatchSet: 2 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vincent Tran <[email protected]> Gerrit-Comment-Date: Sat, 02 Jul 2022 00:55:01 +0000 Gerrit-HasComments: Yes
