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 2018/07/18 21:35:39 UTC

[2/5] impala git commit: IMPALA-7059: Inconsistent privilege between DESCRIBE and DESCRIBE DATABASE

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>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/4f2a8158
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/4f2a8158
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/4f2a8158

Branch: refs/heads/master
Commit: 4f2a8158599a4c2201f43d721c93a79c71a67e19
Parents: 9800761
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Wed Jul 11 11:18:05 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Jul 18 03:14:48 2018 +0000

----------------------------------------------------------------------
 .../org/apache/impala/service/Frontend.java     |  5 +-
 .../impala/analysis/AuthorizationStmtTest.java  | 55 ++++++++------------
 2 files changed, 26 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/4f2a8158/fe/src/main/java/org/apache/impala/service/Frontend.java
----------------------------------------------------------------------
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)) {

http://git-wip-us.apache.org/repos/asf/impala/blob/4f2a8158/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 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.