diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/UserAndGroups.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/UserAndGroups.java index 486eba495b..1011c01299 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/UserAndGroups.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/UserAndGroups.java @@ -23,6 +23,21 @@ import java.util.Set; */ public interface UserAndGroups { + /** + * A static, immutable, empty implementation of the UserAndGroups interface. + */ + UserAndGroups EMPTY = new UserAndGroups() { + @Override + public User getUser() { + return null; + } + + @Override + public Set getGroups() { + return null; + } + }; + /** * Retrieves the user, or null if the user is unknown * diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java index f466ce7061..915bff03a4 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactory.java @@ -47,11 +47,12 @@ public final class AuthorizerFactory { } /** - * Checks if another user exists with the same identity. + * Checks if another tenant (user or group) exists with the same identity. * - * @param identifier identity of the user - * @param identity identity of the user - * @return true if another user exists with the same identity, false otherwise + * @param userGroupProvider the userGroupProvider to use to lookup the tenant + * @param identifier identity of the tenant + * @param identity identity of the tenant + * @return true if another tenant exists with the same identity, false otherwise */ private static boolean tenantExists(final UserGroupProvider userGroupProvider, final String identifier, final String identity) { for (User user : userGroupProvider.getUsers()) { @@ -71,6 +72,24 @@ public final class AuthorizerFactory { return false; } + /** + * Check that all users in the group exist. + * + * @param userGroupProvider the userGroupProvider to use to lookup the users + * @param group the group whose users will be checked for existence. + * @return true if another user exists with the same identity, false otherwise + */ + private static boolean allGroupUsersExist(final UserGroupProvider userGroupProvider, final Group group) { + for (String userIdentifier : group.getUsers()) { + User user = userGroupProvider.getUser(userIdentifier); + if (user == null) { + return false; + } + } + + return true; + } + private static void audit(final Authorizer authorizer, final AuthorizationRequest request, final AuthorizationResult result) { // audit when... // 1 - the authorizer supports auditing @@ -223,6 +242,9 @@ public final class AuthorizerFactory { if (tenantExists(baseConfigurableUserGroupProvider, group.getIdentifier(), group.getName())) { throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", group.getName())); } + if (!allGroupUsersExist(baseConfigurableUserGroupProvider, group)) { + throw new IllegalStateException(String.format("Cannot create group '%s' with users that don't exist.", group.getName())); + } return baseConfigurableUserGroupProvider.addGroup(group); } @@ -236,6 +258,9 @@ public final class AuthorizerFactory { if (tenantExists(baseConfigurableUserGroupProvider, group.getIdentifier(), group.getName())) { throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", group.getName())); } + if (!allGroupUsersExist(baseConfigurableUserGroupProvider, group)) { + throw new IllegalStateException(String.format("Cannot update group '%s' to add users that don't exist.", group.getName())); + } if (!baseConfigurableUserGroupProvider.isConfigurable(group)) { throw new IllegalArgumentException("The specified group does not support modification."); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileUserGroupProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileUserGroupProvider.java index edcfa51420..2add45e10e 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileUserGroupProvider.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileUserGroupProvider.java @@ -331,9 +331,6 @@ public class FileUserGroupProvider implements ConfigurableUserGroupProvider { final UserGroupHolder holder = userGroupHolder.get(); final Tenants tenants = holder.getTenants(); - // determine that all users in the group exist before doing anything, throw an exception if they don't - checkGroupUsers(group, tenants.getUsers().getUser()); - // create a new JAXB Group based on the incoming Group final org.apache.nifi.authorization.file.tenants.generated.Group jaxbGroup = new org.apache.nifi.authorization.file.tenants.generated.Group(); jaxbGroup.setIdentifier(group.getIdentifier()); @@ -601,25 +598,6 @@ public class FileUserGroupProvider implements ConfigurableUserGroupProvider { return jaxbUser; } - private Set checkGroupUsers(final Group group, final List users) { - final Set jaxbUsers = new HashSet<>(); - for (String groupUser : group.getUsers()) { - boolean found = false; - for (org.apache.nifi.authorization.file.tenants.generated.User jaxbUser : users) { - if (jaxbUser.getIdentifier().equals(groupUser)) { - jaxbUsers.add(jaxbUser); - found = true; - break; - } - } - - if (!found) { - throw new IllegalStateException("Unable to add group because user " + groupUser + " does not exist"); - } - } - return jaxbUsers; - } - /** * Loads the authorizations file and populates the AuthorizationsHolder, only called during start-up. * diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java index 79e70a0745..8140ef96da 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileAuthorizerTest.java @@ -1150,7 +1150,7 @@ public class FileAuthorizerTest { assertEquals(3, groups.size()); } - @Test(expected = IllegalStateException.class) + @Test public void testAddGroupWhenUserDoesNotExist() throws Exception { writeFile(primaryAuthorizations, EMPTY_AUTHORIZATIONS); writeFile(primaryTenants, EMPTY_TENANTS); @@ -1164,6 +1164,8 @@ public class FileAuthorizerTest { .build(); authorizer.addGroup(group); + + assertEquals(1, authorizer.getGroups().size()); } @Test diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileUserGroupProviderTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileUserGroupProviderTest.java index 8b91f78e59..a4b77644d5 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileUserGroupProviderTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/test/java/org/apache/nifi/authorization/FileUserGroupProviderTest.java @@ -551,7 +551,7 @@ public class FileUserGroupProviderTest { assertEquals(3, groups.size()); } - @Test(expected = IllegalStateException.class) + @Test public void testAddGroupWhenUserDoesNotExist() throws Exception { writeFile(primaryTenants, EMPTY_TENANTS); userGroupProvider.onConfigured(configurationContext); @@ -564,6 +564,7 @@ public class FileUserGroupProviderTest { .build(); userGroupProvider.addGroup(group); + assertEquals(1, userGroupProvider.getGroups().size()); } @Test diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProvider.java index badd37d527..21409266ac 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProvider.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProvider.java @@ -171,13 +171,41 @@ public class CompositeConfigurableUserGroupProvider extends CompositeUserGroupPr @Override public UserAndGroups getUserAndGroups(String identity) throws AuthorizationAccessException { - UserAndGroups userAndGroups = configurableUserGroupProvider.getUserAndGroups(identity); + final CompositeUserAndGroups combinedResult; - if (userAndGroups.getUser() == null) { - userAndGroups = super.getUserAndGroups(identity); + // First, lookup user and groups by identity and combine data from all providers + UserAndGroups configurableProviderResult = configurableUserGroupProvider.getUserAndGroups(identity); + UserAndGroups compositeProvidersResult = super.getUserAndGroups(identity); + + if (configurableProviderResult.getUser() != null && compositeProvidersResult.getUser() != null) { + throw new IllegalStateException("Multiple UserGroupProviders claim to provide user " + identity); + + } else if (configurableProviderResult.getUser() != null) { + combinedResult = new CompositeUserAndGroups(configurableProviderResult.getUser(), configurableProviderResult.getGroups()); + combinedResult.addAllGroups(compositeProvidersResult.getGroups()); + + } else if (compositeProvidersResult.getUser() != null) { + combinedResult = new CompositeUserAndGroups(compositeProvidersResult.getUser(), compositeProvidersResult.getGroups()); + combinedResult.addAllGroups(configurableProviderResult.getGroups()); + + } else { + return UserAndGroups.EMPTY; } - return userAndGroups; + // Second, lookup groups containing the user identifier + String userIdentifier = combinedResult.getUser().getIdentifier(); + for (final Group group : configurableUserGroupProvider.getGroups()) { + if (group.getUsers() != null && group.getUsers().contains(userIdentifier)) { + combinedResult.addGroup(group); + } + } + for (final Group group : super.getGroups()) { + if (group.getUsers() != null && group.getUsers().contains(userIdentifier)) { + combinedResult.addGroup(group); + } + } + + return combinedResult; } @Override diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserAndGroups.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserAndGroups.java new file mode 100644 index 0000000000..010de8b4c7 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserAndGroups.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.authorization; + +import java.util.HashSet; +import java.util.Set; + +public class CompositeUserAndGroups implements UserAndGroups { + + private User user; + private Set groups; + + public CompositeUserAndGroups() { + this.user = null; + this.groups = null; + } + + public CompositeUserAndGroups(User user, Set groups) { + this.user = user; + setGroups(groups); + } + + @Override + public User getUser() { + return user; + } + + public void setUser(User user) { + this.user = user; + } + + @Override + public Set getGroups() { + return groups; + } + + public void setGroups(Set groups) { + // copy the collection so that if we add to this collection it does not modify other references + if (groups != null) { + this.groups = new HashSet<>(groups); + } else { + this.groups = null; + } + } + + public void addAllGroups(Set groups) { + if (groups != null) { + if (this.groups == null) { + this.groups = new HashSet<>(); + } + this.groups.addAll(groups); + } + } + + public void addGroup(Group group) { + if (group != null) { + if (this.groups == null) { + this.groups = new HashSet<>(); + } + this.groups.add(group); + } + } + +} diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserGroupProvider.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserGroupProvider.java index e27f08e91c..5f351912b4 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserGroupProvider.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserGroupProvider.java @@ -20,6 +20,8 @@ import org.apache.commons.lang3.StringUtils; import org.apache.nifi.authorization.exception.AuthorizationAccessException; import org.apache.nifi.authorization.exception.AuthorizerCreationException; import org.apache.nifi.authorization.exception.AuthorizerDestructionException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.HashSet; @@ -30,6 +32,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; public class CompositeUserGroupProvider implements UserGroupProvider { + private static final Logger logger = LoggerFactory.getLogger(CompositeUserGroupProvider.class); static final String PROP_USER_GROUP_PROVIDER_PREFIX = "User Group Provider "; static final Pattern USER_GROUP_PROVIDER_PATTERN = Pattern.compile(PROP_USER_GROUP_PROVIDER_PREFIX + "\\S+"); @@ -142,33 +145,56 @@ public class CompositeUserGroupProvider implements UserGroupProvider { @Override public UserAndGroups getUserAndGroups(String identity) throws AuthorizationAccessException { - UserAndGroups userAndGroups = null; + // This method builds a UserAndGroups response by combining the data from all providers using a two-pass approach + + CompositeUserAndGroups compositeUserAndGroups = new CompositeUserAndGroups(); + + // First pass - call getUserAndGroups(identity) on all providers, aggregate the responses, check for multiple + // user identity matches, which should not happen as identities should by globally unique. + String providerClassForUser = ""; for (final UserGroupProvider userGroupProvider : userGroupProviders) { - userAndGroups = userGroupProvider.getUserAndGroups(identity); + UserAndGroups userAndGroups = userGroupProvider.getUserAndGroups(identity); if (userAndGroups.getUser() != null) { - break; + // is this the first match on the user? + if(compositeUserAndGroups.getUser() == null) { + compositeUserAndGroups.setUser(userAndGroups.getUser()); + providerClassForUser = userGroupProvider.getClass().getName(); + } else { + logger.warn("Multiple UserGroupProviders are claiming to provide user '{}': [{} and {}] ", + identity, + userAndGroups.getUser(), + providerClassForUser, userGroupProvider.getClass().getName()); + throw new IllegalStateException("Multiple UserGroupProviders are claiming to provide user " + identity); + } + } + + if (userAndGroups.getGroups() != null) { + compositeUserAndGroups.addAllGroups(userAndGroups.getGroups()); } } - if (userAndGroups == null) { - // per API, returning non null with null user/groups - return new UserAndGroups() { - @Override - public User getUser() { - return null; - } - - @Override - public Set getGroups() { - return null; - } - }; - } else { - // a delegated provider contained a matching user - return userAndGroups; + if (compositeUserAndGroups.getUser() == null) { + logger.debug("No user found for identity {}", identity); + return UserAndGroups.EMPTY; } + + // Second pass - Now that we've matched a user, call getGroups() on all providers, and + // check all groups to see if they contain the user identifier corresponding to the identity. + // This is necessary because a provider might only know about a group<->userIdentifier mapping + // without knowing the user identifier. + String userIdentifier = compositeUserAndGroups.getUser().getIdentifier(); + for (final UserGroupProvider userGroupProvider : userGroupProviders) { + for (final Group group : userGroupProvider.getGroups()) { + if (group.getUsers() != null && group.getUsers().contains(userIdentifier)) { + compositeUserAndGroups.addGroup(group); + } + } + } + + return compositeUserAndGroups; + } @Override diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProviderTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProviderTest.java index 53c4a1de62..cdef935977 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProviderTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProviderTest.java @@ -33,7 +33,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGroupProviderTest { +public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGroupProviderTestBase { public static final String USER_5_IDENTIFIER = "user-identifier-5"; public static final String USER_5_IDENTITY = "user-identity-5"; @@ -99,7 +99,7 @@ public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGro // users and groups assertEquals(3, userGroupProvider.getUsers().size()); - assertEquals(1, userGroupProvider.getGroups().size()); + assertEquals(2, userGroupProvider.getGroups().size()); // unknown assertNull(userGroupProvider.getUser(NOT_A_REAL_USER_IDENTIFIER)); @@ -111,8 +111,49 @@ public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGro assertNull(unknownUserAndGroups.getGroups()); // providers - testConfigurableUserGroupProvider(userGroupProvider); - testConflictingUserGroupProvider(userGroupProvider); + try { + testConfigurableUserGroupProvider(userGroupProvider); + assertTrue("Should never get here as we expect the line above to throw an exception", false); + } catch (Exception e) { + assertTrue(e instanceof IllegalStateException); + assertTrue(e.getMessage().contains(USER_1_IDENTITY)); + } + + try { + testConflictingUserGroupProvider(userGroupProvider); + assertTrue("Should never get here as we expect the line above to throw an exception", false); + } catch (Exception e) { + assertTrue(e instanceof IllegalStateException); + assertTrue(e.getMessage().contains(USER_1_IDENTITY)); + } + } + + @Test + public void testConfigurableUserGroupProviderWithCollaboratingUserGroupProvider() throws Exception { + final UserGroupProvider userGroupProvider = initCompositeUserGroupProvider(new CompositeConfigurableUserGroupProvider(), lookup -> { + when(lookup.getUserGroupProvider(eq(CONFIGURABLE_USER_GROUP_PROVIDER))).thenReturn(getConfigurableUserGroupProvider()); + }, configurationContext -> { + when(configurationContext.getProperty(PROP_CONFIGURABLE_USER_GROUP_PROVIDER)).thenReturn(new StandardPropertyValue(CONFIGURABLE_USER_GROUP_PROVIDER, null)); + }, getCollaboratingUserGroupProvider()); + + // users and groups + assertEquals(3, userGroupProvider.getUsers().size()); + assertEquals(2, userGroupProvider.getGroups().size()); + + // unknown + assertNull(userGroupProvider.getUser(NOT_A_REAL_USER_IDENTIFIER)); + assertNull(userGroupProvider.getUserByIdentity(NOT_A_REAL_USER_IDENTITY)); + + final UserAndGroups unknownUserAndGroups = userGroupProvider.getUserAndGroups(NOT_A_REAL_USER_IDENTITY); + assertNotNull(unknownUserAndGroups); + assertNull(unknownUserAndGroups.getUser()); + assertNull(unknownUserAndGroups.getGroups()); + + // providers + final UserAndGroups user1AndGroups = userGroupProvider.getUserAndGroups(USER_1_IDENTITY); + assertNotNull(user1AndGroups); + assertNotNull(user1AndGroups.getUser()); + assertEquals(2, user1AndGroups.getGroups().size()); // from CollaboratingUGP } private UserGroupProvider getConfigurableUserGroupProvider() { @@ -122,7 +163,7 @@ public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGro ).collect(Collectors.toSet()); final Set groups = Stream.of( - new Group.Builder().identifier(GROUP_2_IDENTIFIER).name(GROUP_2_NAME).addUser(USER_1_IDENTIFIER).build() + new Group.Builder().identifier(GROUP_1_IDENTIFIER).name(GROUP_1_NAME).addUser(USER_1_IDENTIFIER).build() ).collect(Collectors.toSet()); return new SimpleConfigurableUserGroupProvider(users, groups); @@ -145,7 +186,7 @@ public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGro assertNotNull(user5AndGroups.getUser()); assertTrue(user5AndGroups.getGroups().isEmpty()); - assertNotNull(userGroupProvider.getGroup(GROUP_2_IDENTIFIER)); - assertEquals(1, userGroupProvider.getGroup(GROUP_2_IDENTIFIER).getUsers().size()); + assertNotNull(userGroupProvider.getGroup(GROUP_1_IDENTIFIER)); + assertEquals(1, userGroupProvider.getGroup(GROUP_1_IDENTIFIER).getUsers().size()); } } \ No newline at end of file diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTest.java index ba4499d5f3..acc351bc7b 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTest.java @@ -18,16 +18,8 @@ package org.apache.nifi.authorization; import org.apache.nifi.attribute.expression.language.StandardPropertyValue; import org.apache.nifi.authorization.exception.AuthorizerCreationException; -import org.apache.nifi.components.PropertyValue; import org.junit.Test; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.function.Consumer; -import java.util.stream.Collectors; -import java.util.stream.Stream; - import static org.apache.nifi.authorization.CompositeUserGroupProvider.PROP_USER_GROUP_PROVIDER_PREFIX; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -37,28 +29,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class CompositeUserGroupProviderTest { - - public static final String USER_1_IDENTIFIER = "user-identifier-1"; - public static final String USER_1_IDENTITY = "user-identity-1"; - - public static final String USER_2_IDENTIFIER = "user-identifier-2"; - public static final String USER_2_IDENTITY = "user-identity-2"; - - public static final String USER_3_IDENTIFIER = "user-identifier-3"; - public static final String USER_3_IDENTITY = "user-identity-3"; - - public static final String USER_4_IDENTIFIER = "user-identifier-4"; - public static final String USER_4_IDENTITY = "user-identity-4"; - - public static final String GROUP_1_IDENTIFIER = "group-identifier-1"; - public static final String GROUP_1_NAME = "group-name-1"; - - public static final String GROUP_2_IDENTIFIER = "group-identifier-2"; - public static final String GROUP_2_NAME = "group-name-2"; - - public static final String NOT_A_REAL_USER_IDENTIFIER = "not-a-real-user-identifier"; - public static final String NOT_A_REAL_USER_IDENTITY = "not-a-real-user-identity"; +public class CompositeUserGroupProviderTest extends CompositeUserGroupProviderTestBase { @Test(expected = AuthorizerCreationException.class) public void testNoConfiguredProviders() throws Exception { @@ -154,148 +125,49 @@ public class CompositeUserGroupProviderTest { assertNull(unknownUserAndGroups.getGroups()); // providers - testUserGroupProviderOne(userGroupProvider); testUserGroupProviderTwo(userGroupProvider); - testConflictingUserGroupProvider(userGroupProvider); + + try { + testUserGroupProviderOne(userGroupProvider); + assertTrue("Should never get here as we expect the line above to throw an exception", false); + } catch (Exception e) { + assertTrue(e instanceof IllegalStateException); + assertTrue(e.getMessage().contains(USER_1_IDENTITY)); + } + + try { + testConflictingUserGroupProvider(userGroupProvider); + assertTrue("Should never get here as we expect the line above to throw an exception", false); + } catch (Exception e) { + assertTrue(e instanceof IllegalStateException); + assertTrue(e.getMessage().contains(USER_1_IDENTITY)); + } } - protected UserGroupProvider getUserGroupProviderOne() { - final Set users = Stream.of( - new User.Builder().identifier(USER_1_IDENTIFIER).identity(USER_1_IDENTITY).build(), - new User.Builder().identifier(USER_2_IDENTIFIER).identity(USER_2_IDENTITY).build() - ).collect(Collectors.toSet()); + @Test + public void testMultipleProvidersWithCollaboratingUserGroupProvider() throws Exception { + final UserGroupProvider userGroupProvider = initCompositeUserGroupProvider(new CompositeUserGroupProvider(), null, null, + getUserGroupProviderOne(), getUserGroupProviderTwo(), getCollaboratingUserGroupProvider()); - final Set groups = Stream.of( - new Group.Builder().identifier(GROUP_1_IDENTIFIER).name(GROUP_1_NAME).addUser(USER_1_IDENTIFIER).build() - ).collect(Collectors.toSet()); + // users and groups + assertEquals(4, userGroupProvider.getUsers().size()); + assertEquals(2, userGroupProvider.getGroups().size()); - return new SimpleUserGroupProvider(users, groups); - } + // unknown + assertNull(userGroupProvider.getUser(NOT_A_REAL_USER_IDENTIFIER)); + assertNull(userGroupProvider.getUserByIdentity(NOT_A_REAL_USER_IDENTITY)); - protected void testUserGroupProviderOne(final UserGroupProvider userGroupProvider) { - // users - assertNotNull(userGroupProvider.getUser(USER_1_IDENTIFIER)); - assertNotNull(userGroupProvider.getUserByIdentity(USER_1_IDENTITY)); + final UserAndGroups unknownUserAndGroups = userGroupProvider.getUserAndGroups(NOT_A_REAL_USER_IDENTITY); + assertNotNull(unknownUserAndGroups); + assertNull(unknownUserAndGroups.getUser()); + assertNull(unknownUserAndGroups.getGroups()); - assertNotNull(userGroupProvider.getUser(USER_2_IDENTIFIER)); - assertNotNull(userGroupProvider.getUserByIdentity(USER_2_IDENTITY)); + // providers + testUserGroupProviderTwo(userGroupProvider); final UserAndGroups user1AndGroups = userGroupProvider.getUserAndGroups(USER_1_IDENTITY); assertNotNull(user1AndGroups); assertNotNull(user1AndGroups.getUser()); - assertEquals(1, user1AndGroups.getGroups().size()); - - final UserAndGroups user2AndGroups = userGroupProvider.getUserAndGroups(USER_2_IDENTITY); - assertNotNull(user2AndGroups); - assertNotNull(user2AndGroups.getUser()); - assertTrue(user2AndGroups.getGroups().isEmpty()); - - // groups - assertNotNull(userGroupProvider.getGroup(GROUP_1_IDENTIFIER)); - assertEquals(1, userGroupProvider.getGroup(GROUP_1_IDENTIFIER).getUsers().size()); - } - - protected UserGroupProvider getUserGroupProviderTwo() { - final Set users = Stream.of( - new User.Builder().identifier(USER_3_IDENTIFIER).identity(USER_3_IDENTITY).build() - ).collect(Collectors.toSet()); - - final Set groups = Stream.of( - new Group.Builder().identifier(GROUP_2_IDENTIFIER).name(GROUP_2_NAME).addUser(USER_3_IDENTIFIER).build() - ).collect(Collectors.toSet()); - - return new SimpleUserGroupProvider(users, groups); - } - - protected void testUserGroupProviderTwo(final UserGroupProvider userGroupProvider) { - // users - assertNotNull(userGroupProvider.getUser(USER_3_IDENTIFIER)); - assertNotNull(userGroupProvider.getUserByIdentity(USER_3_IDENTITY)); - - final UserAndGroups user3AndGroups = userGroupProvider.getUserAndGroups(USER_3_IDENTITY); - assertNotNull(user3AndGroups); - assertNotNull(user3AndGroups.getUser()); - assertEquals(1, user3AndGroups.getGroups().size()); - - // groups - assertNotNull(userGroupProvider.getGroup(GROUP_2_IDENTIFIER)); - assertEquals(1, userGroupProvider.getGroup(GROUP_2_IDENTIFIER).getUsers().size()); - } - - protected UserGroupProvider getConflictingUserGroupProvider() { - final Set users = Stream.of( - new User.Builder().identifier(USER_1_IDENTIFIER).identity(USER_1_IDENTITY).build(), - new User.Builder().identifier(USER_4_IDENTIFIER).identity(USER_4_IDENTITY).build() - ).collect(Collectors.toSet()); - - final Set groups = Stream.of( - new Group.Builder().identifier(GROUP_2_IDENTIFIER).name(GROUP_2_NAME).addUser(USER_1_IDENTIFIER).addUser(USER_4_IDENTIFIER).build() - ).collect(Collectors.toSet()); - - return new SimpleUserGroupProvider(users, groups); - } - - protected void testConflictingUserGroupProvider(final UserGroupProvider userGroupProvider) { - assertNotNull(userGroupProvider.getUser(USER_4_IDENTIFIER)); - assertNotNull(userGroupProvider.getUserByIdentity(USER_4_IDENTITY)); - } - - private void mockProperties(final AuthorizerConfigurationContext configurationContext) { - when(configurationContext.getProperties()).then((invocation) -> { - final Map properties = new HashMap<>(); - - int i = 1; - while (true) { - final String key = PROP_USER_GROUP_PROVIDER_PREFIX + i++; - final PropertyValue value = configurationContext.getProperty(key); - if (value == null) { - break; - } else { - properties.put(key, value.getValue()); - } - } - - return properties; - }); - } - - protected UserGroupProvider initCompositeUserGroupProvider( - final CompositeUserGroupProvider compositeUserGroupProvider, final Consumer lookupConsumer, - final Consumer configurationContextConsumer, final UserGroupProvider... providers) { - - // initialization - final UserGroupProviderLookup lookup = mock(UserGroupProviderLookup.class); - - for (int i = 1; i <= providers.length; i++) { - when(lookup.getUserGroupProvider(eq(String.valueOf(i)))).thenReturn(providers[i - 1]); - } - - // allow callers to mock additional providers - if (lookupConsumer != null) { - lookupConsumer.accept(lookup); - } - - final UserGroupProviderInitializationContext initializationContext = mock(UserGroupProviderInitializationContext.class); - when(initializationContext.getUserGroupProviderLookup()).thenReturn(lookup); - - compositeUserGroupProvider.initialize(initializationContext); - - // configuration - final AuthorizerConfigurationContext configurationContext = mock(AuthorizerConfigurationContext.class); - - for (int i = 1; i <= providers.length; i++) { - when(configurationContext.getProperty(eq(PROP_USER_GROUP_PROVIDER_PREFIX + i))).thenReturn(new StandardPropertyValue(String.valueOf(i), null)); - } - - // allow callers to mock additional properties - if (configurationContextConsumer != null) { - configurationContextConsumer.accept(configurationContext); - } - - mockProperties(configurationContext); - - compositeUserGroupProvider.onConfigured(configurationContext); - - return compositeUserGroupProvider; + assertEquals(2, user1AndGroups.getGroups().size()); // from UGP1 and CollaboratingUGP } } \ No newline at end of file diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTestBase.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTestBase.java new file mode 100644 index 0000000000..9ff8552edf --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTestBase.java @@ -0,0 +1,237 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.authorization; + +import org.apache.nifi.attribute.expression.language.StandardPropertyValue; +import org.apache.nifi.components.PropertyValue; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static org.apache.nifi.authorization.CompositeUserGroupProvider.PROP_USER_GROUP_PROVIDER_PREFIX; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class CompositeUserGroupProviderTestBase { + + public static final String USER_1_IDENTIFIER = "user-identifier-1"; + public static final String USER_1_IDENTITY = "user-identity-1"; + + public static final String USER_2_IDENTIFIER = "user-identifier-2"; + public static final String USER_2_IDENTITY = "user-identity-2"; + + public static final String USER_3_IDENTIFIER = "user-identifier-3"; + public static final String USER_3_IDENTITY = "user-identity-3"; + + public static final String USER_4_IDENTIFIER = "user-identifier-4"; + public static final String USER_4_IDENTITY = "user-identity-4"; + + public static final String GROUP_1_IDENTIFIER = "group-identifier-1"; + public static final String GROUP_1_NAME = "group-name-1"; + + public static final String GROUP_2_IDENTIFIER = "group-identifier-2"; + public static final String GROUP_2_NAME = "group-name-2"; + + public static final String NOT_A_REAL_USER_IDENTIFIER = "not-a-real-user-identifier"; + public static final String NOT_A_REAL_USER_IDENTITY = "not-a-real-user-identity"; + + protected UserGroupProvider getUserGroupProviderOne() { + final Set users = Stream.of( + new User.Builder().identifier(USER_1_IDENTIFIER).identity(USER_1_IDENTITY).build(), + new User.Builder().identifier(USER_2_IDENTIFIER).identity(USER_2_IDENTITY).build() + ).collect(Collectors.toSet()); + + final Set groups = Stream.of( + new Group.Builder().identifier(GROUP_1_IDENTIFIER).name(GROUP_1_NAME).addUser(USER_1_IDENTIFIER).build() + ).collect(Collectors.toSet()); + + return new SimpleUserGroupProvider(users, groups); + } + + protected void testUserGroupProviderOne(final UserGroupProvider userGroupProvider) { + // users + assertNotNull(userGroupProvider.getUser(USER_1_IDENTIFIER)); + assertNotNull(userGroupProvider.getUserByIdentity(USER_1_IDENTITY)); + + assertNotNull(userGroupProvider.getUser(USER_2_IDENTIFIER)); + assertNotNull(userGroupProvider.getUserByIdentity(USER_2_IDENTITY)); + + // When used with ConflictingUserGroupProvider, we expect the line below to throw IllegalStateException + final UserAndGroups user1AndGroups = userGroupProvider.getUserAndGroups(USER_1_IDENTITY); + assertNotNull(user1AndGroups); + assertNotNull(user1AndGroups.getUser()); + assertEquals(1, user1AndGroups.getGroups().size()); + + final UserAndGroups user2AndGroups = userGroupProvider.getUserAndGroups(USER_2_IDENTITY); + assertNotNull(user2AndGroups); + assertNotNull(user2AndGroups.getUser()); + assertTrue(user2AndGroups.getGroups().isEmpty()); + + // groups + assertNotNull(userGroupProvider.getGroup(GROUP_1_IDENTIFIER)); + assertEquals(1, userGroupProvider.getGroup(GROUP_1_IDENTIFIER).getUsers().size()); + } + + protected UserGroupProvider getUserGroupProviderTwo() { + final Set users = Stream.of( + new User.Builder().identifier(USER_3_IDENTIFIER).identity(USER_3_IDENTITY).build() + ).collect(Collectors.toSet()); + + final Set groups = Stream.of( + new Group.Builder().identifier(GROUP_2_IDENTIFIER).name(GROUP_2_NAME).addUser(USER_3_IDENTIFIER).build() + ).collect(Collectors.toSet()); + + return new SimpleUserGroupProvider(users, groups); + } + + protected void testUserGroupProviderTwo(final UserGroupProvider userGroupProvider) { + // users + assertNotNull(userGroupProvider.getUser(USER_3_IDENTIFIER)); + assertNotNull(userGroupProvider.getUserByIdentity(USER_3_IDENTITY)); + + final UserAndGroups user3AndGroups = userGroupProvider.getUserAndGroups(USER_3_IDENTITY); + assertNotNull(user3AndGroups); + assertNotNull(user3AndGroups.getUser()); + assertEquals(1, user3AndGroups.getGroups().size()); + + // groups + assertNotNull(userGroupProvider.getGroup(GROUP_2_IDENTIFIER)); + assertEquals(1, userGroupProvider.getGroup(GROUP_2_IDENTIFIER).getUsers().size()); + } + + protected UserGroupProvider getConflictingUserGroupProvider() { + final Set users = Stream.of( + new User.Builder().identifier(USER_1_IDENTIFIER).identity(USER_1_IDENTITY).build(), + new User.Builder().identifier(USER_4_IDENTIFIER).identity(USER_4_IDENTITY).build() + ).collect(Collectors.toSet()); + + final Set groups = Stream.of( + new Group.Builder().identifier(GROUP_2_IDENTIFIER).name(GROUP_2_NAME).addUser(USER_1_IDENTIFIER).addUser(USER_4_IDENTIFIER).build() + ).collect(Collectors.toSet()); + + return new SimpleUserGroupProvider(users, groups); + } + + protected void testConflictingUserGroupProvider(final UserGroupProvider userGroupProvider) { + assertNotNull(userGroupProvider.getUser(USER_4_IDENTIFIER)); + assertNotNull(userGroupProvider.getUserByIdentity(USER_4_IDENTITY)); + + assertNotNull(userGroupProvider.getUser(USER_1_IDENTIFIER)); + assertNotNull(userGroupProvider.getUserByIdentity(USER_1_IDENTITY)); + + // When used with UserGroupProviderOne, we expect the line below to throw IllegalStateException + final UserAndGroups user1AndGroups = userGroupProvider.getUserAndGroups(USER_1_IDENTITY); + assertNotNull(user1AndGroups); + assertNotNull(user1AndGroups.getUser()); + assertEquals(1, user1AndGroups.getGroups().size()); + } + + protected UserGroupProvider getCollaboratingUserGroupProvider() { + final Set users = Stream.of( + new User.Builder().identifier(USER_4_IDENTIFIER).identity(USER_4_IDENTITY).build() + ).collect(Collectors.toSet()); + + final Set groups = Stream.of( + // USER_1_IDENTIFIER is from UserGroupProviderOne + new Group.Builder().identifier(GROUP_2_IDENTIFIER).name(GROUP_2_NAME).addUser(USER_1_IDENTIFIER).addUser(USER_4_IDENTIFIER).build() + ).collect(Collectors.toSet()); + + return new SimpleUserGroupProvider(users, groups); + } + + protected void testCollaboratingUserGroupProvider(final UserGroupProvider userGroupProvider) { + // users + assertNotNull(userGroupProvider.getUser(USER_4_IDENTIFIER)); + assertNotNull(userGroupProvider.getUserByIdentity(USER_4_IDENTITY)); + + final UserAndGroups user4AndGroups = userGroupProvider.getUserAndGroups(USER_4_IDENTITY); + assertNotNull(user4AndGroups); + assertNotNull(user4AndGroups.getUser()); + assertEquals(1, user4AndGroups.getGroups().size()); + + // groups + assertNotNull(userGroupProvider.getGroup(GROUP_2_IDENTIFIER)); + assertEquals(2, userGroupProvider.getGroup(GROUP_2_IDENTIFIER).getUsers().size()); + } + + protected void mockProperties(final AuthorizerConfigurationContext configurationContext) { + when(configurationContext.getProperties()).then((invocation) -> { + final Map properties = new HashMap<>(); + + int i = 1; + while (true) { + final String key = PROP_USER_GROUP_PROVIDER_PREFIX + i++; + final PropertyValue value = configurationContext.getProperty(key); + if (value == null) { + break; + } else { + properties.put(key, value.getValue()); + } + } + + return properties; + }); + } + + protected UserGroupProvider initCompositeUserGroupProvider( + final CompositeUserGroupProvider compositeUserGroupProvider, final Consumer lookupConsumer, + final Consumer configurationContextConsumer, final UserGroupProvider... providers) { + + // initialization + final UserGroupProviderLookup lookup = mock(UserGroupProviderLookup.class); + + for (int i = 1; i <= providers.length; i++) { + when(lookup.getUserGroupProvider(eq(String.valueOf(i)))).thenReturn(providers[i - 1]); + } + + // allow callers to mock additional providers + if (lookupConsumer != null) { + lookupConsumer.accept(lookup); + } + + final UserGroupProviderInitializationContext initializationContext = mock(UserGroupProviderInitializationContext.class); + when(initializationContext.getUserGroupProviderLookup()).thenReturn(lookup); + + compositeUserGroupProvider.initialize(initializationContext); + + // configuration + final AuthorizerConfigurationContext configurationContext = mock(AuthorizerConfigurationContext.class); + + for (int i = 1; i <= providers.length; i++) { + when(configurationContext.getProperty(eq(PROP_USER_GROUP_PROVIDER_PREFIX + i))).thenReturn(new StandardPropertyValue(String.valueOf(i), null)); + } + + // allow callers to mock additional properties + if (configurationContextConsumer != null) { + configurationContextConsumer.accept(configurationContext); + } + + mockProperties(configurationContext); + + compositeUserGroupProvider.onConfigured(configurationContext); + + return compositeUserGroupProvider; + } +} \ No newline at end of file