Repository: incubator-sentry Updated Branches: refs/heads/master a2b6184d2 -> 297e6dadc
SENTRY-549: SentryStore should support more actions for drop/rename (Xiaomeng Huang via Lenni Kuff) Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/297e6dad Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/297e6dad Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/297e6dad Branch: refs/heads/master Commit: 297e6dadc7ad4180a6439b6950ec285f1174183e Parents: a2b6184 Author: Lenni Kuff <[email protected]> Authored: Sun Dec 7 23:53:33 2014 -0800 Committer: Lenni Kuff <[email protected]> Committed: Sun Dec 7 23:53:33 2014 -0800 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 54 ++++++++++---- .../db/service/persistent/TestSentryStore.java | 76 +++++++++++++++++++- .../TestDbPrivilegeCleanupOnDrop.java | 66 ++++++++++++++++- 3 files changed, 176 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/297e6dad/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 83ac3be..f98e853 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -95,6 +95,17 @@ public class SentryStore { public static String NULL_COL = "__NULL__"; static final String DEFAULT_DATA_DIR = "sentry_policy_db"; + private static final Set<String> ALL_ACTIONS = Sets.newHashSet(AccessConstants.ALL, + AccessConstants.SELECT, AccessConstants.INSERT, AccessConstants.ALTER, + AccessConstants.CREATE, AccessConstants.DROP, AccessConstants.INDEX, + AccessConstants.LOCK); + + // Now partial revoke just support action with SELECT,INSERT and ALL. + // e.g. If we REVOKE SELECT from a privilege with action ALL, it will leads to INSERT + // Otherwise, if we revoke other privilege(e.g. ALTER,DROP...), we will remove it from a role directly. + private static final Set<String> PARTIAL_REVOKE_ACTIONS = Sets.newHashSet(AccessConstants.ALL, + AccessConstants.ACTION_ALL.toLowerCase(), AccessConstants.SELECT, AccessConstants.INSERT); + /** * Commit order sequence id. This is used by notification handlers * to know the order in which events where committed to the database. @@ -506,7 +517,7 @@ public class SentryStore { // Get the privilege graph populateChildren(pm, Sets.newHashSet(roleName), mPrivilege, privilegeGraph); for (MSentryPrivilege childPriv : privilegeGraph) { - revokePartial(pm, tPrivilege, mRole, childPriv); + revokePrivilegeFromRole(pm, tPrivilege, mRole, childPriv); } pm.makePersistent(mRole); } @@ -562,9 +573,24 @@ public class SentryStore { persistedPriv.appendRole(mRole); pm.makePersistent(persistedPriv); } - } + /** + * Revoke privilege from role + */ + private void revokePrivilegeFromRole(PersistenceManager pm, TSentryPrivilege tPrivilege, + MSentryRole mRole, MSentryPrivilege mPrivilege) throws SentryInvalidInputException { + if (PARTIAL_REVOKE_ACTIONS.contains(mPrivilege.getAction())) { + // if this privilege is in {ALL,SELECT,INSERT} + // we will do partial revoke + revokePartial(pm, tPrivilege, mRole, mPrivilege); + } else { + // if this privilege is not ALL, SELECT nor INSERT, + // we will revoke it from role directly + mPrivilege.removeRole(mRole); + pm.makePersistent(mPrivilege); + } + } /** * Explore Privilege graph and collect child privileges. @@ -680,12 +706,12 @@ public class SentryStore { private MSentryPrivilege getMSentryPrivilege(TSentryPrivilege tPriv, PersistenceManager pm) { Query query = pm.newQuery(MSentryPrivilege.class); query.setFilter("this.serverName == \"" + toNULLCol(safeTrimLower(tPriv.getServerName())) + "\" " - + "&& this.dbName == \"" + toNULLCol(safeTrimLower(tPriv.getDbName())) + "\" " - + "&& this.tableName == \"" + toNULLCol(safeTrimLower(tPriv.getTableName())) + "\" " - + "&& this.columnName == \"" + toNULLCol(safeTrimLower(tPriv.getColumnName())) + "\" " - + "&& this.URI == \"" + toNULLCol(safeTrim(tPriv.getURI())) + "\" " - + "&& this.grantOption == grantOption " - + "&& this.action == \"" + toNULLCol(safeTrimLower(tPriv.getAction())) + "\""); + + "&& this.dbName == \"" + toNULLCol(safeTrimLower(tPriv.getDbName())) + "\" " + + "&& this.tableName == \"" + toNULLCol(safeTrimLower(tPriv.getTableName())) + "\" " + + "&& this.columnName == \"" + toNULLCol(safeTrimLower(tPriv.getColumnName())) + "\" " + + "&& this.URI == \"" + toNULLCol(safeTrim(tPriv.getURI())) + "\" " + + "&& this.grantOption == grantOption " + + "&& this.action == \"" + toNULLCol(safeTrimLower(tPriv.getAction())) + "\""); query.declareParameters("Boolean grantOption"); query.setUnique(true); Boolean grantOption = null; @@ -1423,8 +1449,7 @@ public class SentryStore { pm = openTransaction(); if (isMultiActionsSupported(tPrivilege)) { - for (String privilegeAction : Sets.newHashSet(AccessConstants.ALL, - AccessConstants.SELECT, AccessConstants.INSERT)) { + for (String privilegeAction : ALL_ACTIONS) { tPrivilege.setAction(privilegeAction); dropPrivilegeForAllRoles(pm, new TSentryPrivilege(tPrivilege)); } @@ -1463,8 +1488,7 @@ public class SentryStore { pm = openTransaction(); // In case of tables or DBs, check all actions if (isMultiActionsSupported(tPrivilege)) { - for (String privilegeAction : Sets.newHashSet(AccessConstants.ALL, - AccessConstants.SELECT, AccessConstants.INSERT)) { + for (String privilegeAction : ALL_ACTIONS) { tPrivilege.setAction(privilegeAction); newPrivilege.setAction(privilegeAction); renamePrivilegeForAllRoles(pm, tPrivilege, newPrivilege); @@ -1583,15 +1607,15 @@ public class SentryStore { } public static String toNULLCol(String s) { - return Strings.isNullOrEmpty(s) ? NULL_COL : s; + return Strings.isNullOrEmpty(s) ? NULL_COL : s; } public static String fromNULLCol(String s) { - return isNULL(s) ? "" : s; + return isNULL(s) ? "" : s; } public static boolean isNULL(String s) { - return Strings.isNullOrEmpty(s) || s.equals(NULL_COL); + return Strings.isNullOrEmpty(s) || s.equals(NULL_COL); } /** http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/297e6dad/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java index 8ca1f83..8fbe3f4 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java @@ -1261,10 +1261,12 @@ public class TestSentryStore { } /** - * Regression test for SENTRY-547 + * Regression test for SENTRY-547 and SENTRY-548 * Use case: * GRANT INSERT on TABLE tbl1 to ROLE role1 * GRANT SELECT on TABLE tbl1 to ROLE role1 + * GRANT ALTER on TABLE tbl1 to ROLE role1 + * GRANT DROP on TABLE tbl1 to ROLE role1 * DROP TABLE tbl1 * * After drop tbl1, role1 should have 0 privileges @@ -1290,10 +1292,20 @@ public class TestSentryStore { privilege_tbl1); privilege_tbl1_select.setAction(AccessConstants.SELECT); + TSentryPrivilege privilege_tbl1_alter = new TSentryPrivilege( + privilege_tbl1); + privilege_tbl1_alter.setAction(AccessConstants.ALTER); + + TSentryPrivilege privilege_tbl1_drop = new TSentryPrivilege( + privilege_tbl1); + privilege_tbl1_drop.setAction(AccessConstants.DROP); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_insert); sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_select); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_alter); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_drop); - assertEquals(2, sentryStore.getAllTSentryPrivilegesByRoleName(roleName1).size()); + assertEquals(4, sentryStore.getAllTSentryPrivilegesByRoleName(roleName1).size()); // after drop privilege_tbl1, role1 should have 0 privileges sentryStore.dropPrivilege(toTSentryAuthorizable(privilege_tbl1)); @@ -1440,6 +1452,66 @@ public class TestSentryStore { } } + /** + * Regression test for SENTRY-550 + * Use case: + * GRANT INSERT on TABLE tbl1 to ROLE role1 + * GRANT SELECT on TABLE tbl1 to ROLE role1 + * GRANT ALTER on TABLE tbl1 to ROLE role1 + * GRANT DROP on TABLE tbl1 to ROLE role1 + * RENAME TABLE tbl1 to tbl2 + * + * After rename tbl1 to tbl2, table name of all role1's privileges should be "tbl2" + */ + @Test + public void testRenameTableWithMultiAction() throws Exception { + String roleName1 = "role1"; + String grantor = "g1"; + String table1 = "tbl1", table2 = "tbl2"; + sentryStore.createSentryRole(roleName1); + + TSentryPrivilege privilege_tbl1 = new TSentryPrivilege(); + privilege_tbl1.setPrivilegeScope("TABLE"); + privilege_tbl1.setServerName("server1"); + privilege_tbl1.setDbName("db1"); + privilege_tbl1.setTableName(table1); + privilege_tbl1.setCreateTime(System.currentTimeMillis()); + + TSentryPrivilege privilege_tbl1_insert = new TSentryPrivilege( + privilege_tbl1); + privilege_tbl1_insert.setAction(AccessConstants.INSERT); + + TSentryPrivilege privilege_tbl1_select = new TSentryPrivilege( + privilege_tbl1); + privilege_tbl1_select.setAction(AccessConstants.SELECT); + + TSentryPrivilege privilege_tbl1_alter = new TSentryPrivilege( + privilege_tbl1); + privilege_tbl1_alter.setAction(AccessConstants.ALTER); + + TSentryPrivilege privilege_tbl1_drop = new TSentryPrivilege( + privilege_tbl1); + privilege_tbl1_drop.setAction(AccessConstants.DROP); + + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_insert); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_select); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_alter); + sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName1, privilege_tbl1_drop); + + TSentryAuthorizable oldTable = toTSentryAuthorizable(privilege_tbl1); + TSentryAuthorizable newTable = toTSentryAuthorizable(privilege_tbl1); + newTable.setTable(table2); + sentryStore.renamePrivilege(oldTable, newTable); + + // after rename tbl1 to tbl2, all table name of role's privilege will be tbl2 + Set<TSentryPrivilege> privilegeSet = sentryStore + .getAllTSentryPrivilegesByRoleName(roleName1); + assertEquals(4, privilegeSet.size()); + for (TSentryPrivilege privilege : privilegeSet) { + assertTrue(table2.equalsIgnoreCase(privilege.getTableName())); + } + } + @Test public void testSentryRoleSize() throws Exception { for( long i = 0; i< 5; i++ ) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/297e6dad/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java index 0959d2e..a35cf21 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java @@ -17,13 +17,16 @@ */ package org.apache.sentry.tests.e2e.dbprovider; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import java.io.File; import java.io.FileOutputStream; import java.sql.Connection; import java.sql.ResultSet; +import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; import java.util.List; @@ -79,7 +82,7 @@ public class TestDbPrivilegeCleanupOnDrop extends * drop table and verify that the no privileges are referring to it drop db * and verify that the no privileges are referring to it drop db cascade * verify that the no privileges are referring to db and tables under it - * + * * @throws Exception */ @Test @@ -98,10 +101,23 @@ public class TestDbPrivilegeCleanupOnDrop extends } /** + * Return the remaining rows of the current resultSet + * Cautiously it will modify the cursor position of the resultSet + * + */ + private void assertRemainingRows(ResultSet resultSet, int expected) throws SQLException{ + int count = 0; + while(resultSet.next()) { + count++; + } + assertThat(count, is(expected)); + } + + /** * drop table and verify that the no privileges are referring to it drop db * and verify that the no privileges are referring to it drop db cascade * verify that the no privileges are referring to db and tables under it - * + * * @throws Exception */ @Test @@ -120,7 +136,7 @@ public class TestDbPrivilegeCleanupOnDrop extends /** * rename table and verify that the no privileges are referring to it old table * verify that the same privileges are created for the new table name - * + * * @throws Exception */ @Test @@ -157,6 +173,50 @@ public class TestDbPrivilegeCleanupOnDrop extends connection.close(); } + /** + * After we drop/rename table, we will drop/rename all privileges(ALL,SELECT,INSERT,ALTER,DROP...) + * from this role + * + * @throws Exception + */ + @Test + public void testDropAndRenameWithMultiAction() throws Exception { + super.setupAdmin(); + + Connection connection = context.createConnection(ADMIN1); + Statement statement = context.createStatement(connection); + statement.execute("CREATE ROLE user_role"); + statement.execute("GRANT ROLE user_role TO GROUP " + USERGROUP1); + + statement.execute("CREATE DATABASE " + DB1); + statement.execute("USE " + DB1); + statement.execute("CREATE TABLE t1 (c1 string)"); + + // Grant SELECT/INSERT/DROP/ALTER to TABLE t1 + statement.execute("GRANT SELECT ON TABLE t1 TO ROLE user_role"); + statement.execute("GRANT INSERT ON TABLE t1 TO ROLE user_role"); + statement.execute("GRANT ALTER ON TABLE t1 TO ROLE user_role"); + statement.execute("GRANT DROP ON TABLE t1 TO ROLE user_role"); + // For rename, grant CREATE to DB1 + statement.execute("GRANT CREATE ON DATABASE " + DB1 + " TO ROLE user_role"); + + // After rename table t1 to t2, user_role will have no permission to drop t1 + connection = context.createConnection(USER1_1); + statement = context.createStatement(connection); + statement.execute("USE " + DB1); + statement.execute("ALTER TABLE t1 RENAME TO t2"); + context.assertSentrySemanticException(statement, "drop table t1", semanticException); + + // After rename table t1 to t2, user_role should have permission to drop t2 + statement.execute("drop table t2"); + ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE user_role"); + // user_role will revoke all privilege from table t2, only remain CREATE on db_1 + assertRemainingRows(resultSet, 1); + + statement.close(); + connection.close(); + } + // Create test roles private void setupRoles(Statement statement) throws Exception { statement.execute("CREATE ROLE all_db1");
