NIFI-7067 Allow a user and group with the same name/identity to exist

This closes #4019
This commit is contained in:
Bryan Bende 2020-01-27 11:13:16 -05:00 committed by Matt Gilman
parent 98f9b7c033
commit 5d851e6a13
No known key found for this signature in database
GPG Key ID: DF61EC19432AEE37
3 changed files with 46 additions and 26 deletions

View File

@ -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 userGroupProvider the userGroupProvider to use to lookup the user
* @param identifier identity of the tenant * @param identifier identity of the user
* @param identity identity of the tenant * @param identity identity of the user
* @return true if another tenant exists with the same identity, false otherwise * @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()) { for (User user : userGroupProvider.getUsers()) {
if (!user.getIdentifier().equals(identifier) if (!user.getIdentifier().equals(identifier)
&& user.getIdentity().equals(identity)) { && 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()) { for (Group group : userGroupProvider.getGroups()) {
if (!group.getIdentifier().equals(identifier) if (!group.getIdentifier().equals(identifier)
&& group.getName().equals(identity)) { && group.getName().equals(identity)) {
@ -209,7 +221,7 @@ public final class AuthorizerFactory {
@Override @Override
public User addUser(User user) throws AuthorizationAccessException { 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())); throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", user.getIdentity()));
} }
return baseConfigurableUserGroupProvider.addUser(user); return baseConfigurableUserGroupProvider.addUser(user);
@ -222,7 +234,7 @@ public final class AuthorizerFactory {
@Override @Override
public User updateUser(User user) throws AuthorizationAccessException { 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())); throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", user.getIdentity()));
} }
if (!baseConfigurableUserGroupProvider.isConfigurable(user)) { if (!baseConfigurableUserGroupProvider.isConfigurable(user)) {
@ -241,7 +253,7 @@ public final class AuthorizerFactory {
@Override @Override
public Group addGroup(Group group) throws AuthorizationAccessException { 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())); throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", group.getName()));
} }
if (!allGroupUsersExist(baseConfigurableUserGroupProvider, group)) { if (!allGroupUsersExist(baseConfigurableUserGroupProvider, group)) {
@ -257,7 +269,7 @@ public final class AuthorizerFactory {
@Override @Override
public Group updateGroup(Group group) throws AuthorizationAccessException { 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())); throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", group.getName()));
} }
if (!allGroupUsersExist(baseConfigurableUserGroupProvider, group)) { if (!allGroupUsersExist(baseConfigurableUserGroupProvider, group)) {
@ -378,14 +390,14 @@ public final class AuthorizerFactory {
// ensure that only one group exists per identity // ensure that only one group exists per identity
for (User user : userGroupProvider.getUsers()) { 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())); throw new AuthorizerCreationException(String.format("Found multiple users/user groups with identity '%s'.", user.getIdentity()));
} }
} }
// ensure that only one group exists per identity // ensure that only one group exists per identity
for (Group group : userGroupProvider.getGroups()) { 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())); throw new AuthorizerCreationException(String.format("Found multiple users/user groups with name '%s'.", group.getName()));
} }
} }

View File

@ -161,8 +161,11 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG
authorizers.put(authorizer.getIdentifier(), createAuthorizer(authorizer.getIdentifier(), authorizer.getClazz(), authorizer.getClasspath())); 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()) { for (final org.apache.nifi.authorization.generated.Authorizer provider : authorizerConfiguration.getAuthorizer()) {
if (provider.getIdentifier().equals(authorizerIdentifier)) {
continue;
}
final Authorizer instance = authorizers.get(provider.getIdentifier()); final Authorizer instance = authorizers.get(provider.getIdentifier());
instance.onConfigured(loadAuthorizerConfiguration(provider.getIdentifier(), provider.getProperty())); instance.onConfigured(loadAuthorizerConfiguration(provider.getIdentifier(), provider.getProperty()));
} }
@ -174,7 +177,23 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG
if (authorizer == null) { if (authorizer == null) {
throw new Exception(String.format("The specified authorizer '%s' could not be found.", authorizerIdentifier)); throw new Exception(String.format("The specified authorizer '%s' could not be found.", authorizerIdentifier));
} else { } else {
// install integrity checks
authorizer = AuthorizerFactory.installIntegrityChecks(authorizer); 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);
} }
} }
} }

View File

@ -185,13 +185,7 @@ public class AuthorizerFactoryTest {
userGroupProvider.addGroup(group1); userGroupProvider.addGroup(group1);
User user = new User.Builder().identifier("user-id-2").identity("abc").build(); User user = new User.Builder().identifier("user-id-2").identity("abc").build();
userGroupProvider.addUser(user);
try {
userGroupProvider.addUser(user);
Assert.fail("Should have thrown exception");
} catch (IllegalStateException e) {
}
} }
@Test @Test
@ -208,12 +202,7 @@ public class AuthorizerFactoryTest {
userGroupProvider.addUser(user); userGroupProvider.addUser(user);
Group group1 = new Group.Builder().identifier("group-id-1").name("abc").build(); Group group1 = new Group.Builder().identifier("group-id-1").name("abc").build();
try { userGroupProvider.addGroup(group1);
userGroupProvider.addGroup(group1);
Assert.fail("Should have thrown exception");
} catch (IllegalStateException e) {
}
} }
@Test @Test