sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ha...@apache.org
Subject sentry git commit: SENTRY-1217: NPE for list_sentry_privileges_by_authorizable when activeRoleSet is not set (Hao Hao, Reviewed by: Lenni Kuff)
Date Tue, 26 Apr 2016 04:13:51 GMT
Repository: sentry
Updated Branches:
  refs/heads/master 66b32afa8 -> 370fab099


SENTRY-1217: NPE for list_sentry_privileges_by_authorizable when activeRoleSet is not set
(Hao Hao, Reviewed by: Lenni Kuff)

Change-Id: I8a59320d737209234fe6105a7ba734fd1df45566


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

Branch: refs/heads/master
Commit: 370fab099b93d4265f25af01d1d864b7a829d86d
Parents: 66b32af
Author: hahao <hao.hao@cloudera.com>
Authored: Mon Apr 25 21:12:57 2016 -0700
Committer: hahao <hao.hao@cloudera.com>
Committed: Mon Apr 25 21:12:57 2016 -0700

----------------------------------------------------------------------
 .../thrift/SentryGenericPolicyProcessor.java    | 25 +++++++++++++++-----
 .../TestPrivilegeOperatePersistence.java        |  2 ++
 .../TestSentryGenericPolicyProcessor.java       | 12 +++++++++-
 3 files changed, 32 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/sentry/blob/370fab09/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 19c8dd8..2a287e9 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
@@ -689,11 +689,12 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
           requestedGroups = memberGroups;
         }
 
-        // Disallow non-admin to lookup roles that they are not part of
+        Set<String> grantedRoles = toTrimmedLower(store.getRolesByGroups(request.getComponent(),
requestedGroups));
+
+        // If activeRoleSet is not null, disallow non-admin to lookup roles that they are
not part of.
         if (activeRoleSet != null && !activeRoleSet.isAll()) {
-          Set<String> grantedRoles = toTrimmedLower(store.getRolesByGroups(request.getComponent(),
requestedGroups));
-          Set<String> activeRoleNames = toTrimmedLower(activeRoleSet.getRoles());
 
+          Set<String> activeRoleNames = toTrimmedLower(activeRoleSet.getRoles());
           for (String activeRole : activeRoleNames) {
             if (!grantedRoles.contains(activeRole)) {
               throw new SentryAccessDeniedException(ACCESS_DENIAL_MESSAGE
@@ -703,18 +704,30 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService.
 
           // For non-admin, valid active roles are intersection of active roles and granted
roles.
           validActiveRoles.addAll(activeRoleSet.isAll() ? grantedRoles : Sets.intersection(activeRoleNames,
grantedRoles));
+        } else {
+          // For non-admin, if activeRoleSet is null, valid active roles would be the granted
roles.
+          validActiveRoles.addAll(grantedRoles);
         }
       } else {
         Set<String> allRoles = toTrimmedLower(store.getAllRoleNames());
-        Set<String> activeRoleNames = toTrimmedLower(activeRoleSet.getRoles());
+        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, 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(activeRoleSet.isAll() ? allRoles : Sets.intersection(activeRoleNames,
allRoles));
+          validActiveRoles.addAll(isAllRoleSet ? allRoles : Sets.intersection(activeRoleNames,
allRoles));
         } else {
           Set<String> requestedRoles = toTrimmedLower(store.getRolesByGroups(request.getComponent(),
requestedGroups));
-          validActiveRoles.addAll(activeRoleSet.isAll() ? allRoles : Sets.intersection(activeRoleNames,
requestedRoles));
+          validActiveRoles.addAll(isAllRoleSet ? allRoles : Sets.intersection(activeRoleNames,
requestedRoles));
         }
       }
 

http://git-wip-us.apache.org/repos/asf/sentry/blob/370fab09/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
index 9cbd1bd..deefefa 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java
@@ -966,6 +966,8 @@ public class TestPrivilegeOperatePersistence extends SentryStoreIntegrationBase
 
     assertEquals(0, sentryStore.getPrivilegesByAuthorizable(SEARCH, service1, null,
         Arrays.asList(new Collection(COLLECTION_NAME), new Field(FIELD_NAME))).size());
+    assertEquals(1, sentryStore.getPrivilegesByAuthorizable(SEARCH, service1, Sets.newHashSet(roleName1),
+    Arrays.asList(new Collection(COLLECTION_NAME), new Field(FIELD_NAME))).size());
     assertEquals(2, sentryStore.getPrivilegesByAuthorizable(SEARCH, service1,
         Sets.newHashSet(roleName1), null).size());
     assertEquals(2, sentryStore.getPrivilegesByAuthorizable(SEARCH, service1,

http://git-wip-us.apache.org/repos/asf/sentry/blob/370fab09/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
index 84eeb82..cc0b28e 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
@@ -300,17 +300,27 @@ public class TestSentryGenericPolicyProcessor extends org.junit.Assert
{
     assertEquals(Status.OK, fromTSentryStatus(response3.getStatus()));
     assertEquals(2, response3.getPrivileges().size());
 
+    // Optional parameters activeRoleSet and requested group name are both provided.
     TListSentryPrivilegesByAuthRequest request4 = new TListSentryPrivilegesByAuthRequest();
     request4.setGroups(Sets.newHashSet(groupName));
     request4.setRoleSet(new TSentryActiveRoleSet(true, null));
     request4.setRequestorUserName(ADMIN_USER);
-
     Set<String> authorizablesSet = Sets.newHashSet("Collection=c1->Field=f1");
     request4.setAuthorizablesSet(authorizablesSet);
 
     TListSentryPrivilegesByAuthResponse response4 = processor.list_sentry_privileges_by_authorizable(request4);
     assertEquals(Status.OK, fromTSentryStatus(response4.getStatus()));
     assertEquals(1, response4.getPrivilegesMapByAuth().size());
+
+    // Optional parameters activeRoleSet and requested group name are both not provided.
+    TListSentryPrivilegesByAuthRequest request5 = new TListSentryPrivilegesByAuthRequest();
+    request5.setRequestorUserName("not_" + ADMIN_USER);
+    authorizablesSet = Sets.newHashSet("Collection=c1->Field=f2");
+    request5.setAuthorizablesSet(authorizablesSet);
+
+    TListSentryPrivilegesByAuthResponse response5 = processor.list_sentry_privileges_by_authorizable(request5);
+    assertEquals(Status.OK, fromTSentryStatus(response5.getStatus()));
+    assertEquals(1, response5.getPrivilegesMapByAuth().size());
   }
 
   @Test(expected=SentryConfigurationException.class)


Mime
View raw message