Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/15123 )
Change subject: IMPALA-7002: Throw AuthorizationException when user accessing non-existent table/database in CTE without any privilege. ...................................................................... Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/15123/6/fe/src/main/java/org/apache/impala/analysis/WithClause.java File fe/src/main/java/org/apache/impala/analysis/WithClause.java: http://gerrit.cloudera.org:8080/#/c/15123/6/fe/src/main/java/org/apache/impala/analysis/WithClause.java@96 PS6, Line 96: finally { : // Register all privilege requests made from the root analyzer to the input : // analyzer so that caller could do authorization for all the requests collected : // during analysis and report an authorization error over an analysis error. : // We should not accidentally reveal the non-existence of a database/table if : // the user is not authorized. : for (PrivilegeRequest req : withClauseAnalyzer.getPrivilegeReqs()) { : analyzer.registerPrivReq(req); : } : } > After taking a closer look at this patch, I have a question about what we s Look into the code. The local views will be used for further analysis, audit events will be used to create base exec request after analyzeAndAuthorize() is successfully return. If the user has been granted to access the whole server, authzChecker.authorize() will not throw exception for non existing database/table. Then analyzeAndAuthorize() will throw AnalysisException. This cause frontend to return analysis error without any further analysis, nor executing the request. The local views and audit events will not be referenced after analyze() throw AnalysisException. So it's not necessary to register local view and add events in the finally block in WithClause.analyze(). -- To view, visit http://gerrit.cloudera.org:8080/15123 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia6b657a7147a136198a9a97a679c9131ee814577 Gerrit-Change-Number: 15123 Gerrit-PatchSet: 6 Gerrit-Owner: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Fri, 07 Feb 2020 21:46:47 +0000 Gerrit-HasComments: Yes
