sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sp...@apache.org
Subject sentry git commit: SENTRY-2315: The grant all operation is not dropping the create/alter/drop/index/lock privileges (Sergio Pena, reviewed by Na Li)
Date Wed, 05 Sep 2018 21:07:13 GMT
Repository: sentry
Updated Branches:
  refs/heads/master d9cc5c9fd -> 2492be15d


SENTRY-2315: The grant all operation is not dropping the create/alter/drop/index/lock privileges
(Sergio Pena, reviewed by Na Li)


Project: http://git-wip-us.apache.org/repos/asf/sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/2492be15
Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/2492be15
Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/2492be15

Branch: refs/heads/master
Commit: 2492be15d2140658e7aca39c78d4719659992553
Parents: d9cc5c9
Author: Sergio Pena <sergio.pena@cloudera.com>
Authored: Wed Sep 5 16:06:52 2018 -0500
Committer: Sergio Pena <sergio.pena@cloudera.com>
Committed: Wed Sep 5 16:06:52 2018 -0500

----------------------------------------------------------------------
 .../db/service/persistent/SentryStore.java      | 51 ++++++++----
 .../db/service/persistent/TestSentryStore.java  | 83 ++++++++++++++++++++
 2 files changed, 118 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/2492be15/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
index db8c08b..1eda41b 100644
--- a/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
+++ b/sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
@@ -813,25 +813,11 @@ public class SentryStore implements SentryStoreInterface {
     if ((!isNULL(privilege.getColumnName()) || !isNULL(privilege.getTableName())
         || !isNULL(privilege.getDbName()))
         && !AccessConstants.OWNER.equalsIgnoreCase(privilege.getAction())) {
-      // If Grant is for ALL and Either INSERT/SELECT already exists..
+      // If Grant is for ALL and individual privileges already exists (i.e. insert/select/create..)
       // need to remove it and GRANT ALL..
       if (AccessConstants.ALL.equalsIgnoreCase(privilege.getAction())
           || AccessConstants.ACTION_ALL.equalsIgnoreCase(privilege.getAction())) {
-        TSentryPrivilege tNotAll = new TSentryPrivilege(privilege);
-        tNotAll.setAction(AccessConstants.SELECT);
-        MSentryPrivilege mSelect =
-            findMatchPrivilege(mEntity.getPrivileges(), convertToMSentryPrivilege(tNotAll));
-        tNotAll.setAction(AccessConstants.INSERT);
-        MSentryPrivilege mInsert =
-            findMatchPrivilege(mEntity.getPrivileges(), convertToMSentryPrivilege(tNotAll));
-        if (mSelect != null) {
-          mSelect.removePrincipal(mEntity);
-          persistPrivilege(pm, mSelect);
-        }
-        if (mInsert != null) {
-          mInsert.removePrincipal(mEntity);
-          persistPrivilege(pm, mInsert);
-        }
+        dropPrivilegesForGrantAll(pm, mEntity, privilege);
       } else {
         // If Grant is for Either INSERT/SELECT and ALL already exists..
         // do nothing..
@@ -861,6 +847,39 @@ public class SentryStore implements SentryStoreInterface {
   }
 
   /**
+   * Drop all individual privileges from the privilege entity that form the grant all operation.
+   *
+   * @param pm The PersistenceManager to persist the changes.
+   * @param principal The Sentry principal from where to drop the privileges.
+   * @param privilege The Sentry privilege that has the authorizable object from where to
drop the privileges.
+   * @throws SentryInvalidInputException If an error occurs when dropping the privileges.
+   */
+  private void dropPrivilegesForGrantAll(PersistenceManager pm, PrivilegePrincipal principal,
+    TSentryPrivilege privilege) throws SentryInvalidInputException {
+
+    // Re-use this object to search for the specific privilege
+    TSentryPrivilege tNotAll = new TSentryPrivilege(privilege);
+
+    for (String action : ALL_ACTIONS) {
+      // These privileges do not form part of the grant all operation.
+      // For instance, a role/user may have the OWNER and ALL privileges together.
+      if (action.equalsIgnoreCase(AccessConstants.OWNER)) {
+        continue;
+      }
+
+      // Set the action to search in the set of privileges of the entity
+      tNotAll.setAction(action);
+
+      MSentryPrivilege mAction =
+        findMatchPrivilege(principal.getPrivileges(), convertToMSentryPrivilege(tNotAll));
+      if (mAction != null) {
+        mAction.removePrincipal(principal);
+        persistPrivilege(pm, mAction);
+      }
+    }
+  }
+
+  /**
    * Alter a given sentry user to grant a set of privileges.
    * Internally calls alterSentryGrantPrivilege.
    *

http://git-wip-us.apache.org/repos/asf/sentry/blob/2492be15/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
----------------------------------------------------------------------
diff --git a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
index 6b732ea..d8c0ab4 100644
--- a/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
+++ b/sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
@@ -44,6 +44,7 @@ import org.apache.hadoop.security.alias.CredentialProvider;
 import org.apache.hadoop.security.alias.CredentialProviderFactory;
 import org.apache.hadoop.security.alias.UserProvider;
 import org.apache.sentry.SentryOwnerInfo;
+import org.apache.sentry.api.common.ApiConstants.PrivilegeScope;
 import org.apache.sentry.api.service.thrift.TSentryPrivilegeMap;
 import org.apache.sentry.core.common.exception.SentryAccessDeniedException;
 import org.apache.sentry.core.common.exception.SentryInvalidInputException;
@@ -922,6 +923,81 @@ public class TestSentryStore extends org.junit.Assert {
 
   }
 
+  @Test
+  public void testDropIndividualPrivilegesWhenGrantAllIsGranted() throws Exception {
+    final String GRANTOR = "g1";
+    final String ROLE_NAME = "r1";
+    final String SERVER_NAME = "server1";
+    final String DB_NAME = "db1";
+    final String TABLE_NAME = "table1";
+
+    TSentryPrivilege allPrivilege = toTSentryPrivilege(AccessConstants.ACTION_ALL,
+      PrivilegeScope.SERVER.toString(), SERVER_NAME, DB_NAME, TABLE_NAME, TSentryGrantOption.FALSE);
+
+    TSentryPrivilege allWithGrant = toTSentryPrivilege(AccessConstants.ACTION_ALL,
+      PrivilegeScope.SERVER.toString(), SERVER_NAME, DB_NAME, TABLE_NAME, TSentryGrantOption.TRUE);
+
+    TSentryPrivilege ownerPrivilege = toTSentryPrivilege(AccessConstants.OWNER,
+      PrivilegeScope.DATABASE.toString(), SERVER_NAME, DB_NAME, TABLE_NAME);
+
+    Set<TSentryPrivilege> grantPrivileges = Sets.newHashSet(
+      toTSentryPrivilege(AccessConstants.SELECT, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+      toTSentryPrivilege(AccessConstants.INSERT, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+      toTSentryPrivilege(AccessConstants.CREATE, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+      toTSentryPrivilege(AccessConstants.ALTER, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+      toTSentryPrivilege(AccessConstants.DROP, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+      toTSentryPrivilege(AccessConstants.INDEX, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+      toTSentryPrivilege(AccessConstants.LOCK, PrivilegeScope.SERVER.toString(), SERVER_NAME,
+        DB_NAME, TABLE_NAME),
+
+      // This special privilege will not be removed when granting all privileges
+      ownerPrivilege
+    );
+
+    // Grant individual privileges to a role
+    createRole(ROLE_NAME);
+    sentryStore.alterSentryRoleGrantPrivileges(GRANTOR, ROLE_NAME, grantPrivileges);
+
+    // Check those individual privileges are granted
+    Set<TSentryPrivilege> rolePrivileges = sentryStore.getAllTSentryPrivilegesByRoleName(ROLE_NAME);
+    assertEquals(grantPrivileges.size(), rolePrivileges.size());
+    for (TSentryPrivilege privilege : grantPrivileges) {
+      assertTrue(String.format("Privilege %s was not granted.", privilege.getAction()),
+        rolePrivileges.contains(privilege));
+    }
+
+    // Grant the ALL privilege (this should remove all individual privileges, and grant only
ALL)
+    sentryStore.alterSentryRoleGrantPrivileges(GRANTOR, ROLE_NAME, Sets.newHashSet(allPrivilege));
+
+    // Check the ALL and OWNER privileges are the only privileges
+    rolePrivileges = sentryStore.getAllTSentryPrivilegesByRoleName(ROLE_NAME);
+    assertEquals(2, rolePrivileges.size()); // ALL and OWNER privileges should be there
+    assertTrue("Privilege ALL was not granted.", rolePrivileges.contains(allPrivilege));
+    assertTrue("Privilege OWNER was dropped.", rolePrivileges.contains(ownerPrivilege));
+
+    // Check the ALL WITH GRANT privilege just replaces the ALL and keeps the OWNER privilege
+    sentryStore.alterSentryRoleGrantPrivileges(GRANTOR, ROLE_NAME, Sets.newHashSet(allWithGrant));
+    rolePrivileges = sentryStore.getAllTSentryPrivilegesByRoleName(ROLE_NAME);
+    assertTrue("Privilege ALL WITH GRANT was not granted.", rolePrivileges.contains(allWithGrant));
+    assertTrue("Privilege OWNER was dropped.", rolePrivileges.contains(ownerPrivilege));
+
+    // Check the ALL privilege just replaces the ALL WITH GRANT and keeps the OWNER privilege
+    sentryStore.alterSentryRoleGrantPrivileges(GRANTOR, ROLE_NAME, Sets.newHashSet(allPrivilege));
+    rolePrivileges = sentryStore.getAllTSentryPrivilegesByRoleName(ROLE_NAME);
+    assertTrue("Privilege ALL was not granted.", rolePrivileges.contains(allPrivilege));
+    assertTrue("Privilege OWNER was dropped.", rolePrivileges.contains(ownerPrivilege));
+
+
+    // Clean-up test
+    sentryStore.dropSentryRole(ROLE_NAME);
+  }
+
   //TODO Use new transaction Manager logic, Instead of
 
   @Test
@@ -4464,4 +4540,11 @@ public class TestSentryStore extends org.junit.Assert {
 
     return privilege;
   }
+
+  private TSentryPrivilege toTSentryPrivilege(String action, String scope, String server,
+    String dbName, String tableName, TSentryGrantOption grantOption) {
+    TSentryPrivilege privilege = toTSentryPrivilege(action, scope, server, dbName, tableName);
+    privilege.setGrantOption(grantOption);
+    return privilege;
+  }
 }


Mime
View raw message