You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/04/13 00:27:56 UTC

impala git commit: IMPALA-6804: Allow SELECT and INSERT privileges on SERVER

Repository: impala
Updated Branches:
  refs/heads/master 9a751f00b -> 5417e712f


IMPALA-6804: Allow SELECT and INSERT privileges on SERVER

This change add the last two privileges at the SERVER level.

GRANT SELECT ON SERVER TO ROLE test_role;
REVOKE SELECT ON SERVER FROM ROLE test_role;
GRANT INSERT ON SERVER TO ROLE test_role
REVOKE INSERT ON SERVER FROM ROLE test_role

Testing:
Added tests to cover server level select and insert.
Ran all front-end tests.

Change-Id: If6510b4d54c9902029a357f1aebb2570cd75855d
Reviewed-on: http://gerrit.cloudera.org:8080/9972
Reviewed-by: Alex Behm <al...@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/5417e712
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/5417e712
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/5417e712

Branch: refs/heads/master
Commit: 5417e712f33dfb963b4646723ec173a5f3c4f49b
Parents: 9a751f0
Author: Adam Holley <gi...@holleyism.com>
Authored: Tue Apr 10 11:29:55 2018 -0500
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Thu Apr 12 23:57:54 2018 +0000

----------------------------------------------------------------------
 .../apache/impala/analysis/PrivilegeSpec.java   |   8 -
 .../impala/analysis/AnalyzeAuthStmtsTest.java   |   8 +-
 .../impala/analysis/AuthorizationTest.java      | 192 +++++++++++++++++--
 3 files changed, 179 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/5417e712/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
