sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lsk...@apache.org
Subject incubator-sentry git commit: SENTRY-552: Sentry Store recursive revoke of privilege levels < ALL does not properly downgrade child privileges (Dapeng Sun via Lenni Kuff)
Date Tue, 02 Dec 2014 08:32:52 GMT
Repository: incubator-sentry
Updated Branches:
  refs/heads/master 437f6d96c -> d88c157d8


SENTRY-552: Sentry Store recursive revoke of privilege levels < ALL does not properly downgrade
child privileges (Dapeng Sun 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/d88c157d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/d88c157d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/d88c157d

Branch: refs/heads/master
Commit: d88c157d83c10fdf66b76ead7b7c16fe60d82e26
Parents: 437f6d9
Author: Lenni Kuff <lskuff@cloudera.com>
Authored: Tue Dec 2 00:30:22 2014 -0800
Committer: Lenni Kuff <lskuff@cloudera.com>
Committed: Tue Dec 2 00:32:26 2014 -0800

----------------------------------------------------------------------
 .../db/service/model/MSentryPrivilege.java      |   5 +
 .../db/service/persistent/SentryStore.java      |  18 ++
 .../db/service/persistent/TestSentryStore.java  | 203 ++++++++++++++++++-
 .../e2e/dbprovider/TestDatabaseProvider.java    |  37 +++-
 4 files changed, 251 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/d88c157d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
index ed081a3..1c68a0f 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java
@@ -307,4 +307,9 @@ public class MSentryPrivilege {
     return SentryStore.isNULL(s);
   }
 
+  public boolean isActionALL() {
+    return AccessConstants.ACTION_ALL.equalsIgnoreCase(action)
+        || AccessConstants.ALL.equals(action);
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/d88c157d/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 073bb33..7d2cb12 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
@@ -581,6 +581,24 @@ public class SentryStore {
             && (!isNULL(childPriv.getColumnName()))) {
           populateChildren(roleNames, childPriv, children);
         }
+        // The method getChildPrivileges() didn't do filter on "action",
+        // if the action is not "All", it should judge the action of children privilege.
+        // For example: a user has a privilege “All on Col1”,
+        // if the operation is “REVOKE INSERT on table”
+        // the privilege should be the child of table level privilege.
+        // but the privilege may still have other meaning, likes "SELECT on Col1".
+        // and the privileges like "SELECT on Col1" should not be revoke.
+        if (!priv.isActionALL()) {
+          if (childPriv.isActionALL()) {
+            // If the child privilege is All, we should convert it to the same
+            // privilege with parent
+            childPriv.setAction(priv.getAction());
+          }
+          // Only include privilege that imply the parent privilege.
+          if (!priv.implies(childPriv)) {
+            continue;
+          }
+        }
         children.add(childPriv);
       }
     }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/d88c157d/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 5bb00f5..9dca5fb 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
@@ -404,9 +404,12 @@ public class TestSentryStore {
     assertEquals(privileges.toString(), 0, privileges.size());
   }
 
