Repository: sentry Updated Branches: refs/heads/master c523c46ee -> d89955bd6
SENTRY-1234: JDO exception for list_sentry_privileges_by_authorizable (Hao Hao, Reviewed by: Anne Yu) Change-Id: Ifb1d9810577bf687ba83be8d0807aee64550742a Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/d89955bd Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/d89955bd Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/d89955bd Branch: refs/heads/master Commit: d89955bd6c5002b30a49a0832e0f3ac7b27af63a Parents: c523c46 Author: hahao <[email protected]> Authored: Thu May 5 13:26:54 2016 -0700 Committer: hahao <[email protected]> Committed: Thu May 5 13:26:54 2016 -0700 ---------------------------------------------------------------------- .../service/persistent/DelegateSentryStore.java | 22 +++++-- .../thrift/SentryGenericPolicyProcessor.java | 25 +++----- .../thrift/SentryGenericServiceClient.java | 20 ++++++ .../SentryGenericServiceClientDefaultImpl.java | 21 ++----- .../TestSentryGenericServiceIntegration.java | 64 ++++++++++++++++++++ 5 files changed, 114 insertions(+), 38 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/d89955bd/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java index d51b3ba..23f6a2d 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java @@ -440,14 +440,15 @@ public class DelegateSentryStore implements SentryStoreLayer { service = toTrimmedLower(service); Set<MSentryGMPrivilege> privileges = Sets.newHashSet(); + + if (validActiveRoles == null || validActiveRoles.isEmpty()) { + return privileges; + } + PersistenceManager pm = null; try { pm = openTransaction(); - if (validActiveRoles == null || validActiveRoles.size() == 0) { - return privileges; - } - Set<MSentryRole> mRoles = Sets.newHashSet(); for (String role : validActiveRoles) { MSentryRole mRole = getRole(role, pm); @@ -455,8 +456,19 @@ public class DelegateSentryStore implements SentryStoreLayer { mRoles.add(mRole); } } + //get the privileges - privileges.addAll(privilegeOperator.getPrivilegesByAuthorizable(component, service, mRoles, authorizables, pm)); + Set<MSentryGMPrivilege> mSentryGMPrivileges = privilegeOperator.getPrivilegesByAuthorizable(component, service, mRoles, authorizables, pm); + + for (MSentryGMPrivilege mSentryGMPrivilege : mSentryGMPrivileges) { + /** + * force to load all roles related this privilege + * avoid the lazy-loading + */ + pm.retrieve(mSentryGMPrivilege); + privileges.add(mSentryGMPrivilege); + } + } finally { commitTransaction(pm); } http://git-wip-us.apache.org/repos/asf/sentry/blob/d89955bd/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java index 2a287e9..57ea7b4 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java @@ -709,25 +709,18 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. validActiveRoles.addAll(grantedRoles); } } else { - Set<String> allRoles = toTrimmedLower(store.getAllRoleNames()); - Set<String> activeRoleNames = Sets.newHashSet(); - boolean isAllRoleSet = false; - - // If activeRoleSet (which is optional) is null, valid active role will be all roles. - if (activeRoleSet != null) { - activeRoleNames = toTrimmedLower(activeRoleSet.getRoles()); - isAllRoleSet = activeRoleSet.isAll(); - } else { - isAllRoleSet = true; + // For admin, if requestedGroups are empty, requested roles will be all roles. + Set<String> requestedRoles = toTrimmedLower(store.getAllRoleNames()); + if (requestedGroups != null && !requestedGroups.isEmpty()) { + requestedRoles = toTrimmedLower(store.getRolesByGroups(request.getComponent(), requestedGroups)); } - // For admin, if requestedGroups are empty, valid active roles are intersection of active roles and all roles. - // Otherwise, valid active roles are intersection of active roles and the roles of requestedGroups. - if (requestedGroups == null || requestedGroups.isEmpty()) { - validActiveRoles.addAll(isAllRoleSet ? allRoles : Sets.intersection(activeRoleNames, allRoles)); + // If activeRoleSet (which is optional) is not null, valid active role will be intersection + // of active roles and requested roles. Otherwise, valid active roles are the requested roles. + if (activeRoleSet != null && !activeRoleSet.isAll()) { + validActiveRoles.addAll(Sets.intersection(toTrimmedLower(activeRoleSet.getRoles()), requestedRoles)); } else { - Set<String> requestedRoles = toTrimmedLower(store.getRolesByGroups(request.getComponent(), requestedGroups)); - validActiveRoles.addAll(isAllRoleSet ? allRoles : Sets.intersection(activeRoleNames, requestedRoles)); + validActiveRoles.addAll(requestedRoles); } } http://git-wip-us.apache.org/repos/asf/sentry/blob/d89955bd/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java index 6050289..76ff15b 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java @@ -18,6 +18,7 @@ package org.apache.sentry.provider.db.generic.service.thrift; import java.util.List; +import java.util.Map; import java.util.Set; import org.apache.sentry.SentryUserException; @@ -173,5 +174,24 @@ public interface SentryGenericServiceClient { String serviceName, ActiveRoleSet roleSet, Set<String> groups, List<? extends Authorizable> authorizables) throws SentryUserException; + /** + * Get sentry privileges based on valid active roles and the authorize objects. Note that + * it is client responsibility to ensure the requestor username, etc. is not impersonated. + * + * @param component: The request respond to which component. + * @param serviceName: The name of service. + * @param requestorUserName: The requestor user name. + * @param authorizablesSet: The set of authorize objects. One authorize object is represented + * as a string. e.g resourceType1=resourceName1->resourceType2=resourceName2->resourceType3=resourceName3. + * @param groups: The requested groups. + * @param roleSet: The active roles set. + * + * @returns The mapping of authorize objects and TSentryPrivilegeMap(<role, set<privileges>). + * @throws SentryUserException + */ + Map<String, TSentryPrivilegeMap> listPrivilegsbyAuthorizable(String component, + String serviceName, String requestorUserName, Set<String> authorizablesSet, + Set<String> groups, ActiveRoleSet roleSet) throws SentryUserException; + void close(); } http://git-wip-us.apache.org/repos/asf/sentry/blob/d89955bd/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java index dce3dad..74b6963 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java @@ -24,7 +24,6 @@ import java.util.*; import javax.security.auth.callback.CallbackHandler; -import com.google.common.collect.Sets; import org.apache.hadoop.conf.Configuration; import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION; import org.apache.hadoop.net.NetUtils; @@ -537,14 +536,6 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi } } - private List<TAuthorizable> fromAuthorizable(List<? extends Authorizable> authorizables) { - List<TAuthorizable> tAuthorizables = Lists.newArrayList(); - for (Authorizable authorizable : authorizables) { - tAuthorizables.add(new TAuthorizable(authorizable.getTypeName(), authorizable.getName())); - } - return tAuthorizables; - } - /** * Get sentry privileges based on valid active roles and the authorize objects. Note that * it is client responsibility to ensure the requestor username, etc. is not impersonated. @@ -552,8 +543,8 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi * @param component: The request respond to which component. * @param serviceName: The name of service. * @param requestorUserName: The requestor user name. - * @param authorizablesSet: The set of authorize objects. Represented as a string. e.g - * resourceType1=resourceName1->resourceType2=resourceName2->resourceType3=resourceName3. + * @param authorizablesSet: The set of authorize objects. One authorize object is represented + * as a string. e.g resourceType1=resourceName1->resourceType2=resourceName2->resourceType3=resourceName3. * @param groups: The requested groups. * @param roleSet: The active roles set. * @@ -561,20 +552,16 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi * @throws SentryUserException */ public Map<String, TSentryPrivilegeMap> listPrivilegsbyAuthorizable(String component, - String serviceName, String requestorUserName, Set<List<? extends Authorizable>> authorizablesSet, + String serviceName, String requestorUserName, Set<String> authorizablesSet, Set<String> groups, ActiveRoleSet roleSet) throws SentryUserException { - Set<List<TAuthorizable>> authSet = Sets.newHashSet(); - for (List<? extends Authorizable> authorizables : authorizablesSet) { - authSet.add(fromAuthorizable(authorizables)); - } - TListSentryPrivilegesByAuthRequest request = new TListSentryPrivilegesByAuthRequest(); request.setProtocol_version(sentry_common_serviceConstants.TSENTRY_SERVICE_V2); request.setComponent(component); request.setServiceName(serviceName); request.setRequestorUserName(requestorUserName); + request.setAuthorizablesSet(authorizablesSet); if (groups == null) { request.setGroups(new HashSet<String>()); http://git-wip-us.apache.org/repos/asf/sentry/blob/d89955bd/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java index fcf0e7b..e230505 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java @@ -23,6 +23,7 @@ import static org.junit.Assert.fail; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Set; import org.apache.sentry.SentryUserException; @@ -386,6 +387,69 @@ public class TestSentryGenericServiceIntegration extends SentryGenericServiceInt } @Test + public void testGetPrivilegeByAuthorizable() throws Exception { + runTestAsSubject(new TestOperation(){ + @Override + public void runTestAsSubject() throws Exception { + String adminUser = ADMIN_USER; + Set<String> adminGroup = Sets.newHashSet(ADMIN_GROUP); + String testRole = "role1"; + Set<String> testGroup = Sets.newHashSet("group1"); + String testUser = "user1"; + setLocalGroupMapping(adminUser, adminGroup); + setLocalGroupMapping(testUser, testGroup); + writePolicyFile(); + + client.createRole(adminUser, testRole, SOLR); + client.addRoleToGroups(adminUser, testRole, SOLR, adminGroup); + + TSentryPrivilege queryPrivilege = new TSentryPrivilege(SOLR, "service1", + fromAuthorizable(Arrays.asList(new Collection("c1"), new Field("f1"))), + SearchConstants.QUERY); + + TSentryPrivilege updatePrivilege = new TSentryPrivilege(SOLR, "service1", + fromAuthorizable(Arrays.asList(new Collection("c1"), new Field("f2"))), + SearchConstants.UPDATE); + + client.grantPrivilege(adminUser, testRole, SOLR, queryPrivilege); + client.grantPrivilege(adminUser, testRole, SOLR, updatePrivilege); + + //test listPrivilegsbyAuthorizable without requested group and active role set. + assertEquals(1, client.listPrivilegsbyAuthorizable(SOLR, "service1", adminUser, + Sets.newHashSet(new String("Collection=c1->Field=f1")), null, null).size()); + + //test listPrivilegsbyAuthorizable with requested group (testGroup) + Map<String, TSentryPrivilegeMap> privilegeMap = client.listPrivilegsbyAuthorizable(SOLR, + "service1", adminUser, Sets.newHashSet(new String("Collection=c1->Field=f1")), testGroup, null); + TSentryPrivilegeMap actualMap = privilegeMap.get(new String("Collection=c1->Field=f1")); + assertEquals(0, actualMap.getPrivilegeMap().size()); + + //test listPrivilegsbyAuthorizable with active role set. + ActiveRoleSet roleSet = ActiveRoleSet.ALL; + assertEquals(1, client.listPrivilegsbyAuthorizable(SOLR, "service1", adminUser, + Sets.newHashSet(new String("Collection=c1->Field=f1")), null, roleSet).size()); + privilegeMap = client.listPrivilegsbyAuthorizable(SOLR, + "service1", adminUser, Sets.newHashSet(new String("Collection=c1->Field=f1")), null, roleSet); + actualMap = privilegeMap.get(new String("Collection=c1->Field=f1")); + assertEquals(1, actualMap.getPrivilegeMap().size()); + + privilegeMap = client.listPrivilegsbyAuthorizable(SOLR, + "service1", testUser, Sets.newHashSet(new String("Collection=c1->Field=f1")), null, roleSet); + actualMap = privilegeMap.get(new String("Collection=c1->Field=f1")); + assertEquals(0, actualMap.getPrivilegeMap().size()); + + // grant tesRole to testGroup. + client.addRoleToGroups(adminUser, testRole, SOLR, testGroup); + + privilegeMap = client.listPrivilegsbyAuthorizable(SOLR, + "service1", testUser, Sets.newHashSet(new String("Collection=c1")), null, roleSet); + actualMap = privilegeMap.get(new String("Collection=c1")); + assertEquals(1, actualMap.getPrivilegeMap().size()); + assertEquals(2, actualMap.getPrivilegeMap().get(testRole).size()); + }}); + } + + @Test public void testDropAndRenamePrivilege() throws Exception { runTestAsSubject(new TestOperation(){ @Override
