sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From linaataus...@apache.org
Subject sentry git commit: SENTRY-2245: Remove privileges that do not associate with a role or a user (Na Li, reviewed by Sergio Pena, Kalyan Kumar Kalvagadda)
Date Wed, 06 Jun 2018 20:19:07 GMT
Repository: sentry
Updated Branches:
  refs/heads/master 360e9c304 -> 315a8913a


SENTRY-2245: Remove privileges that do not associate with a role or a user (Na Li, reviewed
by Sergio Pena, Kalyan Kumar Kalvagadda)


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

Branch: refs/heads/master
Commit: 315a8913aa9dfc5fad7a9863c766267fd6f7877b
Parents: 360e9c3
Author: lina.li <lina.li@cloudera.com>
Authored: Wed Jun 6 15:18:31 2018 -0500
Committer: lina.li <lina.li@cloudera.com>
Committed: Wed Jun 6 15:18:31 2018 -0500

----------------------------------------------------------------------
 .../db/service/persistent/SentryStore.java      | 67 +++++++-------------
 .../db/service/persistent/TestSentryStore.java  | 10 +++
 2 files changed, 34 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/315a8913/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 7bf3e80..e6b71b5 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
@@ -843,11 +843,11 @@ public class SentryStore {
             findMatchPrivilege(mRole.getPrivileges(), convertToMSentryPrivilege(tNotAll));
         if (mSelect != null) {
           mSelect.removeRole(mRole);
-          pm.makePersistent(mSelect);
+          persistPrivilege(pm, mSelect);
         }
         if (mInsert != null) {
           mInsert.removeRole(mRole);
-          pm.makePersistent(mInsert);
+          persistPrivilege(pm, mInsert);
         }
       } else {
         // If Grant is for Either INSERT/SELECT and ALL already exists..
@@ -1072,11 +1072,11 @@ public class SentryStore {
             findMatchPrivilege(mUser.getPrivileges(), convertToMSentryPrivilege(tNotAll));
         if (mSelect != null) {
           mSelect.removeUser(mUser);
-          pm.makePersistent(mSelect);
+          persistPrivilege(pm, mSelect);
         }
         if (mInsert != null) {
           mInsert.removeUser(mUser);
-          pm.makePersistent(mInsert);
+          persistPrivilege(pm, mInsert);
         }
       } else {
         // If Grant is for Either INSERT/SELECT and ALL already exists..
@@ -1421,11 +1421,7 @@ public class SentryStore {
           persistedPriv.removeUser(mUser);
         }
 
-        if (isPrivilegeStall(persistedPriv)) {
-          pm.deletePersistent(persistedPriv);
-        } else {
-          pm.makePersistent(persistedPriv);
-        }
+        persistPrivilege(pm, persistedPriv);
       }
     } else {
 
@@ -1457,7 +1453,16 @@ public class SentryStore {
     return false;
   }
 
-  private boolean isPrivilegeStall(MSentryPrivilege privilege) {
+  private void persistPrivilege(PersistenceManager pm, MSentryPrivilege privilege) {
+    if (isPrivilegeStale(privilege)) {
+        pm.deletePersistent(privilege);
+      return;
+    }
+
+    pm.makePersistent(privilege);
+  }
+
+  private boolean isPrivilegeStale(MSentryPrivilege privilege) {
     if (privilege.getUsers().isEmpty() && privilege.getRoles().isEmpty()) {
       return true;
     }
@@ -1465,7 +1470,7 @@ public class SentryStore {
     return false;
   }
 
-  private boolean isPrivilegeStall(MSentryGMPrivilege privilege) {
+  private boolean isPrivilegeStale(MSentryGMPrivilege privilege) {
     if (privilege.getRoles().isEmpty()) {
       return true;
     }
@@ -1481,21 +1486,13 @@ public class SentryStore {
     persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(persistedPriv), pm);
     if (persistedPriv != null && !persistedPriv.getRoles().isEmpty()) {
       persistedPriv.removeRole(mRole);
-      if (isPrivilegeStall(persistedPriv)) {
-        pm.deletePersistent(persistedPriv);
-      } else {
-        pm.makePersistent(persistedPriv);
-      }
+      persistPrivilege(pm, persistedPriv);
     }
     currentPrivilege.setAction(AccessConstants.ALL);
     persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
     if (persistedPriv != null && mRole.getPrivileges().contains(persistedPriv)) {
       persistedPriv.removeRole(mRole);
-      if (isPrivilegeStall(persistedPriv)) {
-        pm.deletePersistent(persistedPriv);
-      } else {
-        pm.makePersistent(persistedPriv);
-      }
+      persistPrivilege(pm, persistedPriv);
 
       // add decomposted actions
       for (String addAction : addActions) {
@@ -1520,21 +1517,13 @@ public class SentryStore {
     persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(persistedPriv), pm);
     if (persistedPriv != null && !persistedPriv.getUsers().isEmpty()) {
       persistedPriv.removeUser(mUser);
-      if (isPrivilegeStall(persistedPriv)) {
-        pm.deletePersistent(persistedPriv);
-      } else {
-        pm.makePersistent(persistedPriv);
-      }
+      persistPrivilege(pm, persistedPriv);
     }
     currentPrivilege.setAction(AccessConstants.ALL);
     persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(currentPrivilege), pm);
     if (persistedPriv != null && mUser.getPrivileges().contains(persistedPriv)) {
       persistedPriv.removeUser(mUser);
-      if (isPrivilegeStall(persistedPriv)) {
-        pm.deletePersistent(persistedPriv);
-      } else {
-        pm.makePersistent(persistedPriv);
-      }
+      persistPrivilege(pm, persistedPriv);
 
       // add decomposted actions
       for (String addAction : addActions) {
@@ -1567,11 +1556,7 @@ public class SentryStore {
       MSentryPrivilege persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(mPrivilege),
pm);
       if (persistedPriv != null && !persistedPriv.getRoles().isEmpty()) {
         persistedPriv.removeRole(mRole);
-        if (isPrivilegeStall(persistedPriv)) {
-          pm.deletePersistent(persistedPriv);
-        } else {
-          pm.makePersistent(persistedPriv);
-        }
+        persistPrivilege(pm, persistedPriv);
       }
     }
   }
@@ -1592,11 +1577,7 @@ public class SentryStore {
       MSentryPrivilege persistedPriv = getMSentryPrivilege(convertToTSentryPrivilege(mPrivilege),
pm);
       if (persistedPriv != null && !persistedPriv.getUsers().isEmpty()) {
         persistedPriv.removeUser(mUser);
-        if (isPrivilegeStall(persistedPriv)) {
-          pm.deletePersistent(persistedPriv);
-        } else {
-          pm.makePersistent(persistedPriv);
-        }
+        persistPrivilege(pm, persistedPriv);
       }
     }
   }
@@ -1873,7 +1854,7 @@ public class SentryStore {
   private void removeStaledPrivileges(PersistenceManager pm, List<MSentryPrivilege>
privilegesCopy) {
     List<MSentryPrivilege> stalePrivileges = new ArrayList<>(0);
     for (MSentryPrivilege privilege : privilegesCopy) {
-      if (isPrivilegeStall(privilege)) {
+      if (isPrivilegeStale(privilege)) {
         stalePrivileges.add(privilege);
       }
     }
@@ -1885,7 +1866,7 @@ public class SentryStore {
   private void removeStaledGMPrivileges(PersistenceManager pm, List<MSentryGMPrivilege>
privilegesCopy) {
     List<MSentryGMPrivilege> stalePrivileges = new ArrayList<>(0);
     for (MSentryGMPrivilege privilege : privilegesCopy) {
-      if (isPrivilegeStall(privilege)) {
+      if (isPrivilegeStale(privilege)) {
         stalePrivileges.add(privilege);
       }
     }

http://git-wip-us.apache.org/repos/asf/sentry/blob/315a8913/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 95dabec..12c6d91 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
@@ -3978,6 +3978,8 @@ public class TestSentryStore extends org.junit.Assert {
     // second round
     privilege.setAction(AccessConstants.ALL);
     sentryStore.alterSentryRoleGrantPrivilege(grantor, roleName, privilege);
+    List<MSentryPrivilege> totalPrivileges = sentryStore.getAllMSentryPrivileges();
+    assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
     role = sentryStore.getMSentryRoleByName(roleName);
     privileges = role.getPrivileges();
     assertEquals(privileges.toString(), 1, privileges.size());
@@ -3985,6 +3987,8 @@ public class TestSentryStore extends org.junit.Assert {
     privilege.setAction(AccessConstants.INSERT);
     sentryStore.alterSentryRoleRevokePrivilege(grantor, roleName, privilege);
     // after having ALL and revoking INSERT, we should have (SELECT)
+    totalPrivileges = sentryStore.getAllMSentryPrivileges();
+    assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
     role = sentryStore.getMSentryRoleByName(roleName);
     privileges = role.getPrivileges();
     assertEquals(privileges.toString(), 1, privileges.size());
@@ -4023,6 +4027,9 @@ public class TestSentryStore extends org.junit.Assert {
     // second round
     privilege.setAction(AccessConstants.ALL);
     sentryStore.alterSentryUserGrantPrivilege(grantor, userName, privilege);
+    List<MSentryPrivilege> totalPrivileges = sentryStore.getAllMSentryPrivileges();
+    assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
+
     user = sentryStore.getMSentryUserByName(userName);
     privileges = user.getPrivileges();
     assertEquals(privileges.toString(), 1, privileges.size());
@@ -4030,6 +4037,9 @@ public class TestSentryStore extends org.junit.Assert {
     privilege.setAction(AccessConstants.INSERT);
     sentryStore.alterSentryUserRevokePrivilege(grantor, userName, privilege);
     // after having ALL and revoking INSERT, we should have (SELECT)
+    totalPrivileges = sentryStore.getAllMSentryPrivileges();
+    assertEquals(totalPrivileges.toString(),1, totalPrivileges.size());
+
     user = sentryStore.getMSentryUserByName(userName);
     privileges = user.getPrivileges();
     assertEquals(privileges.toString(), 1, privileges.size());


Mime
View raw message