From 5d851e6a136ce28cc4ccd262b4d15a2f37856255 Mon Sep 17 00:00:00 2001 From: Bryan Bende Date: Mon, 27 Jan 2020 11:13:16 -0500 Subject: [PATCH] NIFI-7067 Allow a user and group with the same name/identity to exist This closes #4019 --- .../nifi/authorization/AuthorizerFactory.java | 36 ++++++++++++------- .../authorization/AuthorizerFactoryBean.java | 21 ++++++++++- .../authorization/AuthorizerFactoryTest.java | 15 ++------ 3 files changed, 46 insertions(+), 26 deletions(-) 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 812ea17fe1..b826cf161f 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,14 +47,14 @@ public final class AuthorizerFactory { } /** - * Checks if another tenant (user or group) exists with the same identity. + * Checks if another user exists with the same identity. * - * @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 + * @param userGroupProvider the userGroupProvider to use to lookup the user + * @param identifier identity of the user + * @param identity identity of the user + * @return true if another user exists with the same identity, false otherwise */ - private static boolean tenantExists(final UserGroupProvider userGroupProvider, final String identifier, final String identity) { + private static boolean userExists(final UserGroupProvider userGroupProvider, final String identifier, final String identity) { for (User user : userGroupProvider.getUsers()) { if (!user.getIdentifier().equals(identifier) && user.getIdentity().equals(identity)) { @@ -62,6 +62,18 @@ public final class AuthorizerFactory { } } + return false; + } + + /** + * Checks if another group exists with the same identity. + * + * @param userGroupProvider the userGroupProvider to use to lookup the group + * @param identifier identity of the group + * @param identity identity of the group + * @return true if another group exists with the same identity, false otherwise + */ + private static boolean groupExists(final UserGroupProvider userGroupProvider, final String identifier, final String identity) { for (Group group : userGroupProvider.getGroups()) { if (!group.getIdentifier().equals(identifier) && group.getName().equals(identity)) { @@ -209,7 +221,7 @@ public final class AuthorizerFactory { @Override public User addUser(User user) throws AuthorizationAccessException { - if (tenantExists(baseConfigurableUserGroupProvider, user.getIdentifier(), user.getIdentity())) { + if (userExists(baseConfigurableUserGroupProvider, user.getIdentifier(), user.getIdentity())) { throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", user.getIdentity())); } return baseConfigurableUserGroupProvider.addUser(user); @@ -222,7 +234,7 @@ public final class AuthorizerFactory { @Override public User updateUser(User user) throws AuthorizationAccessException { - if (tenantExists(baseConfigurableUserGroupProvider, user.getIdentifier(), user.getIdentity())) { + if (userExists(baseConfigurableUserGroupProvider, user.getIdentifier(), user.getIdentity())) { throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", user.getIdentity())); } if (!baseConfigurableUserGroupProvider.isConfigurable(user)) { @@ -241,7 +253,7 @@ public final class AuthorizerFactory { @Override public Group addGroup(Group group) throws AuthorizationAccessException { - if (tenantExists(baseConfigurableUserGroupProvider, group.getIdentifier(), group.getName())) { + if (groupExists(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)) { @@ -257,7 +269,7 @@ public final class AuthorizerFactory { @Override public Group updateGroup(Group group) throws AuthorizationAccessException { - if (tenantExists(baseConfigurableUserGroupProvider, group.getIdentifier(), group.getName())) { + if (groupExists(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)) { @@ -378,14 +390,14 @@ public final class AuthorizerFactory { // ensure that only one group exists per identity for (User user : userGroupProvider.getUsers()) { - if (tenantExists(userGroupProvider, user.getIdentifier(), user.getIdentity())) { + if (userExists(userGroupProvider, user.getIdentifier(), user.getIdentity())) { throw new AuthorizerCreationException(String.format("Found multiple users/user groups with identity '%s'.", user.getIdentity())); } } // ensure that only one group exists per identity for (Group group : userGroupProvider.getGroups()) { - if (tenantExists(userGroupProvider, group.getIdentifier(), group.getName())) { + if (groupExists(userGroupProvider, group.getIdentifier(), group.getName())) { throw new AuthorizerCreationException(String.format("Found multiple users/user groups with name '%s'.", group.getName())); } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java index 9d18e034d5..ec3ab9649b 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java @@ -161,8 +161,11 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG authorizers.put(authorizer.getIdentifier(), createAuthorizer(authorizer.getIdentifier(), authorizer.getClazz(), authorizer.getClasspath())); } - // configure each authorizer + // configure each authorizer, except the authorizer that is selected in nifi.properties for (final org.apache.nifi.authorization.generated.Authorizer provider : authorizerConfiguration.getAuthorizer()) { + if (provider.getIdentifier().equals(authorizerIdentifier)) { + continue; + } final Authorizer instance = authorizers.get(provider.getIdentifier()); instance.onConfigured(loadAuthorizerConfiguration(provider.getIdentifier(), provider.getProperty())); } @@ -174,7 +177,23 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG if (authorizer == null) { throw new Exception(String.format("The specified authorizer '%s' could not be found.", authorizerIdentifier)); } else { + // install integrity checks authorizer = AuthorizerFactory.installIntegrityChecks(authorizer); + + // configure authorizer after integrity checks are installed + AuthorizerConfigurationContext authorizerConfigurationContext = null; + for (final org.apache.nifi.authorization.generated.Authorizer provider : authorizerConfiguration.getAuthorizer()) { + if (provider.getIdentifier().equals(authorizerIdentifier)) { + authorizerConfigurationContext = loadAuthorizerConfiguration(provider.getIdentifier(), provider.getProperty()); + break; + } + } + + if (authorizerConfigurationContext == null) { + throw new IllegalStateException("Unable to load configuration for authorizer with id: " + authorizerIdentifier); + } + + authorizer.onConfigured(authorizerConfigurationContext); } } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java index 167a7d4de2..6dbe4bbdc7 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/java/org/apache/nifi/authorization/AuthorizerFactoryTest.java @@ -185,13 +185,7 @@ public class AuthorizerFactoryTest { userGroupProvider.addGroup(group1); User user = new User.Builder().identifier("user-id-2").identity("abc").build(); - - try { - userGroupProvider.addUser(user); - Assert.fail("Should have thrown exception"); - } catch (IllegalStateException e) { - - } + userGroupProvider.addUser(user); } @Test @@ -208,12 +202,7 @@ public class AuthorizerFactoryTest { userGroupProvider.addUser(user); Group group1 = new Group.Builder().identifier("group-id-1").name("abc").build(); - try { - userGroupProvider.addGroup(group1); - Assert.fail("Should have thrown exception"); - } catch (IllegalStateException e) { - - } + userGroupProvider.addGroup(group1); } @Test