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 <[email protected]> Authored: Thu Jun 19 11:21:25 2014 -0700 Committer: Sravya Tirukkovalur <[email protected]> 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 {
