Repository: sentry Updated Branches: refs/heads/master a7b68f62c -> 2c9a927a9
SENTRY-2409: ALTER TABLE SET OWNER does not allow to change the table if using only the table name Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/2c9a927a Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/2c9a927a Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/2c9a927a Branch: refs/heads/master Commit: 2c9a927a9e87cba0e4c0f34fc0b55887c6636927 Parents: a7b68f6 Author: lina.li <[email protected]> Authored: Wed Sep 19 23:09:33 2018 -0500 Committer: lina.li <[email protected]> Committed: Tue Oct 2 12:02:59 2018 -0500 ---------------------------------------------------------------------- .../binding/hive/HiveAuthzBindingHook.java | 12 ++ .../hive/authz/HiveAuthzPrivilegesMap.java | 2 +- .../e2e/dbprovider/TestOwnerPrivileges.java | 181 ++++++++++++++++++- 3 files changed, 188 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/2c9a927a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java index 6731d1a..a4002b7 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java @@ -276,6 +276,18 @@ public class HiveAuthzBindingHook extends HiveAuthzBindingHookBase { ASTNode newTableNode = (ASTNode)childASTNode.getChild(0); currOutDB = extractDatabase(newTableNode); } + + // only get DB and table name from TOK_TABNAME for "ALTER TABLE SET OWNER" command + // otherwise, it is possible to mistake output table name as input table name. Ideally, + // the root token type of the command should be "TOK_ALTERTABLE_OWNER", not "TOK_ALTERTABLE" + String command = context.getCommand(); + if (command != null && command.toLowerCase().contains("set owner") && + "TOK_TABNAME".equals(childASTNode.getText())) { + + extractDbTableNameFromTOKTABLE(childASTNode); + currDB = currOutDB; + currTab = currOutTab; + } } break; http://git-wip-us.apache.org/repos/asf/sentry/blob/2c9a927a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java index 385ca98..1aaa9b3 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java @@ -120,7 +120,7 @@ public class HiveAuthzPrivilegesMap { /* TODO: once HIVE-18762 is in the hiver version integrated with Sentry, uncomment this block HiveAuthzPrivileges alterTableSetOwnerPrivilege = new HiveAuthzPrivileges.AuthzPrivilegeBuilder(). - addInputObjectPriviledge(AuthorizableType.Table, EnumSet.of(DBModelAction.ALL)). + addOutputObjectPriviledge(AuthorizableType.Table, EnumSet.of(DBModelAction.ALL)). setOperationScope(HiveOperationScope.TABLE). setOperationType(HiveOperationType.DDL). setGrantOption(true). http://git-wip-us.apache.org/repos/asf/sentry/blob/2c9a927a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java index 55a79ee..880fa94 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java @@ -35,6 +35,7 @@ import java.util.Set; import org.apache.commons.lang.StringUtils; import org.apache.hadoop.fs.permission.FsAction; +import org.apache.hive.service.cli.HiveSQLException; import org.apache.sentry.tests.e2e.hdfs.TestHDFSIntegrationBase; import org.apache.sentry.tests.e2e.hive.StaticUserGroup; import org.apache.sentry.service.common.ServiceConstants.SentryPrincipalType; @@ -640,6 +641,170 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { } /** + * Verify that the owner privilege is updated when the ownership is changed when DB name + * is not explicitly specified + * + * @throws Exception + */ + @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry") + @Test + public void testAlterTableWithoutDB() throws Exception { + dbNames = new String[]{DB1}; + String allWithGrantRole = "allWithGrant_role"; + String ownerRole = "owner_role"; + roles = new String[]{"admin_role", "create_db1", "owner_role"}; + + // create required roles + setupUserRoles(roles, statementAdmin); + + // create test DB + statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE"); + statementAdmin.execute("CREATE DATABASE " + DB1); + + // setup privileges for USER1 + statementAdmin.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1"); + statementAdmin.execute("USE " + DB1); + + // USER1 create table + Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1); + Statement statementUSER1_1 = connectionUSER1_1.createStatement(); + statementUSER1_1.execute("USE " + DB1); + statementUSER1_1.execute("CREATE TABLE " + tableName1 + + " (under_col int comment 'the under column')"); + + // verify privileges created for new table + verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), + DB1, tableName1, 1); + + Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1); + Statement statementUSER2_1 = connectionUSER2_1.createStatement(); + + try { + // create role that has all with grant on the table + statementAdmin.execute("create role " + allWithGrantRole); + statementAdmin.execute("grant role " + allWithGrantRole + " to group " + USERGROUP2); + statementAdmin.execute("grant all on table " + DB1 + "." + tableName1 + " to role " + + allWithGrantRole + " with grant option"); + statementUSER2_1.execute("USE " + DB1); + + // user2_1 having all with grant on this table and can issue command: alter table set owner + // alter table set owner to a role + statementUSER2_1 + .execute("ALTER TABLE " + tableName1 + " SET OWNER ROLE " + ownerRole); + + // verify privileges is transferred to role owner_role, which is associated with USERGROUP1, + // therefore to USER1_1 + verifyTableOwnerPrivilegeExistForPrincipal(statementAdmin, SentryPrincipalType.ROLE, + Lists.newArrayList(ownerRole), + DB1, tableName1, 1); + + // alter table set owner to user USER1_1 and verify privileges is transferred to USER USER1_1 + statementUSER2_1 + .execute("ALTER TABLE " + tableName1 + " SET OWNER USER " + USER1_1); + verifyTableOwnerPrivilegeExistForPrincipal(statementAdmin, SentryPrincipalType.USER, + Lists.newArrayList(USER1_1), DB1, tableName1, 1); + } finally { + statementAdmin.execute("drop role " + allWithGrantRole); + + statementAdmin.close(); + connection.close(); + + statementUSER1_1.close(); + connectionUSER1_1.close(); + + statementUSER2_1.close(); + connectionUSER2_1.close(); + } + } + + + /** + * Verify that the owner privilege is not updated for user who does not have all with grant option + * when DB name is not explicitly specified + * + * @throws Exception + */ + @Ignore("Enable the test once HIVE-18762 is in the hiver version integrated with Sentry") + @Test + public void testAlterTableNegativeWithoutDB() throws Exception { + dbNames = new String[]{DB1}; + String allWithOutGrantRole = "allWithOutGrant_role"; + String ownerRole = "owner_role"; + roles = new String[]{"admin_role", "create_db1", "owner_role"}; + + // create required roles + setupUserRoles(roles, statementAdmin); + + // create test DB + statementAdmin.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE"); + statementAdmin.execute("CREATE DATABASE " + DB1); + + // setup privileges for USER1 + statementAdmin.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE create_db1"); + statementAdmin.execute("USE " + DB1); + + // USER1 create table + Connection connectionUSER1_1 = hiveServer2.createConnection(USER1_1, USER1_1); + Statement statementUSER1_1 = connectionUSER1_1.createStatement(); + statementUSER1_1.execute("USE " + DB1); + statementUSER1_1.execute("CREATE TABLE " + tableName1 + + " (under_col int comment 'the under column')"); + + // verify privileges created for new table + verifyTableOwnerPrivilegeExistForPrincipal(statementUSER1_1, SentryPrincipalType.USER, Lists.newArrayList(USER1_1), + DB1, tableName1, 1); + + Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1); + Statement statementUSER2_1 = connectionUSER2_1.createStatement(); + + try { + // create role that has all with grant on the table + statementAdmin.execute("create role " + allWithOutGrantRole); + statementAdmin.execute("grant role " + allWithOutGrantRole + " to group " + USERGROUP2); + statementAdmin.execute("grant all on table " + DB1 + "." + tableName1 + " to role " + + allWithOutGrantRole); + statementUSER2_1.execute("USE " + DB1); + + // user2_1 having all without grant on this table and can not issue command: + // alter table set owner to a role + try { + statementUSER2_1 + .execute("ALTER TABLE " + tableName1 + " SET OWNER ROLE " + ownerRole); + Assert.fail("User without grant permission should not be allowed to change the owner"); + } catch (HiveSQLException ex) { + String exMessage = ex.getMessage(); + Assert.assertTrue( + "Expect required privileges: Server=server1->Db=db_1->Table=tb_1->action=*->grantOption=true; not in Exception message: " + exMessage, + exMessage.contains("The required privileges: Server=server1->Db=db_1->Table=tb_1->action=*->grantOption=true;")); + } + + // user2_1 having all without grant on this table and can not issue command: + // alter table set owner to user USER1_1 + try { + statementUSER2_1 + .execute("ALTER TABLE " + tableName1 + " SET OWNER USER " + USER1_1); + Assert.fail("User without grant permission should not be allowed to change the owner"); + } catch (HiveSQLException ex) { + String exMessage = ex.getMessage(); + Assert.assertTrue( + "Expect required privileges: Server=server1->Db=db_1->Table=tb_1->action=*->grantOption=true; not in Exception message: " + exMessage, + exMessage.contains("The required privileges: Server=server1->Db=db_1->Table=tb_1->action=*->grantOption=true;")); + } + } finally { + statementAdmin.execute("drop role " + allWithOutGrantRole); + + statementAdmin.close(); + connection.close(); + + statementUSER1_1.close(); + connectionUSER1_1.close(); + + statementUSER2_1.close(); + connectionUSER2_1.close(); + } + } + + /** * Verify that the user who can call alter table set owner on this table * * @throws Exception @@ -692,11 +857,15 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { } // admin issues alter table set owner - try { - statementAdmin.execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole); - Assert.fail("Expect altering table set owner to fail for admin"); - } catch (Exception ex) { - // admin does not have grant option, so cannot issue this command + if (!ownerPrivilegeGrantEnabled) { + try { + statementAdmin + .execute("ALTER TABLE " + DB1 + "." + tableName1 + " SET OWNER ROLE " + ownerRole); + Assert.fail("Expect altering table set owner to fail for admin"); + } catch (Exception ex) { + // admin is owner of the db. It does not have grant option if owner does not have + // grant option, so cannot issue this command + } } Connection connectionUSER2_1 = hiveServer2.createConnection(USER2_1, USER2_1); @@ -800,7 +969,7 @@ public class TestOwnerPrivileges extends TestHDFSIntegrationBase { } // Create test roles - protected void setupUserRoles(String[] roles, Statement statement) throws Exception { + protected void setupUserRoles(String[] roles, Statement statementAdmin) throws Exception { Set<String> userRoles = Sets.newHashSet(roles); userRoles.remove("admin_role");
