You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2020/07/20 09:36:41 UTC

[impala] 02/02: IMPALA-7001: Fix Privilege inconsistency between SHOW TABLES and SHOW FUNCTIONS

This is an automated email from the ASF dual-hosted git repository.

stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 472f49705d9db315e5704b49e3eda7eed95bd921
Author: Adam Tamas <ta...@cloudera.com>
AuthorDate: Tue Jul 14 09:20:07 2020 +0200

    IMPALA-7001: Fix Privilege inconsistency between SHOW TABLES and SHOW FUNCTIONS
    
    In "show tables" ANY privilege was used, whereas in "show functions"
    the required privilege was VIEW_METADATA.
    To solve the inconsistency "show functions" will use ANY instead of
    VIEW_METADATA similar to "show tables".
    
    After this, an user granted only the privilege of CREATE is now able to
    execute "show functions" after this patch, making it easier for the
    user to manage the functions it creates.
    
    Testing:
    -Ran CORE tests.
    -Added new tests to check the privilege.
    
    Change-Id: I9ae7546c206daaf98ecc3de449069027c43c6e1a
    Reviewed-on: http://gerrit.cloudera.org:8080/16199
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../apache/impala/analysis/AnalysisContext.java    |  2 +-
 .../apache/impala/analysis/ShowFunctionsStmt.java  |  2 +-
 .../org/apache/impala/analysis/AuditingTest.java   | 14 ++++++-
 tests/authorization/test_ranger.py                 | 43 ++++++++++++++++++++++
 4 files changed, 58 insertions(+), 3 deletions(-)

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 df68afb..c2c09cc 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -204,7 +204,7 @@ public class AnalysisContext {
      */
     public boolean isSingleColumnPrivStmt() {
       return isDescribeTableStmt() || isResetMetadataStmt() || isUseStmt()
-          || isShowTablesStmt() || isAlterTableStmt();
+          || isShowTablesStmt() || isAlterTableStmt() || isShowFunctionsStmt();
     }
 
     public AlterTableStmt getAlterTableStmt() {
diff --git a/fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java b/fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
index 68e9aa4..cb74d4a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
@@ -80,7 +80,7 @@ public class ShowFunctionsStmt extends StatementBase {
   @Override
   public void analyze(Analyzer analyzer) throws AnalysisException {
     postAnalysisDb_ = (parsedDb_ == null ? analyzer.getDefaultDb() : parsedDb_);
-    if (analyzer.getDb(postAnalysisDb_, Privilege.VIEW_METADATA) == null) {
+    if (analyzer.getDb(postAnalysisDb_, Privilege.ANY) == null) {
       throw new AnalysisException(Analyzer.DB_DOES_NOT_EXIST_ERROR_MSG + postAnalysisDb_);
     }
   }
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
index 4310c50..17d2bfa 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
@@ -355,7 +355,7 @@ public class AuditingTest extends FrontendTestBase {
   }
 
   @Test
-  public void TestShow() throws AnalysisException, AuthorizationException{
+  public void TestShowViewMetadata() throws AnalysisException, AuthorizationException{
     String[] statsQuals = new String[]{ "partitions", "table stats", "column stats" };
     for (String qual: statsQuals) {
       Set<TAccessEvent> accessEvents =
@@ -366,6 +366,18 @@ public class AuditingTest extends FrontendTestBase {
   }
 
   @Test
+  public void TestShowAny() throws AnalysisException, AuthorizationException {
+    String[] statsQuals = new String[] { "tables", "functions" };
+    for (String qual : statsQuals) {
+        Set<TAccessEvent> accessEvents = AnalyzeAccessEvents(String.format(
+            "show %s in functional", qual));
+        Assert.assertEquals(accessEvents,
+            Sets.newHashSet(new TAccessEvent("functional", TCatalogObjectType
+            .DATABASE, "ANY")));
+    }
+  }
+
+  @Test
   public void TestShowCreateTable() throws AuthorizationException, AnalysisException {
     Set<TAccessEvent> accessEvents =
         AnalyzeAccessEvents("show create table functional.alltypesagg");
diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py
index 42c04d3..1f7e8a9 100644
--- a/tests/authorization/test_ranger.py
+++ b/tests/authorization/test_ranger.py
@@ -815,6 +815,49 @@ class TestRanger(CustomClusterTestSuite):
       pytest.xfail("getTableIfCached() faulty behavior, known issue")
       self._test_ownership()
 
+  @CustomClusterTestSuite.with_args(
+    impalad_args=IMPALAD_ARGS, catalogd_args=CATALOGD_ARGS)
+  def test_show_functions(self, unique_name):
+    user1 = getuser()
+    admin_client = self.create_impala_client()
+    unique_database = unique_name + "_db"
+    privileges = ["ALTER", "DROP", "CREATE", "INSERT", "SELECT", "REFRESH"]
+    fs_prefix = os.getenv("FILESYSTEM_PREFIX") or str()
+    try:
+      # Set-up temp database + function
+      admin_client.execute("drop database if exists {0} cascade".format(unique_database),
+                           user=ADMIN)
+      admin_client.execute("create database {0}".format(unique_database), user=ADMIN)
+      self.execute_query_expect_success(admin_client, "create function {0}.foo() RETURNS"
+                                      " int LOCATION '{1}/test-warehouse/libTestUdfs.so'"
+                                      "SYMBOL='Fn'".format(unique_database, fs_prefix),
+                                      user=ADMIN)
+      # Check "show functions" with no privilege granted.
+      result = self._run_query_as_user("show functions in {0}".format(unique_database),
+          user1, False)
+      err = "User '{0}' does not have privileges to access: {1}.*.*". \
+          format(user1, unique_database)
+      assert err in str(result)
+      for privilege in privileges:
+        try:
+          # Grant privilege
+          self.execute_query_expect_success(admin_client,
+                                            "grant {0} on database {1} to user {2}"
+                                            .format(privilege, unique_database, user1),
+                                            user=ADMIN)
+          # Check with current privilege
+          result = self._run_query_as_user("show functions in {0}"
+                                          .format(unique_database), user1, True)
+          assert "foo()", str(result)
+        finally:
+          # Revoke privilege
+          admin_client.execute("revoke {0} on database {1} from user {2}"
+                              .format(privilege, unique_database, user1), user=ADMIN)
+    finally:
+      # Drop database
+      self._run_query_as_user("drop database {0} cascade".format(unique_database),
+                              ADMIN, True)
+
   def _test_ownership(self):
     """Tests ownership privileges for databases and tables with ranger along with
     some known quirks in the implementation."""