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 <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> 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 <[email protected]> Authored: Wed Jul 11 11:18:05 2018 -0700 Committer: Impala Public Jenkins <[email protected]> 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.
