You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by sr...@apache.org on 2014/06/19 20:22:38 UTC

git commit: SENTRY-305: SHOW CURRENT ROLES shouldn't require admin privileges ( Prasad Mujumdar via Sravya Tirukkovalur)

Repository: incubator-sentry
Updated Branches:
  refs/heads/master f42bc7734 -> e0267df7c


SENTRY-305: SHOW CURRENT ROLES shouldn't require admin privileges ( Prasad Mujumdar via Sravya Tirukkovalur)


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

Branch: refs/heads/master
Commit: e0267df7c4777254228906efec2ec5f64290caca
Parents: f42bc77
Author: Sravya Tirukkovalur <sr...@clouera.com>
Authored: Thu Jun 19 11:21:25 2014 -0700
Committer: Sravya Tirukkovalur <sr...@clouera.com>
Committed: Thu Jun 19 11:21:25 2014 -0700

----------------------------------------------------------------------
 .../hive/ql/exec/SentryGrantRevokeTask.java     |  6 +-
 .../binding/hive/authz/HiveAuthzBinding.java    | 28 ++++++-
 .../db/service/persistent/SentryStore.java      |  9 ++-
 .../thrift/SentryPolicyServiceClient.java       |  7 +-
 .../thrift/SentryPolicyStoreProcessor.java      | 13 +++-
 .../db/service/persistent/TestSentryStore.java  | 25 +++++--
 .../e2e/dbprovider/TestDatabaseProvider.java    | 78 ++++++++++++++++++++
 7 files changed, 146 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
index 0b65008..27a10ee 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
@@ -64,11 +64,11 @@ import org.apache.sentry.core.common.ActiveRoleSet;
 import org.apache.sentry.core.common.Authorizable;
 import org.apache.sentry.core.common.Subject;
 import org.apache.sentry.core.common.utils.PathUtils;
+import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.core.model.db.AccessURI;
 import org.apache.sentry.core.model.db.Database;
 import org.apache.sentry.core.model.db.Server;
 import org.apache.sentry.core.model.db.Table;
-import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.provider.db.SentryAccessDeniedException;
 import org.apache.sentry.provider.db.service.thrift.SentryPolicyServiceClient;
 import org.apache.sentry.provider.db.service.thrift.TSentryPrivilege;
