You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ar...@apache.org on 2019/05/29 23:28:26 UTC

[impala] 04/04: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

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

arodoni pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit ac13cb667edff8d9b91d8bf48940dbe5b2eb19db
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Wed Jul 11 11:18:05 2018 -0700

    IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE
    
    In DESCRIBE DATABASE, having VIEW_METADATA privilege allows seeing the
    metadata information on the target database. Similarly, other SQL show
    commands require VIEW_METADATA privilege on the target database/table.
    In the prior code, DESCRIBE requires SELECT privilege on the target table
    and is inconsistent with the rest of other SQL metadata commands. The
    patch fixes the inconsistency by requiring DESCRIBE to use VIEW_METADATA
    privilege.
    
    Testing:
    - Updated authorization tests
    - Ran all FE tests
    
    Change-Id: I37d1610a922741a6c95059c3beb7d04eb507783f
    Reviewed-on: http://gerrit.cloudera.org:8080/10923
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../java/org/apache/impala/service/Frontend.java   |  5 +-
 .../impala/analysis/AuthorizationStmtTest.java     | 55 +++++++++-------------
 2 files changed, 26 insertions(+), 34 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index a363458..ac65171 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -813,7 +813,8 @@ public class Frontend {
     if (authzConfig_.isEnabled()) {
       // First run a table check
       PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
-          .allOf(Privilege.SELECT).onTable(table.getDb().getName(), table.getName())
+          .allOf(Privilege.VIEW_METADATA).onTable(table.getDb().getName(),
+              table.getName())
           .toRequest();
       if (!authzChecker_.get().hasAccess(user, privilegeRequest)) {
         // Filter out columns that the user is not authorized to see.
@@ -821,7 +822,7 @@ public class Frontend {
         for (Column col: table.getColumnsInHiveOrder()) {
           String colName = col.getName();
           privilegeRequest = new PrivilegeRequestBuilder()
-              .allOf(Privilege.SELECT)
+              .allOf(Privilege.VIEW_METADATA)
               .onColumn(table.getDb().getName(), table.getName(), colName)
               .toRequest();
           if (authzChecker_.get().hasAccess(user, privilegeRequest)) {
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 52e0952..df46899 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -1041,20 +1041,19 @@ public class AuthorizationStmtTest extends FrontendTestBase {
     TTableName tableName = new TTableName("functional", "alltypes");
     TDescribeOutputStyle style = TDescribeOutputStyle.MINIMAL;
     authzTest = authorize("describe functional.alltypes");
-    for (TPrivilegeLevel privilege: new TPrivilegeLevel[]{
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT}) {
+    for (TPrivilegeLevel privilege: viewMetadataPrivileges()) {
       authzTest.okDescribe(tableName, style, ALLTYPES_COLUMNS, null, onServer(privilege))
           .okDescribe(tableName, style, ALLTYPES_COLUMNS, null, onDatabase("functional",
               privilege))
           .okDescribe(tableName, style, ALLTYPES_COLUMNS, null, onTable("functional",
               "alltypes", privilege));
     }
-    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onServer(allExcept(
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
+    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onServer(
+        allExcept(viewMetadataPrivileges())))
         .okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onDatabase("functional",
-            allExcept(TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
+            allExcept(viewMetadataPrivileges())))
         .okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onTable("functional",
-            "alltypes", allExcept(TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
+            "alltypes", allExcept(viewMetadataPrivileges())))
         // In this test, since we only have column level privileges on "id", then
         // only the "id" column should show and the others should not.
         .okDescribe(tableName, style, new String[]{"id"}, ALLTYPES_COLUMNS_WITHOUT_ID,
@@ -1064,26 +1063,24 @@ public class AuthorizationStmtTest extends FrontendTestBase {
     // Describe table extended.
     tableName = new TTableName("functional", "alltypes");
     style = TDescribeOutputStyle.EXTENDED;
-    String[] locationString = new String[]{"Location:"};
     String[] checkStrings = (String[]) ArrayUtils.addAll(ALLTYPES_COLUMNS,
-        locationString);
+        new String[]{"Location:"});
     authzTest = authorize("describe functional.alltypes");
-    for (TPrivilegeLevel privilege: new TPrivilegeLevel[]{
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT}) {
+    for (TPrivilegeLevel privilege: viewMetadataPrivileges()) {
       authzTest.okDescribe(tableName, style, checkStrings, null, onServer(privilege))
           .okDescribe(tableName, style, checkStrings, null, onDatabase("functional",
               privilege))
           .okDescribe(tableName, style, checkStrings, null, onTable("functional",
               "alltypes", privilege));
     }
-    authzTest.okDescribe(tableName, style, locationString, ALLTYPES_COLUMNS,
-        onServer(allExcept(TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
-        .okDescribe(tableName, style, locationString, ALLTYPES_COLUMNS,
-            onDatabase("functional", allExcept(TPrivilegeLevel.ALL,
-            TPrivilegeLevel.SELECT)))
-        .okDescribe(tableName, style, locationString, ALLTYPES_COLUMNS,
-            onTable("functional", "alltypes", allExcept(TPrivilegeLevel.ALL,
-            TPrivilegeLevel.SELECT)))
+    // Describe table without VIEW_METADATA privilege should not show all columns and
+    // location.
+    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS,
+            onServer(allExcept(viewMetadataPrivileges())))
+        .okDescribe(tableName, style, null, ALLTYPES_COLUMNS,
+            onDatabase("functional", allExcept(viewMetadataPrivileges())))
+        .okDescribe(tableName, style, null, ALLTYPES_COLUMNS,
+            onTable("functional", "alltypes", allExcept(viewMetadataPrivileges())))
         // Location should not appear with only column level auth.
         .okDescribe(tableName, style, new String[]{"id"},
             (String[]) ArrayUtils.addAll(ALLTYPES_COLUMNS_WITHOUT_ID,
@@ -1095,20 +1092,17 @@ public class AuthorizationStmtTest extends FrontendTestBase {
     tableName = new TTableName("functional", "alltypes_view");
     style = TDescribeOutputStyle.MINIMAL;
     authzTest = authorize("describe functional.alltypes_view");
-    for (TPrivilegeLevel privilege: new TPrivilegeLevel[]{
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT}) {
+    for (TPrivilegeLevel privilege: viewMetadataPrivileges()) {
       authzTest.okDescribe(tableName, style, ALLTYPES_COLUMNS, null, onServer(privilege))
           .okDescribe(tableName, style, ALLTYPES_COLUMNS, null, onDatabase("functional",
               privilege))
           .okDescribe(tableName, style, ALLTYPES_COLUMNS, null, onTable("functional",
               "alltypes_view", privilege));
     }
-    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onServer(allExcept(
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
+    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onServer(
+        allExcept(viewMetadataPrivileges())))
         .okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onDatabase("functional",
-            allExcept(TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
-        .okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onTable("functional",
-            "alltypes_view", TPrivilegeLevel.INSERT))
+            allExcept(viewMetadataPrivileges())))
         .error(accessError("functional.alltypes_view"));
 
     // Describe view extended.
@@ -1118,20 +1112,17 @@ public class AuthorizationStmtTest extends FrontendTestBase {
     String[] viewStrings = new String[]{"View Original Text:", "View Expanded Text:"};
     checkStrings = (String[]) ArrayUtils.addAll(ALLTYPES_COLUMNS, viewStrings);
     authzTest = authorize("describe functional.alltypes_view");
-    for (TPrivilegeLevel privilege: new TPrivilegeLevel[]{
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT}) {
+    for (TPrivilegeLevel privilege: viewMetadataPrivileges()) {
       authzTest.okDescribe(tableName, style, checkStrings, null, onServer(privilege))
           .okDescribe(tableName, style, checkStrings, null, onDatabase("functional",
               privilege))
           .okDescribe(tableName, style, checkStrings, null, onTable("functional",
               "alltypes_view", privilege));
     }
-    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onServer(allExcept(
-        TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
+    authzTest.okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onServer(
+        allExcept(viewMetadataPrivileges())))
         .okDescribe(tableName, style, null, ALLTYPES_COLUMNS, onDatabase("functional",
-            allExcept(TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT)))
-        .okDescribe(tableName, style, viewStrings, ALLTYPES_COLUMNS, onTable("functional",
-            "alltypes_view", TPrivilegeLevel.INSERT))
+            allExcept(viewMetadataPrivileges())))
         .error(accessError("functional.alltypes_view"));
 
     // Describe specific column on a table.