Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 )
Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES ...................................................................... Patch Set 5: (2 comments) > Patch Set 5: > > (2 comments) http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc@278 PS5, Line 278: simplify_check_on_show_tables > Do we need a flag for this behavior? Seems like a useful thing to do in any This change the query behavior so I think admin should be aware of it. As shown in the added test, if the user only has INSERT privilege on a table but without SELECT or any other privileges, the table should still be shown in the default behavior. With this flag setting to true, this table won't be shown. http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java@796 PS5, Line 796: PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder( : authzFactory_.getAuthorizableFactory()) : .allOf(requiredPrivilege).onAnyColumn(dbName, tblName, tableOwner).build(); : if (!authzChecker_.get().hasAccess(user, privilegeRequest)) { : iter.remove(); : } > It looks like this code loops over all the implied actions of the given pri Yeah, that's an optimization! But changing the order only helps if the user does have SELECT privilege on the table. If the user doesn't have any privileges, the 8 privileges will still be checked one by one and finally return false. See codes in SentryAuthorizationChecker.authorizeResource(): // The Hive Access API does not currently provide a way to check if the user // has any privileges on a given resource. if (request.getPrivilege().hasAnyOf()) { for (ImpalaAction action: actions) { if (provider_.hasAccess(new Subject(user.getShortName()), authorizables, EnumSet.of(action), request.hasGrantOption(), ActiveRoleSet.ALL)) { return true; } } return false; So if the user doesn't have any privileges on most of the tables, there are still a lot of privilege checks to perform. We still need the flag to bypass 7/8 of the checks. Added this optimization for default behavior. -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Mon, 14 Oct 2019 23:35:36 +0000 Gerrit-HasComments: Yes
