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: (4 comments) http://gerrit.cloudera.org:8080/#/c/15123/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15123/6//COMMIT_MSG@7 PS6, Line 7: accessing > nit: accesses will fix it http://gerrit.cloudera.org:8080/#/c/15123/6//COMMIT_MSG@8 PS6, Line 8: any privilege > nit: required privileges will fix it. http://gerrit.cloudera.org:8080/#/c/15123/6//COMMIT_MSG@10 PS6, Line 10: Traced the issue and found that privilege requests collected during : analysis were lost in WithClause::analyze function when analysis : function throw AnalysisException. This caused authorization been : skipped and returned analysis error, instead of authorization error. : This patch register the privilege requests made from root analyzer : to the input analyzer in WithClause::analyze function regardless of : analysis exception. > Currently if a user without required privileges tries to access a non-exist will update as suggested 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); : } : } > Thanks to Wenzhe for the detailed explanation! I have checked the code path So the code change is safe. -- 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: Mon, 10 Feb 2020 19:08:05 +0000 Gerrit-HasComments: Yes