+  /**
+   * Regression test for SENTRY-74 and SENTRY-552
+   */
   @Test
   public void testGrantRevokePrivilegeWithColumn() throws Exception {
-    String roleName = "test-privilege";
+    String roleName = "test-col-privilege";
     String grantor = "g1";
     String server = "server1";
     String db = "db1";
@@ -422,6 +425,8 @@ public class TestSentryStore {
     privilege.setColumnName(column1);
     privilege.setAction(AccessConstants.ALL);
     privilege.setCreateTime(System.currentTimeMillis());
+
+    // Grant ALL on c1 and c2
     assertEquals(seqId + 1, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName,
privilege)
         .getSequenceId());
     privilege.setColumnName(column2);
@@ -430,22 +435,32 @@ public class TestSentryStore {
     MSentryRole role = sentryStore.getMSentryRoleByName(roleName);
     Set<MSentryPrivilege> privileges = role.getPrivileges();
     assertEquals(privileges.toString(), 2, privileges.size());
+
+    // Revoke SELECT on c2
     privilege.setAction(AccessConstants.SELECT);
     assertEquals(seqId + 3, sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName,
privilege)
         .getSequenceId());
 
-    // after having ALL and revoking SELECT, we should have INSERT
+    // At this point c1 has ALL privileges and c2 should have INSERT after revoking SELECT
     role = sentryStore.getMSentryRoleByName(roleName);
     privileges = role.getPrivileges();
     assertEquals(privileges.toString(), 2, privileges.size());
-    MSentryPrivilege mPrivilege = Iterables.get(privileges, 0);
-    assertEquals(server, mPrivilege.getServerName());
-    assertEquals(db, mPrivilege.getDbName());
-    assertEquals(table, mPrivilege.getTableName());
-    assertEquals(AccessConstants.INSERT, mPrivilege.getAction());
-    assertFalse(mPrivilege.getGrantOption());
+    for (MSentryPrivilege mPrivilege: privileges) {
+      assertEquals(server, mPrivilege.getServerName());
+      assertEquals(db, mPrivilege.getDbName());
+      assertEquals(table, mPrivilege.getTableName());
+      assertFalse(mPrivilege.getGrantOption());
+      if (mPrivilege.getColumnName().equals(column1)) {
+        assertEquals(AccessConstants.ALL, mPrivilege.getAction());
+      } else if (mPrivilege.getColumnName().equals(column2)) {
+        assertEquals(AccessConstants.INSERT, mPrivilege.getAction());
+      } else {
+        fail("Unexpected column name: " + mPrivilege.getColumnName());
+      }
+    }
 
-    // after revoking table level privilege, we will remove the two column level items
+    // after revoking INSERT table level privilege will remove privileges from column2
+    // and downgrade column1 to SELECT privileges.
     privilege = new TSentryPrivilege();
     privilege.setPrivilegeScope("TABLE");
     privilege.setServerName(server);
@@ -457,9 +472,179 @@ public class TestSentryStore {
         .getSequenceId());
     role = sentryStore.getMSentryRoleByName(roleName);
     privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 1, privileges.size());
+    assertEquals(column1, Iterables.get(privileges, 0).getColumnName());
+    assertEquals(AccessConstants.SELECT, Iterables.get(privileges, 0).getAction());
+
+    // Revoke ALL from the table should now remove all the column privileges.
+    privilege.setAction(AccessConstants.ALL);
+    privilege.setCreateTime(System.currentTimeMillis());
+    assertEquals(seqId + 5, sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName,
privilege)
+        .getSequenceId());
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
     assertEquals(privileges.toString(), 0, privileges.size());
   }
 
