You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by va...@apache.org on 2017/08/15 20:03:37 UTC

sentry git commit: SENTRY-1881: PrivilegeOperatePersistence throws wrong type of exceptions (Sergio Pena via Vamsee Yarlagadda)

Repository: sentry
Updated Branches:
  refs/heads/master 5842648cc -> b2107fc16


SENTRY-1881: PrivilegeOperatePersistence throws wrong type of exceptions (Sergio Pena via Vamsee Yarlagadda)


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

Branch: refs/heads/master
Commit: b2107fc164fedcfe8f7cb39088111a20b9fbb8c6
Parents: 5842648
Author: Vamsee Yarlagadda <va...@cloudera.com>
Authored: Mon Aug 14 16:42:33 2017 -0700
Committer: Vamsee Yarlagadda <va...@cloudera.com>
Committed: Mon Aug 14 16:42:33 2017 -0700

----------------------------------------------------------------------
 .../authz/SentryAuthorizationValidator.java     | 11 ++-
 .../sentry/sqoop/binding/SqoopAuthBinding.java  |  2 +-
 ...tSqoopAuthorizationProviderGeneralCases.java |  8 +-
 .../core/common/BitFieldActionFactory.java      |  6 +-
 .../core/model/sqoop/SqoopActionFactory.java    | 17 +++--
 .../core/model/sqoop/TestSqoopAction.java       |  3 +-
 .../sentry/policy/common/CommonPrivilege.java   | 13 +++-
 .../persistent/PrivilegeOperatePersistence.java | 78 ++++++++++----------
 8 files changed, 79 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java b/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java
index 51f3f29..186659b 100644
--- a/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java
+++ b/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/authz/SentryAuthorizationValidator.java
@@ -19,6 +19,7 @@ package org.apache.sentry.sqoop.authz;
 import java.util.List;
 
 import org.apache.sentry.core.common.Subject;
+import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.sqoop.PrincipalDesc;
 import org.apache.sentry.sqoop.PrincipalDesc.PrincipalType;
 import org.apache.sentry.sqoop.SentrySqoopError;
@@ -54,9 +55,15 @@ public class SentryAuthorizationValidator extends AuthorizationValidator {
         LOG.debug("Going to authorize check on privilege : " + privilege +
             " for principal: " + principal);
       }