@@ -221,7 +221,7 @@ public class SentryGrantRevokeTask extends Task<DDLWork> implements Serializable
     String name = desc.getName();
     try {
       if (operation.equals(RoleDDLDesc.RoleOperation.SET_ROLE)) {
-        hiveAuthzBinding.setActiveRoleSet(name);
+        hiveAuthzBinding.setActiveRoleSet(name, sentryClient.listUserRoles(subject));
         return RETURN_CODE_SUCCESS;
       } else if (operation.equals(RoleDDLDesc.RoleOperation.CREATE_ROLE)) {
         sentryClient.createRole(subject, name);
@@ -246,7 +246,7 @@ public class SentryGrantRevokeTask extends Task<DDLWork> implements Serializable
       } else if(operation.equals(RoleDDLDesc.RoleOperation.SHOW_CURRENT_ROLE)) {
         ActiveRoleSet roleSet = hiveAuthzBinding.getActiveRoleSet();
         if( roleSet.isAll()) {
-          Set<TSentryRole> roles = sentryClient.listRoles(subject);
+          Set<TSentryRole> roles = sentryClient.listUserRoles(subject);
           writeToFile(writeRolesInfo(roles), desc.getResFile());
           return RETURN_CODE_SUCCESS;
         } else {

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
index b478ec2..cedf368 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
@@ -32,6 +32,7 @@ import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
 import org.apache.hadoop.hive.ql.metadata.AuthorizationException;
 import org.apache.hadoop.hive.ql.plan.HiveOperation;
 import org.apache.hadoop.hive.ql.session.SessionState;
+import org.apache.sentry.SentryUserException;
 import org.apache.sentry.binding.hive.conf.HiveAuthzConf;
 import org.apache.sentry.binding.hive.conf.HiveAuthzConf.AuthzConfVars;
 import org.apache.sentry.binding.hive.conf.InvalidConfigurationException;
@@ -44,8 +45,8 @@ import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType;
 import org.apache.sentry.core.model.db.Server;
 import org.apache.sentry.policy.common.PolicyEngine;
 import org.apache.sentry.provider.common.AuthorizationProvider;
-import org.apache.sentry.provider.common.NoAuthorizationProvider;
 import org.apache.sentry.provider.common.ProviderBackend;
+import org.apache.sentry.provider.db.service.thrift.TSentryRole;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -87,10 +88,15 @@ public class HiveAuthzBinding {
     this.open = true;
     this.activeRoleSet = parseActiveRoleSet(hiveConf.get(HiveAuthzConf.SENTRY_ACTIVE_ROLE_SET,
         authzConf.get(HiveAuthzConf.SENTRY_ACTIVE_ROLE_SET, "")).trim());
+  }
 
+  private static ActiveRoleSet parseActiveRoleSet(String name)
+      throws SentryUserException {
+    return parseActiveRoleSet(name, null);
   }
 
-  private static ActiveRoleSet parseActiveRoleSet(String name) {
+  private static ActiveRoleSet parseActiveRoleSet(String name,
+      Set<TSentryRole> allowedRoles) throws SentryUserException {
     // if unset, then we choose the default of ALL
     if (name.isEmpty()) {
       return ActiveRoleSet.ALL;
@@ -102,6 +108,19 @@ public class HiveAuthzBinding {
       String msg = "Role " + name + " is reserved";
       throw new IllegalArgumentException(msg);
     } else {
+      if (allowedRoles != null) {
+        // check if the user has been granted the role
+        boolean foundRole = false;
+        for (TSentryRole role : allowedRoles) {
+          if (role.getRoleName().equalsIgnoreCase(name)) {
+            foundRole = true;
+            break;
+          }
+        }
+        if (!foundRole) {
+          throw new SentryUserException("Not authorized to set role " + name);
+        }
+      }
       return new ActiveRoleSet(Sets.newHashSet(ROLE_SET_SPLITTER.split(name)));
     }
   }
@@ -312,8 +331,9 @@ public class HiveAuthzBinding {
       }
   }
 
-  public void setActiveRoleSet(String activeRoleSet) {
-    this.activeRoleSet = parseActiveRoleSet(activeRoleSet);
+  public void setActiveRoleSet(String activeRoleSet,
+      Set<TSentryRole> allowedRoles) throws SentryUserException {
+    this.activeRoleSet = parseActiveRoleSet(activeRoleSet, allowedRoles);
     hiveConf.set(HiveAuthzConf.SENTRY_ACTIVE_ROLE_SET, activeRoleSet);
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/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 fb8cfc2..cf381e5 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
@@ -41,7 +41,6 @@ import javax.jdo.Transaction;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.sentry.core.model.db.AccessConstants;
-import org.apache.sentry.core.model.db.DBModelAction;
 import org.apache.sentry.core.model.db.DBModelAuthorizable.AuthorizableType;
 import org.apache.sentry.provider.common.ProviderConstants;
 import org.apache.sentry.provider.db.SentryAccessDeniedException;
@@ -878,9 +877,13 @@ public class SentryStore {
    * @return : Set of thrift sentry role objects
    * @throws SentryNoSuchObjectException
    */
-  public Set<TSentryRole> getTSentryRolesByGroupName(String groupName)
+  public Set<TSentryRole> getTSentryRolesByGroupName(Set<String> groupNames)
       throws SentryNoSuchObjectException {
-    return convertToTSentryRoles(getMSentryRolesByGroupName(groupName));
+    Set<MSentryRole> roleSet = Sets.newHashSet();
+    for (String groupName : groupNames) {
+      roleSet.addAll(getMSentryRolesByGroupName(groupName));
+    }
+    return convertToTSentryRoles(roleSet);
   }
 
   private SetMultimap<String, String> getRoleToPrivilegeMap(Set<String> groups) {

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/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 27f617f..2db73c6 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
@@ -279,10 +279,15 @@ public class SentryPolicyServiceClient {
   }
 
   public Set<TSentryRole> listRoles(String requestorUserName)
-       throws SentryUserException {
+      throws SentryUserException {
     return listRolesByGroupName(requestorUserName, null);
   }
 
+  public Set<TSentryRole> listUserRoles(String requestorUserName)
+      throws SentryUserException {
+    return listRolesByGroupName(requestorUserName, AccessConstants.ALL);
+  }
+
   public void grantURIPrivilege(String requestorUserName,
       String roleName, String server, String uri)
   throws SentryUserException {

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/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 097056b..724dfa9 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
@@ -26,6 +26,7 @@ import java.util.Set;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.sentry.SentryUserException;
+import org.apache.sentry.core.model.db.AccessConstants;
 import org.apache.sentry.provider.common.GroupMappingService;
 import org.apache.sentry.provider.db.SentryAccessDeniedException;
 import org.apache.sentry.provider.db.SentryAlreadyExistsException;
@@ -312,11 +313,17 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface {
     TListSentryRolesResponse response = new TListSentryRolesResponse();
     TSentryResponseStatus status;
     Set<TSentryRole> roleSet = new HashSet<TSentryRole>();
+    Set<String> groups = new HashSet<String>();
     try {
-      //TODO: Handle authorization for metadata queries
-      authorize(request.getRequestorUserName(),
+      // Don't check admin permissions for listing requestor's own roles
+      if (AccessConstants.ALL.equalsIgnoreCase(request.getGroupName())) {
+        groups = getRequestorGroups(request.getRequestorUserName());
+      } else {
+        authorize(request.getRequestorUserName(),
           getRequestorGroups(request.getRequestorUserName()));
-      roleSet = sentryStore.getTSentryRolesByGroupName(request.getGroupName());
+        groups.add(request.getGroupName());
+      }
+      roleSet = sentryStore.getTSentryRolesByGroupName(groups);
       response.setRoles(roleSet);
       response.setStatus(Status.OK());
     } catch (SentryNoSuchObjectException e) {

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/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 35ba83a..144e20e 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
@@ -23,20 +23,14 @@ import static junit.framework.Assert.assertTrue;
 import static junit.framework.Assert.fail;
 
 import java.io.File;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.sentry.core.common.ActiveRoleSet;
-import org.apache.sentry.core.common.Authorizable;
 import org.apache.sentry.core.model.db.AccessConstants;
-import org.apache.sentry.core.model.db.AccessURI;
-import org.apache.sentry.core.model.db.Server;
 import org.apache.sentry.provider.db.SentryAlreadyExistsException;
 import org.apache.sentry.provider.db.SentryNoSuchObjectException;
 import org.apache.sentry.provider.db.service.model.MSentryPrivilege;
@@ -340,4 +334,23 @@ public class TestSentryStore {
             new TSentryActiveRoleSet(false, new HashSet<String>()))));
   }
 
+  @Test
+  public void testListRole() throws Exception {
+    String roleName1 = "role1", roleName2 = "role2", roleName3 = "role3";
+    String group1 = "group1", group2 = "group2";
+    String grantor = "g1";
+
+    sentryStore.createSentryRole(roleName1, grantor);
+    sentryStore.createSentryRole(roleName2, grantor);
+    sentryStore.createSentryRole(roleName3, grantor);
+
+    sentryStore.alterSentryRoleAddGroups(grantor, roleName1, Sets.newHashSet(new TSentryGroup(group1)));
+    sentryStore.alterSentryRoleAddGroups(grantor, roleName2, Sets.newHashSet(new TSentryGroup(group2)));
+    sentryStore.alterSentryRoleAddGroups(grantor, roleName3,
+        Sets.newHashSet(new TSentryGroup(group1), new TSentryGroup(group2)));
+
+    assertEquals(2, sentryStore.getTSentryRolesByGroupName(Sets.newHashSet(group1)).size());
+    assertEquals(2, sentryStore.getTSentryRolesByGroupName(Sets.newHashSet(group2)).size());
+    assertEquals(3, sentryStore.getTSentryRolesByGroupName(Sets.newHashSet(group1,group2)).size());
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e0267df7/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 52c9e9e..e32aae8 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
@@ -21,6 +21,7 @@ import static org.hamcrest.Matchers.equalToIgnoringCase;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.FileOutputStream;
@@ -1608,6 +1609,7 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration {
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
     statement.execute("CREATE ROLE role1");
+    statement.execute("GRANT ROLE role1 TO GROUP " + ADMINGROUP);
     statement.execute("SET ROLE role1");
     ResultSet resultSet = statement.executeQuery("SHOW CURRENT ROLES");
     ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
@@ -1621,6 +1623,82 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration {
     connection.close();
   }
 
+  /***
+   * Verify 'SHOW CURRENT ROLES' show all roles for the give user when there no
+   * 'SET ROLE' called
+   * @throws Exception
+   */
+  @Test
+  public void testShowAllCurrentRoles() throws Exception {
+    Connection connection = context.createConnection(ADMIN1);
+    Statement statement = context.createStatement(connection);
+    String testRole1 = "testRole1", testRole2 = "testRole2";
+    statement.execute("CREATE ROLE " + testRole1);
+    statement.execute("CREATE ROLE " + testRole2);
+    statement.execute("GRANT ROLE " + testRole1 + " TO GROUP " + ADMINGROUP);
+    statement.execute("GRANT ROLE " + testRole2 + " TO GROUP " + ADMINGROUP);
+    statement.execute("GRANT ROLE " + testRole1 + " TO GROUP " + USERGROUP1);
+    statement.execute("GRANT ROLE " + testRole2 + " TO GROUP " + USERGROUP1);
+
+    ResultSet resultSet = statement.executeQuery("SHOW CURRENT ROLES");
+    assertResultSize(resultSet, 2);
+
+    statement.execute("SET ROLE " + testRole1);
+    resultSet = statement.executeQuery("SHOW CURRENT ROLES");
+    assertResultSize(resultSet, 1);
+
+    statement.close();
+    connection.close();
+
+    connection = context.createConnection(USER1_1);
+    statement = context.createStatement(connection);
+
+    resultSet = statement.executeQuery("SHOW CURRENT ROLES");
+    assertResultSize(resultSet, 2);
+
+    statement.execute("SET ROLE " + testRole2);
+    resultSet = statement.executeQuery("SHOW CURRENT ROLES");
+    assertResultSize(resultSet, 1);
+
+    statement.close();
+    connection.close();
+  }
+
+  @Test
+  public void testSetRole() throws Exception {
+    Connection connection = context.createConnection(ADMIN1);
+    Statement statement = context.createStatement(connection);
+    String testRole0 = "testRole1", testRole1 = "testRole2";
+    statement.execute("CREATE ROLE " + testRole0);
+    statement.execute("CREATE ROLE " + testRole1);
+
+    statement.execute("GRANT ROLE " + testRole0 + " TO GROUP " + ADMINGROUP);
+    statement.execute("GRANT ROLE " + testRole1 + " TO GROUP " + USERGROUP1);
+
+    statement.execute("SET ROLE " + testRole0);
+    try {
+      statement.execute("SET ROLE " + testRole1);
+      fail("User " + ADMIN1 + " shouldn't be able to set " + testRole1);
+    } catch (SQLException e) {
+      assertTrue(e.getMessage().contains("Not authorized to set role"));
+    }
+    statement.close();
+    connection.close();
+
+    connection = context.createConnection(USER1_1);
+    statement = context.createStatement(connection);
+    statement.execute("SET ROLE " + testRole1);
+    try {
+      statement.execute("SET ROLE " + testRole0);
+      fail("User " + USER1_1 + " shouldn't be able to set " + testRole0);
+    } catch (SQLException e) {
+      assertTrue(e.getMessage().contains("Not authorized to set role"));
+    }
+    statement.close();
+    connection.close();
+
+  }
+
   // See SENTRY-166
   @Test
   public void testUriWithEquals() throws Exception {