You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by br...@apache.org on 2014/03/26 05:13:55 UTC

git commit: SENTRY-137 - Validate privilege scope in sentry service (Prasad via Brock)

Repository: incubator-sentry
Updated Branches:
  refs/heads/master 63c134f36 -> 4247d21fa


SENTRY-137 - Validate privilege scope in sentry service (Prasad via Brock)


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

Branch: refs/heads/master
Commit: 4247d21fa036685a098baac84262aa08693d1d82
Parents: 63c134f
Author: Brock Noland <br...@apache.org>
Authored: Tue Mar 25 21:13:15 2014 -0700
Committer: Brock Noland <br...@apache.org>
Committed: Tue Mar 25 21:13:15 2014 -0700

----------------------------------------------------------------------
 .../thrift/SentryPolicyServiceClient.java       | 38 ++++++++++----------
 .../thrift/SentryPolicyStoreProcessor.java      | 38 +++++++++++++++++---
 .../sentry/service/thrift/ServiceConstants.java |  9 +++++
 3 files changed, 63 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/4247d21f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
index 464569c..7e73b2c 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
@@ -30,6 +30,7 @@ import org.apache.sentry.SentryUserException;
 import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
+import org.apache.sentry.service.thrift.ServiceConstants.PrivilegeScope;
 import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig;
 import org.apache.sentry.service.thrift.ServiceConstants.ThriftConstants;
 import org.apache.sentry.service.thrift.Status;
@@ -154,33 +155,33 @@ public class SentryPolicyServiceClient {
   public void grantURIPrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server, String uri)
   throws SentryUserException {
-    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName, "SERVER", server, uri,
-        null, null, AccessConstants.ALL);
+    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.URI, server, uri, null, null, AccessConstants.ALL);
   }
 
   public void grantServerPrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server)
   throws SentryUserException {
-    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName, "SERVER", server, null,
-        null, null, AccessConstants.ALL);
+    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.SERVER, server, null, null, null, AccessConstants.ALL);
   }
 
   public void grantDatabasePrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server, String db)
   throws SentryUserException {
-    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName, "DATABASE", server, null,
-        db, null, AccessConstants.ALL);
+    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.DATABASE, server, null, db, null, AccessConstants.ALL);
   }
 
   public void grantTablePrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server, String db, String table, String action)
   throws SentryUserException {
-    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName, "TABLE", server, null,
+    grantPrivilege(requestorUserName, requestorUserGroupNames, roleName, PrivilegeScope.TABLE, server, null,
         db, table, action);
   }
 
   private void grantPrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