-      if (!binding.authorize(new Subject(principalDesc.getName()), privilege)) {
+      try {
+        if (!binding.authorize(new Subject(principalDesc.getName()), privilege)) {
+          throw new SqoopException(SecurityError.AUTH_0014, "User " + principalDesc.getName() +
+              " does not have privileges for : " + privilege.toString());
+        }
+      } catch (SentryUserException e) {
         throw new SqoopException(SecurityError.AUTH_0014, "User " + principalDesc.getName() +
-            " does not have privileges for : " + privilege.toString());
+              " with privilege " + privilege.toString() + " could not be authorized because"
+            + " the following error: " + e.getMessage());
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java b/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
index 11e2aa4..5d0831e 100644
--- a/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
+++ b/sentry-binding/sentry-binding-sqoop/src/main/java/org/apache/sentry/sqoop/binding/SqoopAuthBinding.java
@@ -149,7 +149,7 @@ public class SqoopAuthBinding {
    * @param action
    * @return true or false
    */
-  public boolean authorize(Subject subject, MPrivilege privilege) {
+  public boolean authorize(Subject subject, MPrivilege privilege) throws SentryUserException {
     List<Authorizable> authorizables = toAuthorizable(privilege.getResource());
     if (!hasServerInclude(authorizables)) {
       authorizables.add(0, sqoopServer);

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java b/sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java
index 7ce8881..9c925db 100644
--- a/sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java
+++ b/sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/policy/sqoop/TestSqoopAuthorizationProviderGeneralCases.java
@@ -24,6 +24,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.sentry.core.model.sqoop.SqoopActionFactory;
 import org.junit.Assert;
 
 import org.apache.commons.io.FileUtils;
@@ -35,7 +36,6 @@ import org.apache.sentry.core.model.sqoop.Connector;
 import org.apache.sentry.core.model.sqoop.Job;
 import org.apache.sentry.core.model.sqoop.Link;
 import org.apache.sentry.core.model.sqoop.Server;
-import org.apache.sentry.core.model.sqoop.SqoopActionConstant;
 import org.apache.sentry.core.model.sqoop.SqoopActionFactory.SqoopAction;
 import org.apache.sentry.core.model.sqoop.SqoopPrivilegeModel;
 import org.apache.sentry.provider.common.GroupMappingService;
@@ -73,9 +73,9 @@ public class TestSqoopAuthorizationProviderGeneralCases {
   private static final Job job1 = new Job("job1");
   private static final Job job2 = new Job("job2");
 
-  private static final SqoopAction ALL = new SqoopAction(SqoopActionConstant.ALL);
-  private static final SqoopAction READ = new SqoopAction(SqoopActionConstant.READ);
-  private static final SqoopAction WRITE = new SqoopAction(SqoopActionConstant.WRITE);
+  private static final SqoopAction ALL = new SqoopAction(SqoopActionFactory.SqoopActionType.ALL);
+  private static final SqoopAction READ = new SqoopAction(SqoopActionFactory.SqoopActionType.READ);
+  private static final SqoopAction WRITE = new SqoopAction(SqoopActionFactory.SqoopActionType.WRITE);
 
   private static final String ADMIN = "admin";
   private static final String DEVELOPER = "developer";

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldActionFactory.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldActionFactory.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldActionFactory.java
index 3789da7..ac98779 100644
--- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldActionFactory.java
+++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/BitFieldActionFactory.java
@@ -17,6 +17,8 @@
  */
 package org.apache.sentry.core.common;
 
+import org.apache.sentry.core.common.exception.SentryUserException;
+
 import java.util.List;
 
 public abstract class BitFieldActionFactory {
@@ -27,11 +29,11 @@ public abstract class BitFieldActionFactory {
    * @param actionCode
    * @return The BitFieldAction List
    */
-  public abstract List<? extends BitFieldAction> getActionsByCode(int actionCode);
+  public abstract List<? extends BitFieldAction> getActionsByCode(int actionCode) throws SentryUserException;
   /**
    * Get the BitFieldAction from the given name
    * @param name
    * @return
    */
-  public abstract BitFieldAction getActionByName(String name);
+  public abstract BitFieldAction getActionByName(String name) throws SentryUserException;
 }

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java b/sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java
index e7ba5f1..ef190e0 100644
--- a/sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java
+++ b/sentry-core/sentry-core-model-sqoop/src/main/java/org/apache/sentry/core/model/sqoop/SqoopActionFactory.java
@@ -22,9 +22,10 @@ import org.apache.sentry.core.common.BitFieldAction;
 import org.apache.sentry.core.common.BitFieldActionFactory;
 
 import com.google.common.collect.Lists;
+import org.apache.sentry.core.common.exception.SentryUserException;
 
 public class SqoopActionFactory extends BitFieldActionFactory {
-  enum SqoopActionType {
+  public enum SqoopActionType {
     READ(SqoopActionConstant.READ,1),
     WRITE(SqoopActionConstant.WRITE,2),
     ALL(SqoopActionConstant.ALL,READ.getCode() | WRITE.getCode());
@@ -44,16 +45,16 @@ public class SqoopActionFactory extends BitFieldActionFactory {
       return name;
     }
 
-    static SqoopActionType getActionByName(String name) {
+    static SqoopActionType getActionByName(String name) throws SentryUserException {
       for (SqoopActionType action : SqoopActionType.values()) {
         if (action.name.equalsIgnoreCase(name)) {
           return action;
         }
       }
-      throw new RuntimeException("can't get sqoopActionType by name:" + name);
+      throw new SentryUserException("can't get sqoopActionType by name:" + name);
     }
 
-    static List<SqoopActionType> getActionByCode(int code) {
+    static List<SqoopActionType> getActionByCode(int code) throws SentryUserException {
       List<SqoopActionType> actions = Lists.newArrayList();
       for (SqoopActionType action : SqoopActionType.values()) {
         if ((action.code & code) == action.code && action != SqoopActionType.ALL) {
@@ -62,14 +63,14 @@ public class SqoopActionFactory extends BitFieldActionFactory {
         }
       }
       if (actions.isEmpty()) {
-        throw new RuntimeException("can't get sqoopActionType by code:" + code);
+        throw new SentryUserException("can't get sqoopActionType by code:" + code);
       }
       return actions;
     }
   }
 
   public static class SqoopAction extends BitFieldAction {
-    public SqoopAction(String name) {
+    public SqoopAction(String name) throws SentryUserException {
       this(SqoopActionType.getActionByName(name));
     }
     public SqoopAction(SqoopActionType sqoopActionType) {
@@ -78,7 +79,7 @@ public class SqoopActionFactory extends BitFieldActionFactory {
   }
 
   @Override
-  public BitFieldAction getActionByName(String name) {
+  public BitFieldAction getActionByName(String name) throws SentryUserException {
     //Check the name is All
     if (SqoopActionConstant.ALL_NAME.equalsIgnoreCase(name)) {
       return new SqoopAction(SqoopActionType.ALL);
@@ -87,7 +88,7 @@ public class SqoopActionFactory extends BitFieldActionFactory {
   }
 
   @Override
-  public List<? extends BitFieldAction> getActionsByCode(int code) {
+  public List<? extends BitFieldAction> getActionsByCode(int code) throws SentryUserException {
     List<SqoopAction> actions = Lists.newArrayList();
     for (SqoopActionType action : SqoopActionType.getActionByCode(code)) {
       actions.add(new SqoopAction(action));

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java
----------------------------------------------------------------------
diff --git a/sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java b/sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java
index 9c86158..cde9b52 100644
--- a/sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java
+++ b/sentry-core/sentry-core-model-sqoop/src/test/java/org/apache/sentry/core/model/sqoop/TestSqoopAction.java
@@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.model.sqoop.SqoopActionFactory.SqoopAction;
 import org.junit.Test;
 
@@ -29,7 +30,7 @@ public class TestSqoopAction {
   private SqoopActionFactory factory = new SqoopActionFactory();
 
   @Test
-  public void testImpliesAction() {
+  public void testImpliesAction() throws SentryUserException {
     SqoopAction readAction = (SqoopAction)factory.getActionByName(SqoopActionConstant.READ);
     SqoopAction writeAction = (SqoopAction)factory.getActionByName(SqoopActionConstant.WRITE);
     SqoopAction allAction = (SqoopAction)factory.getActionByName(SqoopActionConstant.ALL);

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
----------------------------------------------------------------------
diff --git a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
index e227535..ab55609 100644
--- a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
+++ b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
@@ -23,6 +23,7 @@ import org.apache.sentry.core.common.BitFieldAction;
 import org.apache.sentry.core.common.BitFieldActionFactory;
 import org.apache.sentry.core.common.ImplyMethodType;
 import org.apache.sentry.core.common.Model;
+import org.apache.sentry.core.common.exception.SentryUserException;
 import org.apache.sentry.core.common.utils.KeyValue;
 import org.apache.sentry.core.common.utils.PathUtils;
 import org.apache.sentry.core.common.utils.SentryConstants;
@@ -160,8 +161,16 @@ public class CommonPrivilege implements Privilege {
   // for Solr, the action will be update, query, etc.
   private boolean impliesAction(String policyValue, String requestValue,
                                 BitFieldActionFactory bitFieldActionFactory) {
-    BitFieldAction currentAction = bitFieldActionFactory.getActionByName(policyValue);
-    BitFieldAction requestAction = bitFieldActionFactory.getActionByName(requestValue);
+    BitFieldAction currentAction;
+    BitFieldAction requestAction;
+
+    try {
+      currentAction = bitFieldActionFactory.getActionByName(policyValue);
+      requestAction = bitFieldActionFactory.getActionByName(requestValue);
+    } catch (SentryUserException e) {
+      return false;
+    }
+
     // the action in privilege is not supported
     if (currentAction == null || requestAction == null) {
       return false;

http://git-wip-us.apache.org/repos/asf/sentry/blob/b2107fc1/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
index 37484ed..d8b4887 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java
@@ -74,7 +74,7 @@ public class PrivilegeOperatePersistence {
 
   private final Configuration conf;
 
-  public PrivilegeOperatePersistence(Configuration conf) {
+  PrivilegeOperatePersistence(Configuration conf) {
     this.conf = conf;
   }
 
@@ -131,7 +131,7 @@ public class PrivilegeOperatePersistence {
    * @param privilege Source privilege
    * @return ParamBuilder suitable for executing the query
    */
-  public static QueryParamBuilder populateIncludePrivilegesParams(MSentryGMPrivilege privilege) {
+  private static QueryParamBuilder populateIncludePrivilegesParams(MSentryGMPrivilege privilege) {
     QueryParamBuilder paramBuilder = QueryParamBuilder.newQueryParamBuilder();
     paramBuilder.add(SERVICE_NAME, toNULLCol(privilege.getServiceName()), true);
     paramBuilder.add(COMPONENT_NAME, toNULLCol(privilege.getComponentName()), true);
@@ -184,8 +184,8 @@ public class PrivilegeOperatePersistence {
   }
 
   private void grantRolePartial(MSentryGMPrivilege grantPrivilege,
-      MSentryRole role,PersistenceManager pm) {
-    /**
+      MSentryRole role,PersistenceManager pm) throws SentryUserException {
+    /*
      * If Grant is for ALL action and other actions belongs to ALL action already exists..
      * need to remove it and GRANT ALL action
      */
@@ -194,7 +194,7 @@ public class PrivilegeOperatePersistence {
     BitFieldAction allAction = getAction(component, Action.ALL);
 
     if (action.implies(allAction)) {
-      /**
+      /*
        * ALL action is a multi-bit set action that includes some actions such as INSERT,SELECT and CREATE.
        */
       List<? extends BitFieldAction> actions = getActionFactory(component).getActionsByCode(allAction.getActionCode());
@@ -202,7 +202,7 @@ public class PrivilegeOperatePersistence {
         grantPrivilege.setAction(ac.getValue());
         MSentryGMPrivilege existPriv = getPrivilege(grantPrivilege, pm);
         if (existPriv != null && role.getGmPrivileges().contains(existPriv)) {
-          /**
+          /*
            * force to load all roles related this privilege
            * avoid the lazy-loading risk,such as:
            * if the roles field of privilege aren't loaded, then the roles is a empty set
@@ -215,7 +215,7 @@ public class PrivilegeOperatePersistence {
         }
       }
     } else {
-      /**
+      /*
        * If ALL Action already exists..
        * do nothing.
        */
@@ -226,11 +226,11 @@ public class PrivilegeOperatePersistence {
       }
     }
 
-    /**
+    /*
      * restore the action
      */
     grantPrivilege.setAction(action.getValue());
-    /**
+    /*
      * check the privilege is exist or not
      */
     MSentryGMPrivilege mPrivilege = getPrivilege(grantPrivilege, pm);
@@ -247,18 +247,18 @@ public class PrivilegeOperatePersistence {
     if (mPrivilege == null) {
       mPrivilege = convertToPrivilege(privilege);
     } else {
-      mPrivilege = (MSentryGMPrivilege) pm.detachCopy(mPrivilege);
+      mPrivilege = pm.detachCopy(mPrivilege);
     }
 
     Set<MSentryGMPrivilege> privilegeGraph = Sets.newHashSet();
     privilegeGraph.addAll(populateIncludePrivileges(Sets.newHashSet(role), mPrivilege, pm));
 
-    /**
+    /*
      * Get the privilege graph
      * populateIncludePrivileges will get the privileges that needed revoke
      */
     for (MSentryGMPrivilege persistedPriv : privilegeGraph) {
-      /**
+      /*
        * force to load all roles related this privilege
        * avoid the lazy-loading risk,such as:
        * if the roles field of privilege aren't loaded, then the roles is a empty set
@@ -298,25 +298,25 @@ public class PrivilegeOperatePersistence {
    */
   private void revokeRolePartial(MSentryGMPrivilege revokePrivilege,
       MSentryGMPrivilege persistedPriv, MSentryRole role,
-      PersistenceManager pm) {
+      PersistenceManager pm) throws SentryUserException {
     String component = revokePrivilege.getComponentName();
     BitFieldAction revokeaction = getAction(component, revokePrivilege.getAction());
     BitFieldAction persistedAction = getAction(component, persistedPriv.getAction());
     BitFieldAction allAction = getAction(component, Action.ALL);
 
     if (revokeaction.implies(allAction)) {
-      /**
+      /*
        * if revoke action is ALL, directly revoke its children privileges and itself
        */
       persistedPriv.removeRole(role);
       pm.makePersistent(persistedPriv);
     } else {
-      /**
+      /*
        * if persisted action is ALL, it only revoke the requested action and left partial actions
        * like the requested action is SELECT, the UPDATE and CREATE action are left
        */
       if (persistedAction.implies(allAction)) {
-        /**
+        /*
          * revoke the ALL privilege
          */
         persistedPriv.removeRole(role);
@@ -325,7 +325,7 @@ public class PrivilegeOperatePersistence {
         List<? extends BitFieldAction> actions = getActionFactory(component).getActionsByCode(allAction.getActionCode());
         for (BitFieldAction ac: actions) {
           if (ac.getActionCode() != revokeaction.getActionCode()) {
-            /**
+            /*
              * grant the left privileges to role
              */
             MSentryGMPrivilege tmpPriv = new MSentryGMPrivilege(persistedPriv);
@@ -341,14 +341,14 @@ public class PrivilegeOperatePersistence {
           }
         }
       } else if (revokeaction.implies(persistedAction)) {
-        /**
+        /*
          * if the revoke action is equal to the persisted action and they aren't ALL action
          * directly remove the role from privilege
          */
         persistedPriv.removeRole(role);
         pm.makePersistent(persistedPriv);
       }
-      /**
+      /*
        * if the revoke action is not equal to the persisted action,
        * do nothing
        */
@@ -358,13 +358,13 @@ public class PrivilegeOperatePersistence {
   /**
    * Drop any role related to the requested privilege and its children privileges
    */
-  public void dropPrivilege(PrivilegeObject privilege,PersistenceManager pm) {
+  public void dropPrivilege(PrivilegeObject privilege,PersistenceManager pm) throws SentryUserException {
     MSentryGMPrivilege requestPrivilege = convertToPrivilege(privilege);
 
     if (Strings.isNullOrEmpty(privilege.getAction())) {
       requestPrivilege.setAction(getAction(privilege.getComponent(), Action.ALL).getValue());
     }
-    /**
+    /*
      * Get the privilege graph
      * populateIncludePrivileges will get the privileges that need dropped,
      */
@@ -372,7 +372,7 @@ public class PrivilegeOperatePersistence {
     privilegeGraph.addAll(populateIncludePrivileges(null, requestPrivilege, pm));
 
     for (MSentryGMPrivilege mPrivilege : privilegeGraph) {
-      /**
+      /*
        * force to load all roles related this privilege
        * avoid the lazy-loading
        */
@@ -434,9 +434,9 @@ public class PrivilegeOperatePersistence {
     return privileges;
   }
 
-  public Set<PrivilegeObject> getPrivilegesByProvider(String component,
-      String service, Set<MSentryRole> roles,
-      List<? extends Authorizable> authorizables, PersistenceManager pm) {
+  Set<PrivilegeObject> getPrivilegesByProvider(String component,
+                                               String service, Set<MSentryRole> roles,
+                                               List<? extends Authorizable> authorizables, PersistenceManager pm) {
     Set<PrivilegeObject> privileges = Sets.newHashSet();
     if (roles == null || roles.isEmpty()) {
       return privileges;
@@ -458,9 +458,9 @@ public class PrivilegeOperatePersistence {
     return privileges;
   }
 
-  public Set<MSentryGMPrivilege> getPrivilegesByAuthorizable(String component,
-      String service, Set<MSentryRole> roles,
-      List<? extends Authorizable> authorizables, PersistenceManager pm) {
+  Set<MSentryGMPrivilege> getPrivilegesByAuthorizable(String component,
+                                                      String service, Set<MSentryRole> roles,
+                                                      List<? extends Authorizable> authorizables, PersistenceManager pm) {
 
     Set<MSentryGMPrivilege> privilegeGraph = Sets.newHashSet();
 
@@ -479,7 +479,7 @@ public class PrivilegeOperatePersistence {
       throws SentryUserException {
     MSentryGMPrivilege oldPrivilege = new MSentryGMPrivilege(component, service, oldAuthorizables, null, null);
     oldPrivilege.setAction(getAction(component,Action.ALL).getValue());
-    /**
+    /*
      * Get the privilege graph
      * populateIncludePrivileges will get the old privileges that need dropped
      */
@@ -487,7 +487,7 @@ public class PrivilegeOperatePersistence {
     privilegeGraph.addAll(populateIncludePrivileges(null, oldPrivilege, pm));
 
     for (MSentryGMPrivilege dropPrivilege : privilegeGraph) {
-      /**
+      /*
        * construct the new privilege needed to add
        */
       List<Authorizable> authorizables = new ArrayList<Authorizable>(
@@ -499,7 +499,7 @@ public class PrivilegeOperatePersistence {
           component,service, authorizables, dropPrivilege.getAction(),
           dropPrivilege.getGrantOption());
 
-      /**
+      /*
        * force to load all roles related this privilege
        * avoid the lazy-loading
        */
@@ -513,16 +513,16 @@ public class PrivilegeOperatePersistence {
     }
   }
 
-  private BitFieldAction getAction(String component, String name) {
+  private BitFieldAction getAction(String component, String name) throws SentryUserException {
     BitFieldActionFactory actionFactory = getActionFactory(component);
     BitFieldAction action = actionFactory.getActionByName(name);
     if (action == null) {
-      throw new RuntimeException("Can not get BitFieldAction for name: " + name);
+      throw new SentryUserException("Can not get BitFieldAction for name: " + name);
     }
     return action;
   }
 
-  private BitFieldActionFactory getActionFactory(String component) {
+  private BitFieldActionFactory getActionFactory(String component) throws SentryUserException {
     String caseInsensitiveComponent = component.toLowerCase();
     if (actionFactories.containsKey(caseInsensitiveComponent)) {
       return actionFactories.get(caseInsensitiveComponent);
@@ -534,11 +534,11 @@ public class PrivilegeOperatePersistence {
     return actionFactory;
   }
 
-  private BitFieldActionFactory createActionFactory(String component) {
+  private BitFieldActionFactory createActionFactory(String component) throws SentryUserException {
     String actionFactoryClassName =
       conf.get(String.format(ServiceConstants.ServerConfig.SENTRY_COMPONENT_ACTION_FACTORY_FORMAT, component));
     if (actionFactoryClassName == null) {
-      throw new RuntimeException("ActionFactory not defined for component " + component +
+      throw new SentryUserException("ActionFactory not defined for component " + component +
                                    ". Please define the parameter " +
                                    "sentry." + component + ".action.factory in configuration");
     }
@@ -546,10 +546,10 @@ public class PrivilegeOperatePersistence {
     try {
       actionFactoryClass = Class.forName(actionFactoryClassName);
     } catch (ClassNotFoundException e) {
-      throw new RuntimeException("ActionFactory class " + actionFactoryClassName + " not found.");
+      throw new SentryUserException("ActionFactory class " + actionFactoryClassName + " not found.");
     }
     if (!BitFieldActionFactory.class.isAssignableFrom(actionFactoryClass)) {
-      throw new RuntimeException("ActionFactory class " + actionFactoryClassName + " must extend "
+      throw new SentryUserException("ActionFactory class " + actionFactoryClassName + " must extend "
                                    + BitFieldActionFactory.class.getName());
     }
     BitFieldActionFactory actionFactory;
@@ -558,7 +558,7 @@ public class PrivilegeOperatePersistence {
       actionFactoryConstructor.setAccessible(true);
       actionFactory = (BitFieldActionFactory) actionFactoryClass.newInstance();
     } catch (NoSuchMethodException | InstantiationException | IllegalAccessException e) {
-      throw new RuntimeException("Could not instantiate actionFactory " + actionFactoryClassName +
+      throw new SentryUserException("Could not instantiate actionFactory " + actionFactoryClassName +
                                    " for component: " + component, e);
     }
     return actionFactory;