You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2018/08/04 22:00:19 UTC
[2/3] impala git commit: IMPALA-7325: Incorrect SHOW CREATE VIEW with
built-in functions
IMPALA-7325: Incorrect SHOW CREATE VIEW with built-in functions
In the prior code, the authorization checker for the masked privilege
requests skips the check for system database access. As a result, certain
commands, such as SHOW CREATE VIEW that references built-in database
requires permission to access to the built-in database where accessing
built-in database should always be allowed. The patch fixes it by using
the authorizePrivilegeRequest() method that does a check on the system
database similar to how other authorization checks are performed.
Testing:
- Added new authorization test
- Ran all FE tests
Change-Id: Ia164c55fd9459cf5f11eb72561e9cd4ffe1d5367
Reviewed-on: http://gerrit.cloudera.org:8080/10995
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/504e9955
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/504e9955
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/504e9955
Branch: refs/heads/master
Commit: 504e9955a1481425c9fde64f9259d4b16b0f27c5
Parents: 2aa9ca2
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Thu Jul 19 13:44:27 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Sat Aug 4 17:42:07 2018 +0000
----------------------------------------------------------------------
.../apache/impala/analysis/AnalysisContext.java | 4 ++-
.../impala/analysis/AuthorizationStmtTest.java | 29 ++++++++++++++++++++
.../apache/impala/common/FrontendTestBase.java | 11 +++++++-
3 files changed, 42 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/504e9955/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index eeda2eb..47fe8a7 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -564,7 +564,9 @@ public class AnalysisContext {
// These checks don't result in an AuthorizationException but set the
// 'user_has_profile_access' flag in queryCtx_.
for (Pair<PrivilegeRequest, String> maskedReq: analyzer.getMaskedPrivilegeReqs()) {
- if (!authzChecker.hasAccess(analyzer.getUser(), maskedReq.first)) {
+ try {
+ authorizePrivilegeRequest(authzChecker, maskedReq.first);
+ } catch (AuthorizationException e) {
analysisResult_.setUserHasProfileAccess(false);
if (!Strings.isNullOrEmpty(maskedReq.second)) {
throw new AuthorizationException(maskedReq.second);
http://git-wip-us.apache.org/repos/asf/impala/blob/504e9955/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
index 627ec1a..b27227a 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -993,6 +993,30 @@ public class AuthorizationStmtTest extends FrontendTestBase {
// Show create view on non-existent table.
authorize("show create view functional.notbl").error(accessError("functional.notbl"));
+ // IMPALA-7325: show create view that references built-in function(s) should not
+ // require access to the _impala_builtins database this is because reading metadata
+ // in system database should always be allowed.
+ addTestView(authzCatalog_, "create view functional.test_view as " +
+ "select count(*) from functional.alltypes");
+ test = authorize("show create view functional.test_view");
+ for (TPrivilegeLevel privilege: viewMetadataPrivileges()) {
+ test.ok(onServer(privilege, TPrivilegeLevel.SELECT))
+ .ok(onDatabase("functional", privilege, TPrivilegeLevel.SELECT))
+ .ok(onTable("functional", "test_view", privilege),
+ onTable("functional", "alltypes", privilege, TPrivilegeLevel.SELECT));
+ }
+ test.error(accessError("functional.test_view"))
+ .error(accessError("functional.test_view"), onServer(
+ allExcept(viewMetadataPrivileges())))
+ .error(accessError("functional.test_view"), onDatabase("functional",
+ allExcept(viewMetadataPrivileges())))
+ .error(accessError("functional.test_view"), onTable("functional", "test_view",
+ allExcept(viewMetadataPrivileges())), onTable("functional", "alltypes",
+ TPrivilegeLevel.SELECT))
+ .error(viewDefError("functional.test_view"), onTable("functional", "test_view",
+ TPrivilegeLevel.SELECT), onTable("functional", "alltypes", allExcept(
+ viewMetadataPrivileges())));
+
// Show create function.
ScalarFunction fn = addFunction("functional", "f");
try {
@@ -2217,6 +2241,11 @@ public class AuthorizationStmtTest extends FrontendTestBase {
return "Cannot modify system database.";
}
+ private static String viewDefError(String object) {
+ return "User '%s' does not have privileges to see the definition of view '" +
+ object + "'.";
+ }
+
private static String createFunctionError(String object) {
return "User '%s' does not have privileges to CREATE functions in: " + object;
}
http://git-wip-us.apache.org/repos/asf/impala/blob/504e9955/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
index 7b22551..cbb48ce 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
@@ -211,8 +211,17 @@ public class FrontendTestBase {
* Returns the new view.
*/
protected Table addTestView(String createViewSql) {
+ return addTestView(catalog_, createViewSql);
+ }
+
+ /**
+ * Adds a test-local view to the specified catalog based on the given CREATE VIEW sql.
+ * The test views are registered in testTables_ and removed in the @After method.
+ * Returns the new view.
+ */
+ protected Table addTestView(Catalog catalog, String createViewSql) {
CreateViewStmt createViewStmt = (CreateViewStmt) AnalyzesOk(createViewSql);
- Db db = catalog_.getDb(createViewStmt.getDb());
+ Db db = catalog.getDb(createViewStmt.getDb());
Preconditions.checkNotNull(db, "Test views must be created in an existing db.");
// Do not analyze the stmt to avoid applying rewrites that would alter the view
// definition. We want to model real views as closely as possible.