-      String roleName, String scope, String serverName, String uri, String db, String table, String action)
+      String roleName, PrivilegeScope scope, String serverName, String uri, String db, String table, String action)
   throws SentryUserException {
     TAlterSentryRoleGrantPrivilegeRequest request = new TAlterSentryRoleGrantPrivilegeRequest();
     request.setProtocol_version(ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT);
@@ -188,7 +189,7 @@ public class SentryPolicyServiceClient {
     request.setRequestorGroupNames(requestorUserGroupNames);
     request.setRoleName(roleName);
     TSentryPrivilege privilege = new TSentryPrivilege();
-    privilege.setPrivilegeScope(scope);
+    privilege.setPrivilegeScope(scope.toString());
     privilege.setServerName(serverName);
     privilege.setURI(uri);
     privilege.setDbName(db);
@@ -209,33 +210,34 @@ public class SentryPolicyServiceClient {
   public void revokeURIPrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server, String uri)
   throws SentryUserException {
-    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName, "SERVER", server, uri,
-        null, null, AccessConstants.ALL);
+    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.URI, server, uri, null, null, AccessConstants.ALL);
   }
 
   public void revokeServerPrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server)
   throws SentryUserException {
-    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName, "SERVER", server, null,
-        null, null, AccessConstants.ALL);
+    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.SERVER, server, null, null, null, AccessConstants.ALL);
   }
 
   public void revokeDatabasePrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server, String db)
   throws SentryUserException {
-    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName, "DATABASE", server, null,
-        db, null, AccessConstants.ALL);
+    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.DATABASE, server, null, db, null, AccessConstants.ALL);
   }
 
   public void revokeTablePrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
       String roleName, String server, String db, String table, String action)
   throws SentryUserException {
-    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName, "TABLE", server, null,
+    revokePrivilege(requestorUserName, requestorUserGroupNames, roleName,
+        PrivilegeScope.TABLE, server, null,
         db, table, action);
   }
 
   private void revokePrivilege(String requestorUserName, Set<String> requestorUserGroupNames,
-      String roleName, String scope, String serverName, String uri, String db, String table, String action)
+      String roleName, PrivilegeScope scope, String serverName, String uri, String db, String table, String action)
   throws SentryUserException {
     TAlterSentryRoleRevokePrivilegeRequest request = new TAlterSentryRoleRevokePrivilegeRequest();
     request.setProtocol_version(ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT);
@@ -243,7 +245,7 @@ public class SentryPolicyServiceClient {
     request.setRequestorGroupNames(requestorUserGroupNames);
     request.setRoleName(roleName);
     TSentryPrivilege privilege = new TSentryPrivilege();
-    privilege.setPrivilegeScope(scope);
+    privilege.setPrivilegeScope(scope.toString());
     privilege.setServerName(serverName);
     privilege.setURI(uri);
     privilege.setDbName(db);

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/4247d21f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
index 9562783..9793ab4 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
@@ -23,6 +23,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.sentry.provider.db.SentryAccessDeniedException;
 import org.apache.sentry.provider.db.SentryAlreadyExistsException;
@@ -31,6 +32,7 @@ import org.apache.sentry.provider.db.SentryNoSuchObjectException;
 import org.apache.sentry.provider.db.service.persistent.CommitContext;
 import org.apache.sentry.provider.db.service.persistent.SentryStore;
 import org.apache.sentry.provider.db.service.thrift.PolicyStoreConstants.PolicyStoreServerConfig;
+import org.apache.sentry.service.thrift.ServiceConstants.PrivilegeScope;
 import org.apache.sentry.service.thrift.Status;
 import org.apache.sentry.service.thrift.TSentryResponseStatus;
 import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig;
@@ -106,7 +108,6 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface {
     return handlers;
   }
 
-  //TODO:Validate privilege scope?
   @VisibleForTesting
   public static String constructPrivilegeName(TSentryPrivilege privilege) throws SentryInvalidInputException {
     StringBuilder privilegeName = new StringBuilder();
@@ -115,6 +116,7 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface {
     String tableName = privilege.getTableName();
     String uri = privilege.getURI();
     String action = privilege.getAction();
+    PrivilegeScope scope;
 
     if (serverName == null) {
       throw new SentryInvalidInputException("Server name is null");
@@ -126,10 +128,38 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface {
       }
     }
 
-    if (dbName == null || dbName.equals("")) {
-      if (tableName != null && !tableName.equals("")) {
-        throw new SentryInvalidInputException("Db name can't be null");
+    // Validate privilege scope
+    try {
+      scope = Enum.valueOf(PrivilegeScope.class, privilege.getPrivilegeScope());
+    } catch (IllegalArgumentException e) {
+      throw new SentryInvalidInputException("Invalid Privilege scope: " +
+          privilege.getPrivilegeScope());
+    }
+    if (PrivilegeScope.SERVER.equals(scope)) {
+      if (StringUtils.isNotEmpty(dbName) || StringUtils.isNotEmpty(tableName)) {
+        throw new SentryInvalidInputException("DB and TABLE names should not be "
+            + "set for SERVER scope");
+      }
+    } else if (PrivilegeScope.DATABASE.equals(scope)) {
+      if (StringUtils.isEmpty(dbName)) {
+        throw new SentryInvalidInputException("DB name not set for DB scope");
+      }
+      if (StringUtils.isNotEmpty(tableName)) {
+        StringUtils.isNotEmpty("TABLE names should not be set for DB scope");
       }
+    } else if (PrivilegeScope.TABLE.equals(scope)) {
+      if (StringUtils.isEmpty(dbName) || StringUtils.isEmpty(tableName)) {
+        throw new SentryInvalidInputException("TABLE or DB name not set for TABLE scope");
+      }
+    } else if (PrivilegeScope.URI.equals(scope)){
+      if (StringUtils.isEmpty(uri)) {
+        throw new SentryInvalidInputException("URI path not set for URI scope");
+      }
+      if (StringUtils.isNotEmpty(tableName)) {
+        throw new SentryInvalidInputException("TABLE should not be set for URI scope");
+      }
+    } else {
+      throw new SentryInvalidInputException("Unsupported operation scope: " + scope);
     }
 
     if (uri == null || uri.equals("")) {

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/4247d21f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
index 1b36690..6258471 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
@@ -122,4 +122,13 @@ public class ServiceConstants {
   public static class ThriftConstants extends org.apache.sentry.service.thrift.sentry_common_serviceConstants {
     public static final int TSENTRY_SERVICE_VERSION_CURRENT = TSENTRY_SERVICE_V1;
   }
+
+  /* Privilege operation scope */
+  public static enum PrivilegeScope {
+    SERVER,
+    URI,
+    DATABASE,
+    TABLE,
+    COLUMN
+  }
 }