You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by bb...@apache.org on 2019/02/22 17:38:07 UTC

[nifi-registry] branch master updated: NIFIREG-225: Fix NPE in getAccessPolicesForUser (#156)

This is an automated email from the ASF dual-hosted git repository.

bbende pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi-registry.git


The following commit(s) were added to refs/heads/master by this push:
     new d4410de  NIFIREG-225: Fix NPE in getAccessPolicesForUser (#156)
d4410de is described below

commit d4410de8c01e9a128dd8e6b2769aaf828aa73b24
Author: Kevin Doran <kd...@apache.org>
AuthorDate: Fri Feb 22 12:37:59 2019 -0500

    NIFIREG-225: Fix NPE in getAccessPolicesForUser (#156)
---
 .../registry/service/AuthorizationService.java     | 12 ++++++--
 .../service/AuthorizationServiceSpec.groovy        | 34 ++++++++++++++--------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java b/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java
index 204f966..5190b64 100644
--- a/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java
+++ b/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/AuthorizationService.java
@@ -361,7 +361,15 @@ public class AuthorizationService {
         readLock.lock();
         try {
             return accessPolicyProvider.getAccessPolicies().stream()
-                    .filter(accessPolicy -> accessPolicy.getUsers().contains(userIdentifier))
+                    .filter(accessPolicy -> {
+                        if (accessPolicy.getUsers().contains(userIdentifier)) {
+                            return true;
+                        }
+                        return accessPolicy.getGroups().stream().anyMatch(g -> {
+                            final Group group = userGroupProvider.getGroup(g);
+                            return group != null && group.getUsers().contains(userIdentifier);
+                        });
+                    })
                     .map(this::accessPolicyToSummaryDTO)
                     .collect(Collectors.toList());
         } finally {
@@ -572,7 +580,7 @@ public class AuthorizationService {
         }
 
         Collection<Tenant> userTenants = userGroup.getUsers() != null
-                ? userGroup.getUsers().stream().map(this::tenantIdToDTO).collect(Collectors.toSet()) : null;
+                ? userGroup.getUsers().stream().map(this::tenantIdToDTO).filter(Objects::nonNull).collect(Collectors.toSet()) : null;
         Collection<AccessPolicySummary> accessPolicySummaries = getAccessPolicySummariesForUserGroup(userGroup.getIdentifier());
 
         UserGroup userGroupDTO = new UserGroup(userGroup.getIdentifier(), userGroup.getName());
diff --git a/nifi-registry-core/nifi-registry-framework/src/test/groovy/org/apache/nifi/registry/service/AuthorizationServiceSpec.groovy b/nifi-registry-core/nifi-registry-framework/src/test/groovy/org/apache/nifi/registry/service/AuthorizationServiceSpec.groovy
index 149ec36..4646c2f 100644
--- a/nifi-registry-core/nifi-registry-framework/src/test/groovy/org/apache/nifi/registry/service/AuthorizationServiceSpec.groovy
+++ b/nifi-registry-core/nifi-registry-framework/src/test/groovy/org/apache/nifi/registry/service/AuthorizationServiceSpec.groovy
@@ -106,25 +106,36 @@ class AuthorizationServiceSpec extends Specification {
     def "get user"() {
 
         setup:
-        userGroupProvider.getGroups() >> new HashSet<Group>()
-        accessPolicyProvider.getAccessPolicies() >> new HashSet<AccessPolicy>()
+        def user1 = new AuthUser.Builder().identifier("user-id-1").identity("user1").build()
+        def group1 = new Group.Builder().identifier("group-id-1").name("group1").addUser("user-id-1").build()
+        def apBuilder = new org.apache.nifi.registry.security.authorization.AccessPolicy.Builder().resource("/fake-resource").action(RequestAction.READ)
+        def ap1 = apBuilder.identifier("policy-1").addUser("user-id-1").build()
+        def ap2 = apBuilder.identifier("policy-2").clearUsers().addGroup("group-id-1").build()
+        def ap3 = apBuilder.identifier("policy-3").clearGroups().addGroup("does-not-exist").build()
+        userGroupProvider.getUser("does-not-exist") >> null
+        userGroupProvider.getUser("user-id-1") >> user1
+        userGroupProvider.getGroup("group-id-1") >> group1
+        userGroupProvider.getGroup("does-not-exist") >> null
+        userGroupProvider.getGroups() >> new HashSet<Group>([group1])
+        accessPolicyProvider.getAccessPolicies() >> new HashSet<>([ap1, ap2, ap3])
 
 
         when: "get user for existing user identifier"
-        userGroupProvider.getUser("userId") >> new AuthUser.Builder().identifier("userId").identity("username").build()
-        def user1 = authorizationService.getUser("userId")
+        def userDto1 = authorizationService.getUser("user-id-1")
 
         then: "user is returned converted to DTO"
-        with(user1) {
-            identifier == "userId"
-            identity == "username"
+        with(userDto1) {
+            identifier == "user-id-1"
+            identity == "user1"
+            userGroups.size() == 1
+            userGroups[0].identifier == "group-id-1"
+            accessPolicies.size() == 2
+            accessPolicies.stream().noneMatch({it.identifier == "policy-3"})
         }
 
 
-        when: "get user for non-existent user identifier"
-        userGroupProvider.getUser("nonExistentUserId") >> null
-        userGroupProvider.getGroup("nonExistentUserId") >> null
-        def user2 = authorizationService.getUser("nonExistentUserId")
+        when: "get user for non-existent tenant identifier"
+        def user2 = authorizationService.getUser("does-not-exist")
 
         then: "no user is returned"
         user2 == null
@@ -375,7 +386,6 @@ class AuthorizationServiceSpec extends Specification {
 
     }
 
-
     def "update access policy"() {
 
         setup: