Fang-Yu Rao 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) > Patch Set 6: > > (1 comment) Hi Bikram and Wenzhe, I have verified Wenzhe's explanation and think what Wenzhe described is correct. Let me know if you have any other comments. Thanks! 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); : } : } > Look into the code. The local views will be used for further analysis, audi Thanks to Wenzhe for the detailed explanation! I have checked the code path again and think what Wenzhe described is correct. That is, even we moved the code that registers the local views (that for-loop) and the code that adds access events to the finally block. These 2 pieces of information will not be processed afterwards when there is an AnalysisException thrown. Specifically, once there is an AnalysisException thrown, this exception will be caught at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L416. If later on there is an AuthorizationException thrown, that exception will be caught at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L432. In either case, any statement after analysisCtx.analyzeAndAuthorize() at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/service/Frontend.java#L1536 will not be executed, including the code that would processing the audit events at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/service/Frontend.java#L1540. On the other hand, I am answering my previous question myself. If on the Ranger server we grant the user the SELECT privilege of a non-existing resource, then we will see an AuthorizationException, which is not what I had expected before. I expected to see an AnalysisException. But this issue/phenomenon is already there in Impala and thus is independent of Wenzhe's patch. -- 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 <wz...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Sat, 08 Feb 2020 00:16:27 +0000 Gerrit-HasComments: Yes