index ec292a2..d848383 100644
--- a/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
+++ b/fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
@@ -188,14 +188,6 @@ public class PrivilegeSpec implements ParseNode {
 
     switch (scope_) {
       case SERVER:
-        if (privilegeLevel_ != TPrivilegeLevel.ALL &&
-            privilegeLevel_ != TPrivilegeLevel.REFRESH &&
-            privilegeLevel_ != TPrivilegeLevel.CREATE &&
-            privilegeLevel_ != TPrivilegeLevel.ALTER &&
-            privilegeLevel_ != TPrivilegeLevel.DROP) {
-          throw new AnalysisException("Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or " +
-              "'DROP' privilege may be applied at SERVER scope in privilege spec.");
-        }
         break;
       case DATABASE:
         Preconditions.checkState(!Strings.isNullOrEmpty(dbName_));

http://git-wip-us.apache.org/repos/asf/impala/blob/5417e712/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
index c56102f..2741e5d 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
@@ -164,9 +164,7 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest {
           formatArgs));
       AnalyzesOk(String.format("%s INSERT ON DATABASE functional %s myrole",
           formatArgs));
-      AnalysisError(String.format("%s INSERT ON SERVER %s myrole", formatArgs),
-          "Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or 'DROP' privilege may be " +
-          "applied at SERVER scope in privilege spec.");
+      AnalyzesOk(String.format("%s INSERT ON SERVER %s myrole", formatArgs));
       AnalysisError(String.format("%s INSERT ON URI 'hdfs:////abc//123' %s myrole",
           formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " +
           "spec.");
@@ -180,9 +178,7 @@ public class AnalyzeAuthStmtsTest extends AnalyzerTest {
           "%s SELECT ON TABLE functional_kudu.alltypessmall %s myrole", formatArgs));
       AnalyzesOk(String.format("%s SELECT ON DATABASE functional %s myrole",
           formatArgs));
-      AnalysisError(String.format("%s SELECT ON SERVER %s myrole", formatArgs),
-          "Only 'ALL', 'REFRESH', 'CREATE', 'ALTER', or 'DROP' privilege may be " +
-          "applied at SERVER scope in privilege spec.");
+      AnalyzesOk(String.format("%s SELECT ON SERVER %s myrole", formatArgs));
       AnalysisError(String.format("%s SELECT ON URI 'hdfs:////abc//123' %s myrole",
           formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " +
           "spec.");

http://git-wip-us.apache.org/repos/asf/impala/blob/5417e712/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
index a5173fb..9a62e93 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -612,9 +612,9 @@ public class AuthorizationTest extends FrontendTestBase {
     AuthzOk("select * from functional.view_view");
 
     // User does not have SELECT privileges on this view.
-    AuthzError("select * from functional.complex_view_sub",
+    AuthzError("select * from functional.alltypes_view",
         "User '%s' does not have privileges to execute 'SELECT' on: " +
-        "functional.complex_view_sub");
+        "functional.alltypes_view");
 
     // User has SELECT privileges on the view and the join table.
     AuthzOk("select a.id from functional.view_view a "
@@ -648,11 +648,6 @@ public class AuthorizationTest extends FrontendTestBase {
         "User '%s' does not have privileges to execute 'SELECT' on: " +
         "functional.alltypes");
 
-    // Select with no privileges on view.
-    AuthzError("select * from functional.complex_view_sub",
-        "User '%s' does not have privileges to execute 'SELECT' on: " +
-        "functional.complex_view_sub");
-
     // Select without referencing a column.
     AuthzError("select 1 from functional.alltypes",
         "User '%s' does not have privileges to execute 'SELECT' on: " +
@@ -711,7 +706,7 @@ public class AuthorizationTest extends FrontendTestBase {
     AuthzOk("select id, b.key from functional.allcomplextypes a, a.int_map_col b");
 
     // No SELECT privileges on 'alltypessmall'
-    AuthzError("select a.* from functional.alltypesagg cross join " +
+    AuthzError("select a.* from functional.alltypesagg a cross join " +
         "functional.alltypessmall b", "User '%s' does not have privileges to execute " +
         "'SELECT' on: functional.alltypessmall");
 
@@ -765,11 +760,15 @@ public class AuthorizationTest extends FrontendTestBase {
         "functional.alltypes");
 
     // User doesn't have permissions on source table within inline view.
-    AuthzError("insert into functional.alltypes " +
-        "select * from functional.alltypesagg a join (select * from " +
-        "functional_seq.alltypes) b on (a.int_col = b.int_col)",
-        "User '%s' does not have privileges to execute 'SELECT' on: " +
-        "functional_seq.alltypes");
+    AuthzError("insert into functional.alltypes partition (month, year) " +
+        "select id, bool_col, tinyint_col, smallint_col, a.int_col, bigint_col, " +
+        "float_col, a.double_col, a.date_string_col, a.string_col, a.timestamp_col, " +
+        "a.year, a.month " +
+        "from functional_rc.alltypesagg a " +
+        "join (select int_col, double_col, date_string_col, string_col, timestamp_col, " +
+        "year, month from functional_seq.alltypes) b " +
+        "on (a.int_col = b.int_col)", "User '%s' does " +
+        "not have privileges to execute 'SELECT' on: functional_rc.alltypesagg");
 
     // User doesn't have INSERT permissions on the target table but has sufficient SELECT
     // permissions on all the referenced columns of the source table
@@ -1762,8 +1761,8 @@ public class AuthorizationTest extends FrontendTestBase {
     // Column level privileges will allow describe but with reduced data.
     AuthzOk("describe formatted functional.alltypessmall");
     // Insufficient privileges on table for nested column.
-    AuthzError("describe functional.complextypestbl.nested_struct",
-        "User '%s' does not have privileges to access: functional.complextypestbl");
+    AuthzError("describe functional.complextypes_fileformat.s",
+        "User '%s' does not have privileges to access: functional.complextypes_fileformat");
     // Insufficient privileges on view.
     AuthzError("describe functional.alltypes_view_sub",
         "User '%s' does not have privileges to access: functional.alltypes_view_sub");
@@ -2721,6 +2720,169 @@ public class AuthorizationTest extends FrontendTestBase {
   }
 
   @Test
+  public void TestServerLevelInsert() throws ImpalaException {
+    // TODO: Add test support for dynamically changing privileges for
+    // file-based policy.
+    if (ctx_.authzConfig.isFileBasedPolicy()) return;
+
+    SentryPolicyService sentryService =
+        new SentryPolicyService(ctx_.authzConfig.getSentryConfig());
+
+    // User has INSERT privilege on server.
+    String roleName = "insert_role";
+    try {
+      sentryService.createRole(USER, roleName, true);
+      TPrivilege privilege = new TPrivilege("", TPrivilegeLevel.INSERT,
+          TPrivilegeScope.SERVER, false);
+      privilege.setServer_name("server1");
+      sentryService.grantRolePrivilege(USER, roleName, privilege);
+      sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+
+      privilege = new TPrivilege ("", TPrivilegeLevel.SELECT, TPrivilegeScope.DATABASE,
+          false);
+      privilege.setServer_name("server1");
+      privilege.setDb_name("functional_rc");
+      sentryService.grantRolePrivilege(USER, roleName, privilege);
+      sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+
+      privilege = new TPrivilege ("", TPrivilegeLevel.SELECT, TPrivilegeScope.DATABASE,
+          false);
+      privilege.setServer_name("server1");
+      privilege.setDb_name("functional_seq");
+      sentryService.grantRolePrivilege(USER, roleName, privilege);
+      sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+      ctx_.catalog.reset();
+
+      // Copied majority of INSERT tests that should fail from authorization and
+      // ensure they succeed here. Some tests that fail will fail here with
+      // AnalysisException and are covered elsewhere.
+
+      // User has SELECT permissions on source table.
+      AuthzOk("insert into functional.alltypes partition (month, year) " +
+              "select * from functional_rc.alltypes");
+
+      // Ensure INSERT at server does not allow other privileges
+      AuthzError("select * from functional.alltypes",
+          "User '%s' does not have privileges to execute 'SELECT' on: functional");
+      AuthzError("create table functional.new_table (i int)",
+          "User '%s' does not have privileges to execute 'CREATE' on: functional");
+      AuthzError("alter table functional.alltypes add columns (c1 int)",
+          "User '%s' does not have privileges to execute 'ALTER' on: " +
+          "functional.alltypes");
+      AuthzError("drop table functional.alltypes",
+          "User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
+
+      // User has permissions on source table within inline view.
+      AuthzOk("insert into functional.alltypes partition (month, year) " +
+          "select id, bool_col, tinyint_col, smallint_col, a.int_col, bigint_col, " +
+          "float_col, a.double_col, a.date_string_col, a.string_col, a.timestamp_col, " +
+          "a.year, a.month from functional_rc.alltypesagg a " +
+          "join (select int_col, double_col, date_string_col, string_col, " +
+          "timestamp_col, year, month from functional_seq.alltypes) b " +
+          "on (a.int_col = b.int_col)");
+
+      // User has INSERT permissions on the target table and has sufficient SELECT
+      // permissions on all the referenced columns of the source table
+      AuthzOk("insert into functional.alltypestiny partition (month, year) " +
+          "select * from functional_rc.alltypestiny");
+
+      // User has INSERT permissions on target table and column-level
+      // permissions on the source table
+      AuthzOk("insert into functional.alltypes partition (month, year) " +
+          "select * from functional_rc.alltypessmall");
+
+      // Insert and Select allow view_metadata
+      AuthzOk("describe database functional_rc");
+      AuthzOk("describe functional.complextypes_fileformat.s");
+      AuthzOk("describe functional.alltypes_view_sub");
+      AuthzOk("describe functional_rc.alltypes");
+    } finally {
+      sentryService.dropRole(USER, roleName, true);
+      ctx_.catalog.reset();
+    }
+  }
+
+  @Test
+  public void TestServerLevelSelect() throws ImpalaException {
+    // TODO: Add test support for dynamically changing privileges for
+    // file-based policy.
+    if (ctx_.authzConfig.isFileBasedPolicy()) return;
+
+    SentryPolicyService sentryService =
+        new SentryPolicyService(ctx_.authzConfig.getSentryConfig());
+
+    // User has SELECT privilege on server.
+    String roleName = "select_role";
+    try {
+      sentryService.createRole(USER, roleName, true);
+      TPrivilege privilege = new TPrivilege("", TPrivilegeLevel.SELECT,
+          TPrivilegeScope.SERVER, false);
+      privilege.setServer_name("server1");
+      sentryService.grantRolePrivilege(USER, roleName, privilege);
+      sentryService.grantRoleToGroup(USER, roleName, USER.getName());
+      ctx_.catalog.reset();
+
+      // Copied majority of SELECT tests that should fail from authorization and
+      // ensure they succeed here.
+      AuthzOk("select * from functional.alltypes_view_sub");
+      AuthzOk("select * from functional.alltypes");
+
+      // Ensure SELECT at server does not allow other privileges
+      AuthzError("insert into functional.alltypesagg select 1",
+          "User '%s' does not have privileges to execute 'INSERT' on: " +
+          "functional.alltypesagg");
+      AuthzError("create table functional.new_table (i int)",
+          "User '%s' does not have privileges to execute 'CREATE' on: functional");
+      AuthzError("alter table functional.alltypes add columns (c1 int)",
+          "User '%s' does not have privileges to execute 'ALTER' on: " +
+          "functional.alltypes");
+      AuthzError("drop table functional.alltypes",
+          "User '%s' does not have privileges to execute 'DROP' on: functional.alltypes");
+
+      // User has SELECT privileges on the view, and has privileges
+      // to select join table.
+      AuthzOk("select a.id from functional.view_view a " +
+          "join functional.alltypes b ON (a.id = b.id)");
+
+      // User has SELECT privileges on the view which contains a subquery.
+      AuthzOk("select * from functional_rc.subquery_view");
+
+      // Constant select.
+      AuthzOk("select 1");
+
+      // Table within inline view is authorized properly.
+      AuthzOk("select a.* from (select * from functional.alltypes) a");
+
+      // SELECT privileges on all the columns of 'alltypessmall'
+      AuthzOk("select * from functional.alltypessmall");
+
+      // SELECT privileges on table 'alltypessmall'
+      AuthzOk("select count(*) from functional.alltypessmall");
+      AuthzOk("select 1 from functional.alltypessmall");
+
+      // SELECT privileges on column 'month'
+      AuthzOk("select id, int_col, year, month from functional.alltypessmall");
+
+      // SELECT privileges on 'int_array_col'
+      AuthzOk("select a.id, b.item from functional.allcomplextypes a, " +
+          "a.int_array_col b");
+
+      // SELECT privileges on 'alltypessmall'
+      AuthzOk("select a.* from functional.alltypesagg a cross join " +
+          "functional.alltypessmall b");
+
+      // Insert allows view_metadata
+      AuthzOk("describe database functional_rc");
+      AuthzOk("describe functional.complextypes_fileformat.s");
+      AuthzOk("describe functional.alltypes_view_sub");
+      AuthzOk("describe functional_rc.alltypes");
+    } finally {
+      sentryService.dropRole(USER, roleName, true);
+      ctx_.catalog.reset();
+    }
+  }
+
+  @Test
   public void TestServerLevelCreate() throws ImpalaException {
     // TODO: Add test support for dynamically changing privileges for
     // file-based policy.