sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ha...@apache.org
Subject [5/6] sentry git commit: SENTRY-1217: NPE for list_sentry_privileges_by_authorizable when activeRoleSet is not set (Hao Hao, Reviewed by: Lenni Kuff)
Date Wed, 27 Apr 2016 01:30:40 GMT
SENTRY-1217: NPE for list_sentry_privileges_by_authorizable when activeRoleSet is not set (Hao
Hao, Reviewed by: Lenni Kuff)


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

Branch: refs/heads/branch-1.7.0
Commit: 9d175cef62233108c60a0c17f7868c53a1a4b0f3
Parents: 00951b4
Author: hahao <hao.hao@cloudera.com>
Authored: Tue Apr 26 18:02:29 2016 -0700
Committer: hahao <hao.hao@cloudera.com>
Committed: Tue Apr 26 18:27:46 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/9d175cef/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 58be24d..bff97ab 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/9d175cef/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/9d175cef/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