+  /**
+   * Regression test for SENTRY-552
+   */
+  @Test
+  public void testGrantRevokeTablePrivilegeDowngradeByDb() throws Exception {
+    String roleName = "test-table-db-downgrade-privilege";
+    String grantor = "g1";
+    String server = "server1";
+    String db = "db1";
+    String table1 = "tbl1";
+    String table2 = "tbl2";
+    long seqId = sentryStore.createSentryRole(roleName).getSequenceId();
+    TSentryPrivilege privilegeTable1 = new TSentryPrivilege();
+    privilegeTable1.setPrivilegeScope("TABLE");
+    privilegeTable1.setServerName(server);
+    privilegeTable1.setDbName(db);
+    privilegeTable1.setTableName(table1);
+    privilegeTable1.setAction(AccessConstants.ALL);
+    privilegeTable1.setCreateTime(System.currentTimeMillis());
+    TSentryPrivilege privilegeTable2 = privilegeTable1.deepCopy();;
+    privilegeTable2.setTableName(table2);
+
+    // Grant ALL on table1 and table2
+    assertEquals(seqId + 1, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName,
privilegeTable1)
+        .getSequenceId());
+    assertEquals(seqId + 2, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName,
privilegeTable2)
+        .getSequenceId());
+    MSentryRole role = sentryStore.getMSentryRoleByName(roleName);
+    Set<MSentryPrivilege> privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 2, privileges.size());
+
+    // Revoke SELECT on table2
+    privilegeTable2.setAction(AccessConstants.SELECT);
+    assertEquals(seqId + 3, sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName,
privilegeTable2)
+        .getSequenceId());
+    // after having ALL and revoking SELECT, we should have INSERT
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 2, privileges.size());
+
+    // At this point table1 has ALL privileges and table2 should have INSERT after revoking
SELECT
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 2, privileges.size());
+    for (MSentryPrivilege mPrivilege: privileges) {
+      assertEquals(server, mPrivilege.getServerName());
+      assertEquals(db, mPrivilege.getDbName());
+      assertFalse(mPrivilege.getGrantOption());
+      if (mPrivilege.getTableName().equals(table1)) {
+        assertEquals(AccessConstants.ALL, mPrivilege.getAction());
+      } else if (mPrivilege.getTableName().equals(table2)) {
+        assertEquals(AccessConstants.INSERT, mPrivilege.getAction());
+      } else {
+        fail("Unexpected table name: " + mPrivilege.getTableName());
+      }
+    }
+
+    // Revoke INSERT on Database
+    privilegeTable2.setAction(AccessConstants.INSERT);
+    privilegeTable2.setPrivilegeScope("DATABASE");
+    privilegeTable2.unsetTableName();
+    assertEquals(seqId + 4, sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName,
privilegeTable2)
+        .getSequenceId());
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
+
+    // after revoking INSERT database level privilege will remove privileges from table2
+    // and downgrade table1 to SELECT privileges.
+    assertEquals(privileges.toString(), 1, privileges.size());
+    MSentryPrivilege mPrivilege = Iterables.get(privileges, 0);
+    assertEquals(server, mPrivilege.getServerName());
+    assertEquals(db, mPrivilege.getDbName());
+    assertEquals(table1, mPrivilege.getTableName());
+    assertEquals(AccessConstants.SELECT, mPrivilege.getAction());
+    assertFalse(mPrivilege.getGrantOption());
+  }
+
+  /**
+   * Regression test for SENTRY-552
+   */
+  @Test
+  public void testGrantRevokeColumnPrivilegeDowngradeByDb() throws Exception {
+    String roleName = "test-column-db-downgrade-privilege";
+    String grantor = "g1";
+    String server = "server1";
+    String db = "db1";
+    String table = "tbl1";
+    String column1 = "c1";
+    String column2 = "c2";
+    long seqId = sentryStore.createSentryRole(roleName).getSequenceId();
+    TSentryPrivilege privilegeCol1 = new TSentryPrivilege();
+    privilegeCol1.setPrivilegeScope("COLUMN");
+    privilegeCol1.setServerName(server);
+    privilegeCol1.setDbName(db);
+    privilegeCol1.setTableName(table);
+    privilegeCol1.setColumnName(column1);
+    privilegeCol1.setAction(AccessConstants.ALL);
+    privilegeCol1.setCreateTime(System.currentTimeMillis());
+    TSentryPrivilege privilegeCol2 = privilegeCol1.deepCopy();;
+    privilegeCol2.setColumnName(column2);
+
+    // Grant ALL on column1 and column2
+    assertEquals(seqId + 1, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName,
privilegeCol1)
+        .getSequenceId());
+    assertEquals(seqId + 2, sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName,
privilegeCol2)
+        .getSequenceId());
+    MSentryRole role = sentryStore.getMSentryRoleByName(roleName);
+    Set<MSentryPrivilege> privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 2, privileges.size());
+
+    // Revoke SELECT on column2
+    privilegeCol2.setAction(AccessConstants.SELECT);
+    assertEquals(seqId + 3, sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName,
privilegeCol2)
+        .getSequenceId());
+    // after having ALL and revoking SELECT, we should have INSERT
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 2, privileges.size());
+
+    // At this point column1 has ALL privileges and column2 should have INSERT after revoking
SELECT
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
+    assertEquals(privileges.toString(), 2, privileges.size());
+    for (MSentryPrivilege mPrivilege: privileges) {
+      assertEquals(server, mPrivilege.getServerName());
+      assertEquals(db, mPrivilege.getDbName());
+      assertEquals(table, mPrivilege.getTableName());
+      assertFalse(mPrivilege.getGrantOption());
+      if (mPrivilege.getColumnName().equals(column1)) {
+        assertEquals(AccessConstants.ALL, mPrivilege.getAction());
+      } else if (mPrivilege.getColumnName().equals(column2)) {
+        assertEquals(AccessConstants.INSERT, mPrivilege.getAction());
+      } else {
+        fail("Unexpected column name: " + mPrivilege.getColumnName());
+      }
+    }
+
+    // Revoke INSERT on Database
+    privilegeCol2.setAction(AccessConstants.INSERT);
+    privilegeCol2.setPrivilegeScope("DATABASE");
+    privilegeCol2.unsetTableName();
+    privilegeCol2.unsetColumnName();
+    assertEquals(seqId + 4, sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName,
privilegeCol2)
+        .getSequenceId());
+    role = sentryStore.getMSentryRoleByName(roleName);
+    privileges = role.getPrivileges();
+
+    // after revoking INSERT database level privilege will remove privileges from column2
+    // and downgrade column1 to SELECT privileges.
+    assertEquals(privileges.toString(), 1, privileges.size());
+    MSentryPrivilege mPrivilege = Iterables.get(privileges, 0);
+    assertEquals(server, mPrivilege.getServerName());
+    assertEquals(db, mPrivilege.getDbName());
+    assertEquals(table, mPrivilege.getTableName());
+    assertEquals(column1, mPrivilege.getColumnName());
+    assertEquals(AccessConstants.SELECT, mPrivilege.getAction());
+    assertFalse(mPrivilege.getGrantOption());
+  }
+
   @Test
   public void testGrantRevokePrivilegeWithGrantOption() throws Exception {
     String roleName = "test-grantOption-table";

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/d88c157d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
index 3189955..4a475ba 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
@@ -122,10 +122,11 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration
{
     connection.close();
   }
 
-
-  // SENTRY-303 tests
+  /**
+   * Regression test for SENTRY-303 and SENTRY-543.
+   */
   @Test
-  public void testGrantSELECTonDb() throws Exception {
+  public void testGrantRevokeSELECTonDb() throws Exception {
     File dataFile = doSetupForGrantDbTests();
 
     Connection connection = context.createConnection(ADMIN1);
@@ -157,6 +158,36 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration
{
     } catch (Exception e) {
       // Ignore
     }
+
+    connection = context.createConnection(ADMIN1);
+    statement = context.createStatement(connection);
+
+    // SENTRY-543 - Verify recursive revoke of SELECT on database removes the correct
+    // privileges.
+    statement.execute("USE " + DB1);
+    statement.execute("GRANT ALL ON TABLE t1 TO ROLE user_role");
+    statement.execute("REVOKE SELECT ON DATABASE " + DB1 + " FROM ROLE user_role");
+    statement.close();
+    connection.close();
+
+    // Switch to user1 - they should not have no access on t1 and t2 now.
+    connection = context.createConnection(USER1_1);
+    statement = context.createStatement(connection);
+
+    try {
+      // SELECT is not allowed
+      statement.execute("SELECT * FROM " + DB1 + ".t1");
+      fail("SELECT should have been revoked from t1");
+    } catch (Exception e) {
+      // Ignore
+    }
+    try {
+      // SELECT is not allowed
+      statement.execute("SELECT * FROM " + DB1 + ".t2");
+      fail("SELECT should have been revoked from t2");
+    } catch (Exception e) {
+      // Ignore
+    }
     statement.close();
     connection.close();
   }


Mime
View raw message