You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by xi...@apache.org on 2015/02/04 03:43:28 UTC
incubator-sentry git commit: SENTRY-617: Improve grant role to groups
(Reviewed by Colin Ma)
Repository: incubator-sentry
Updated Branches:
refs/heads/master 5b6a7aeae -> c80ea5c40
SENTRY-617: Improve grant role to groups (Reviewed by Colin Ma)
Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/c80ea5c4
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/c80ea5c4
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/c80ea5c4
Branch: refs/heads/master
Commit: c80ea5c404321884ed749a5c4a151264c8821116
Parents: 5b6a7ae
Author: Huang Xiaomeng <xi...@intel.com>
Authored: Wed Feb 4 10:21:24 2015 +0800
Committer: Huang Xiaomeng <xi...@intel.com>
Committed: Wed Feb 4 10:21:24 2015 +0800
----------------------------------------------------------------------
.../hive/ql/exec/SentryGrantRevokeTask.java | 20 +++---
.../thrift/SentryPolicyServiceClient.java | 10 ++-
.../SentryPolicyServiceClientDefaultImpl.java | 42 ++++++++++---
.../e2e/dbprovider/TestDatabaseProvider.java | 66 ++++++++++++++++++++
4 files changed, 121 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c80ea5c4/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 69e97a6..0cbb250 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
@@ -86,6 +86,7 @@ import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
public class SentryGrantRevokeTask extends Task<DDLWork> implements Serializable {
private static final Logger LOG = LoggerFactory
@@ -397,21 +398,26 @@ public class SentryGrantRevokeTask extends Task<DDLWork> implements Serializable
boolean grantRole = desc.getGrant();
List<PrincipalDesc> principals = desc.getPrincipalDesc();
List<String> roles = desc.getRoles();
+ // get principals
+ Set<String> groups = Sets.newHashSet();
for (PrincipalDesc principal : principals) {
if (principal.getType() != PrincipalType.GROUP) {
String msg = SentryHiveConstants.GRANT_REVOKE_NOT_SUPPORTED_FOR_PRINCIPAL +
principal.getType();
throw new HiveException(msg);
}
- String groupName = principal.getName();
- for (String roleName : roles) {
- if (grantRole) {
- sentryClient.grantRoleToGroup(subject, groupName, roleName);
- } else {
- sentryClient.revokeRoleFromGroup(subject, groupName, roleName);
- }
+ groups.add(principal.getName());
+ }
+
+ // grant/revoke role to/from principals
+ for (String roleName : roles) {
+ if (grantRole) {
+ sentryClient.grantRoleToGroups(subject, roleName, groups);
+ } else {
+ sentryClient.revokeRoleFromGroups(subject, roleName, groups);
}
}
+
} catch (HiveException e) {
String msg = "Error in grant/revoke operation, error message " + e.getMessage();
LOG.warn(msg, e);
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c80ea5c4/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 962228f..7a9f0df 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
@@ -43,7 +43,7 @@ public interface SentryPolicyServiceClient {
/**
* Gets sentry privilege objects for a given roleName using the Sentry service
- *
+ *
* @param requestorUserName : user on whose behalf the request is issued
* @param roleName : roleName to look up
* @param authorizable : authorizable Hierarchy (server->db->table etc)
@@ -145,6 +145,12 @@ public interface SentryPolicyServiceClient {
public void revokeRoleFromGroup(String requestorUserName, String groupName, String roleName)
throws SentryUserException;
+ public void grantRoleToGroups(String requestorUserName, String roleName, Set<String> groups)
+ throws SentryUserException;
+
+ public void revokeRoleFromGroups(String requestorUserName, String roleName, Set<String> groups)
+ throws SentryUserException;
+
public void dropPrivileges(String requestorUserName,
List<? extends Authorizable> authorizableObjects) throws SentryUserException;
@@ -160,7 +166,7 @@ public interface SentryPolicyServiceClient {
* Returns the configuration value in the sentry server associated with propertyName, or if
* propertyName does not exist, the defaultValue. There is no "requestorUserName" because this is
* regarded as an internal interface.
- *
+ *
* @param propertyName Config attribute to search for
* @param defaultValue String to return if not found
* @return The value of the propertyName
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c80ea5c4/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
index f6fc6d4..44681ca 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java
@@ -657,12 +657,27 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService
}
}
+ @Override
public synchronized void grantRoleToGroup(String requestorUserName,
String groupName, String roleName)
throws SentryUserException {
- TAlterSentryRoleAddGroupsRequest request = new TAlterSentryRoleAddGroupsRequest(ThriftConstants.
-TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName,
- roleName, Sets.newHashSet(new TSentryGroup(groupName)));
+ grantRoleToGroups(requestorUserName, roleName, Sets.newHashSet(groupName));
+ }
+
+ @Override
+ public synchronized void revokeRoleFromGroup(String requestorUserName,
+ String groupName, String roleName)
+ throws SentryUserException {
+ revokeRoleFromGroups(requestorUserName, roleName, Sets.newHashSet(groupName));
+ }
+
+ @Override
+ public synchronized void grantRoleToGroups(String requestorUserName,
+ String roleName, Set<String> groups)
+ throws SentryUserException {
+ TAlterSentryRoleAddGroupsRequest request = new TAlterSentryRoleAddGroupsRequest(
+ ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName,
+ roleName, convert2TGroups(groups));
try {
TAlterSentryRoleAddGroupsResponse response = client.alter_sentry_role_add_groups(request);
Status.throwIfNotOk(response.getStatus());
@@ -671,12 +686,13 @@ TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName,
}
}
- public synchronized void revokeRoleFromGroup(String requestorUserName,
- String groupName, String roleName)
+ @Override
+ public synchronized void revokeRoleFromGroups(String requestorUserName,
+ String roleName, Set<String> groups)
throws SentryUserException {
- TAlterSentryRoleDeleteGroupsRequest request = new TAlterSentryRoleDeleteGroupsRequest(ThriftConstants.
-TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName,
- roleName, Sets.newHashSet(new TSentryGroup(groupName)));
+ TAlterSentryRoleDeleteGroupsRequest request = new TAlterSentryRoleDeleteGroupsRequest(
+ ThriftConstants.TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName,
+ roleName, convert2TGroups(groups));
try {
TAlterSentryRoleDeleteGroupsResponse response = client.alter_sentry_role_delete_groups(request);
Status.throwIfNotOk(response.getStatus());
@@ -685,6 +701,16 @@ TSENTRY_SERVICE_VERSION_CURRENT, requestorUserName,
}
}
+ private Set<TSentryGroup> convert2TGroups(Set<String> groups) {
+ Set<TSentryGroup> tGroups = Sets.newHashSet();
+ if (groups != null) {
+ for (String groupName : groups) {
+ tGroups.add(new TSentryGroup(groupName));
+ }
+ }
+ return tGroups;
+ }
+
public synchronized void dropPrivileges(String requestorUserName,
List<? extends Authorizable> authorizableObjects)
throws SentryUserException {
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c80ea5c4/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 4a475ba..f9e8f80 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
@@ -2003,4 +2003,70 @@ public class TestDatabaseProvider extends AbstractTestWithStaticConfiguration {
}
+ /**
+ * Regression test for SENTRY-617.
+ */
+ @Test
+ public void testGrantRevokeRoleToGroups() throws Exception {
+ super.setupAdmin();
+ Connection connection = context.createConnection(ADMIN1);
+ Statement statement = context.createStatement(connection);
+ statement.execute("DROP DATABASE IF EXISTS " + DB1 + " CASCADE");
+ statement.execute("CREATE DATABASE " + DB1);
+ statement.execute("USE " + DB1);
+ statement.execute("DROP TABLE IF EXISTS t1");
+ statement.execute("CREATE TABLE t1 (c1 string)");
+ statement.execute("CREATE ROLE user_role");
+ statement.execute("GRANT ALL ON TABLE t1 TO ROLE user_role");
+
+ // grant role to group user_group1, group user_group2, user_group3
+ statement.execute("GRANT ROLE user_role TO GROUP " + USERGROUP1
+ + ", GROUP " + USERGROUP2 + ", GROUP " + USERGROUP3);
+
+ // user1, user2, user3 should have permission to access table t1
+ connection = context.createConnection(USER1_1);
+ statement = context.createStatement(connection);
+ statement.execute("SELECT * FROM " + DB1 + ".t1");
+ connection = context.createConnection(USER2_1);
+ statement = context.createStatement(connection);
+ statement.execute("SELECT * FROM " + DB1 + ".t1");
+ connection = context.createConnection(USER3_1);
+ statement = context.createStatement(connection);
+ statement.execute("SELECT * FROM " + DB1 + ".t1");
+
+ // revoke role from group user_group1, group user_group2
+ connection = context.createConnection(ADMIN1);
+ statement = context.createStatement(connection);
+ statement.execute("REVOKE ROLE user_role FROM GROUP " + USERGROUP1
+ + ", GROUP " + USERGROUP2);
+
+ connection = context.createConnection(USER1_1);
+ statement = context.createStatement(connection);
+ try {
+ // INSERT is not allowed
+ statement.execute("SELECT * FROM " + DB1 + ".t1");
+ assertTrue("after revoking, user1 has no permission to access table t1", false);
+ } catch (Exception e) {
+ // Ignore
+ }
+
+ connection = context.createConnection(USER2_1);
+ statement = context.createStatement(connection);
+ try {
+ // INSERT is not allowed
+ statement.execute("SELECT * FROM " + DB1 + ".t1");
+ assertTrue("after revoking, user1 has no permission to access table t1", false);
+ } catch (Exception e) {
+ // Ignore
+ }
+
+ // user3 still has permission to access table t1
+ connection = context.createConnection(USER3_1);
+ statement = context.createStatement(connection);
+ statement.execute("SELECT * FROM " + DB1 + ".t1");
+
+ statement.close();
+ connection.close();
+ }
+
}