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 <xiaomeng.hu...@intel.com> Authored: Wed Feb 4 10:21:24 2015 +0800 Committer: Huang Xiaomeng <xiaomeng.hu...@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(); + } + }
