diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java index 8fbe0fbe8a..98b8102cc0 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AbstractPolicyBasedAuthorizer.java @@ -69,8 +69,21 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { // ensure that only one policy per resource-action exists for (AccessPolicy accessPolicy : getAccessPolicies()) { if (policyExists(accessPolicy)) { - throw new AuthorizerCreationException("Found multiple policies for " + accessPolicy.getResource() - + " with " + accessPolicy.getAction()); + throw new AuthorizerCreationException(String.format("Found multiple policies for '%s' with '%s'.", accessPolicy.getResource(), accessPolicy.getAction())); + } + } + + // ensure that only one group exists per identity + for (User user : getUsers()) { + if (tenantExists(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 : getGroups()) { + if (tenantExists(group.getIdentifier(), group.getName())) { + throw new AuthorizerCreationException(String.format("Found multiple users/user groups with name '%s'.", group.getName())); } } } @@ -100,6 +113,31 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { return false; } + /** + * Checks if another user 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 + */ + private boolean tenantExists(final String identifier, final String identity) { + for (User user : getUsers()) { + if (!user.getIdentifier().equals(identifier) + && user.getIdentity().equals(identity)) { + return true; + } + } + + for (Group group : getGroups()) { + if (!group.getIdentifier().equals(identifier) + && group.getName().equals(identity)) { + return true; + } + } + + return false; + } + @Override public final AuthorizationResult authorize(AuthorizationRequest request) throws AuthorizationAccessException { final UsersAndAccessPolicies usersAndAccessPolicies = getUsersAndAccessPolicies(); @@ -112,7 +150,7 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { final User user = usersAndAccessPolicies.getUser(request.getIdentity()); if (user == null) { - return AuthorizationResult.denied("Unknown user with identity " + request.getIdentity()); + return AuthorizationResult.denied(String.format("Unknown user with identity '%s'.", request.getIdentity())); } final Set userGroups = usersAndAccessPolicies.getGroups(user.getIdentity()); @@ -151,8 +189,23 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { * @param group the Group to add * @return the added Group * @throws AuthorizationAccessException if there was an unexpected error performing the operation + * @throws IllegalStateException if a group with the same name already exists */ - public abstract Group addGroup(Group group) throws AuthorizationAccessException; + public final synchronized Group addGroup(Group group) throws AuthorizationAccessException { + if (tenantExists(group.getIdentifier(), group.getName())) { + throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", group.getName())); + } + return doAddGroup(group); + } + + /** + * Adds a new group. + * + * @param group the Group to add + * @return the added Group + * @throws AuthorizationAccessException if there was an unexpected error performing the operation + */ + public abstract Group doAddGroup(Group group) throws AuthorizationAccessException; /** * Retrieves a Group by id. @@ -169,8 +222,23 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { * @param group an updated group instance * @return the updated group instance, or null if no matching group was found * @throws AuthorizationAccessException if there was an unexpected error performing the operation + * @throws IllegalStateException if there is already a group with the same name */ - public abstract Group updateGroup(Group group) throws AuthorizationAccessException; + public final synchronized Group updateGroup(Group group) throws AuthorizationAccessException { + if (tenantExists(group.getIdentifier(), group.getName())) { + throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", group.getName())); + } + return doUpdateGroup(group); + } + + /** + * The group represented by the provided instance will be updated based on the provided instance. + * + * @param group an updated group instance + * @return the updated group instance, or null if no matching group was found + * @throws AuthorizationAccessException if there was an unexpected error performing the operation + */ + public abstract Group doUpdateGroup(Group group) throws AuthorizationAccessException; /** * Deletes the given group. @@ -196,8 +264,23 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { * @param user the user to add * @return the user that was added * @throws AuthorizationAccessException if there was an unexpected error performing the operation + * @throws IllegalStateException if there is already a user with the same identity */ - public abstract User addUser(User user) throws AuthorizationAccessException; + public final synchronized User addUser(User user) throws AuthorizationAccessException { + if (tenantExists(user.getIdentifier(), user.getIdentity())) { + throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", user.getIdentity())); + } + return doAddUser(user); + } + + /** + * Adds the given user. + * + * @param user the user to add + * @return the user that was added + * @throws AuthorizationAccessException if there was an unexpected error performing the operation + */ + public abstract User doAddUser(User user) throws AuthorizationAccessException; /** * Retrieves the user with the given identifier. @@ -223,8 +306,23 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { * @param user an updated user instance * @return the updated user instance, or null if no matching user was found * @throws AuthorizationAccessException if there was an unexpected error performing the operation + * @throws IllegalStateException if there is already a user with the same identity */ - public abstract User updateUser(User user) throws AuthorizationAccessException; + public final synchronized User updateUser(final User user) throws AuthorizationAccessException { + if (tenantExists(user.getIdentifier(), user.getIdentity())) { + throw new IllegalStateException(String.format("User/user group already exists with the identity '%s'.", user.getIdentity())); + } + return doUpdateUser(user); + } + + /** + * The user represented by the provided instance will be updated based on the provided instance. + * + * @param user an updated user instance + * @return the updated user instance, or null if no matching user was found + * @throws AuthorizationAccessException if there was an unexpected error performing the operation + */ + public abstract User doUpdateUser(User user) throws AuthorizationAccessException; /** * Deletes the given user. @@ -252,8 +350,7 @@ public abstract class AbstractPolicyBasedAuthorizer implements Authorizer { */ public final synchronized AccessPolicy addAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { if (policyExists(accessPolicy)) { - throw new IllegalStateException("Found multiple policies for " + accessPolicy.getResource() - + " with " + accessPolicy.getAction()); + throw new IllegalStateException(String.format("Found multiple policies for '%s' with '%s'.", accessPolicy.getResource(), accessPolicy.getAction())); } return doAddAccessPolicy(accessPolicy); } diff --git a/nifi-framework-api/src/test/java/org/apache/nifi/authorization/MockPolicyBasedAuthorizer.java b/nifi-framework-api/src/test/java/org/apache/nifi/authorization/MockPolicyBasedAuthorizer.java new file mode 100644 index 0000000000..9b50b50ed4 --- /dev/null +++ b/nifi-framework-api/src/test/java/org/apache/nifi/authorization/MockPolicyBasedAuthorizer.java @@ -0,0 +1,183 @@ +/* + * 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.authorization.exception.AuthorizationAccessException; +import org.apache.nifi.authorization.exception.AuthorizerCreationException; +import org.apache.nifi.authorization.exception.AuthorizerDestructionException; + +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Mock implementation of AbstractPolicyBasedAuthorizer. + */ +public class MockPolicyBasedAuthorizer extends AbstractPolicyBasedAuthorizer { + + private Set groups = new HashSet<>(); + private Set users = new HashSet<>(); + private Set policies = new HashSet<>(); + + public MockPolicyBasedAuthorizer() { + + } + + public MockPolicyBasedAuthorizer(Set groups, Set users, Set policies) { + if (groups != null) { + this.groups.addAll(groups); + } + if (users != null) { + this.users.addAll(users); + } + if (policies != null) { + this.policies.addAll(policies); + } + } + + @Override + public Group doAddGroup(Group group) throws AuthorizationAccessException { + groups.add(group); + return group; + } + + @Override + public Group getGroup(String identifier) throws AuthorizationAccessException { + return groups.stream().filter(g -> g.getIdentifier().equals(identifier)).findFirst().get(); + } + + @Override + public Group doUpdateGroup(Group group) throws AuthorizationAccessException { + deleteGroup(group); + return addGroup(group); + } + + @Override + public Group deleteGroup(Group group) throws AuthorizationAccessException { + groups.remove(group); + return group; + } + + @Override + public Set getGroups() throws AuthorizationAccessException { + return groups; + } + + @Override + public User doAddUser(User user) throws AuthorizationAccessException { + users.add(user); + return user; + } + + @Override + public User getUser(String identifier) throws AuthorizationAccessException { + return users.stream().filter(u -> u.getIdentifier().equals(identifier)).findFirst().get(); + } + + @Override + public User getUserByIdentity(String identity) throws AuthorizationAccessException { + return users.stream().filter(u -> u.getIdentity().equals(identity)).findFirst().get(); + } + + @Override + public User doUpdateUser(User user) throws AuthorizationAccessException { + deleteUser(user); + return addUser(user); + } + + @Override + public User deleteUser(User user) throws AuthorizationAccessException { + users.remove(user); + return user; + } + + @Override + public Set getUsers() throws AuthorizationAccessException { + return users; + } + + @Override + protected AccessPolicy doAddAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { + policies.add(accessPolicy); + return accessPolicy; + } + + @Override + public AccessPolicy getAccessPolicy(String identifier) throws AuthorizationAccessException { + return policies.stream().filter(p -> p.getIdentifier().equals(identifier)).findFirst().get(); + } + + @Override + public AccessPolicy updateAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { + deleteAccessPolicy(accessPolicy); + return addAccessPolicy(accessPolicy); + } + + @Override + public AccessPolicy deleteAccessPolicy(AccessPolicy policy) throws AuthorizationAccessException { + policies.remove(policy); + return policy; + } + + @Override + public Set getAccessPolicies() throws AuthorizationAccessException { + return policies; + } + + @Override + public UsersAndAccessPolicies getUsersAndAccessPolicies() throws AuthorizationAccessException { + return new UsersAndAccessPolicies() { + @Override + public AccessPolicy getAccessPolicy(String resourceIdentifier, RequestAction action) { + return null; + } + + @Override + public User getUser(String identity) { + return getUserByIdentity(identity); + } + + @Override + public Set getGroups(String userIdentity) { + User user = getUserByIdentity(userIdentity); + if (user == null) { + return new HashSet<>(); + } else { + return groups.stream() + .filter(g -> g.getUsers().contains(user.getIdentifier())) + .collect(Collectors.toSet()); + } + } + }; + } + + @Override + public void initialize(AuthorizerInitializationContext initializationContext) throws AuthorizerCreationException { + + } + + @Override + public void doOnConfigured(AuthorizerConfigurationContext configurationContext) throws AuthorizerCreationException { + + } + + @Override + public void preDestruction() throws AuthorizerDestructionException { + + } + +} diff --git a/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java b/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java index b173d3decf..a5cf35121f 100644 --- a/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java +++ b/nifi-framework-api/src/test/java/org/apache/nifi/authorization/TestAbstractPolicyBasedAuthorizer.java @@ -16,9 +16,8 @@ */ package org.apache.nifi.authorization; -import org.apache.nifi.authorization.exception.AuthorizationAccessException; +import org.apache.nifi.authorization.MockPolicyBasedAuthorizer; import org.apache.nifi.authorization.exception.AuthorizerCreationException; -import org.apache.nifi.authorization.exception.AuthorizerDestructionException; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; @@ -251,7 +250,7 @@ public class TestAbstractPolicyBasedAuthorizer { assertEquals(authorizer1.getFingerprint(), authorizer2.getFingerprint()); - System.out.println(authorizer1.getFingerprint()); + //System.out.println(authorizer1.getFingerprint()); } @Test @@ -304,7 +303,7 @@ public class TestAbstractPolicyBasedAuthorizer { // make a second authorizer using the memory-backed implementation so we can inherit the fingerprint // and then compute a new fingerprint to compare them - AbstractPolicyBasedAuthorizer authorizer2 = new MemoryPolicyBasedAuthorizer(); + AbstractPolicyBasedAuthorizer authorizer2 = new MockPolicyBasedAuthorizer(); authorizer2.inheritFingerprint(fingerprint1); // computer the fingerprint of the second authorizer and it should be the same as the first @@ -329,117 +328,209 @@ public class TestAbstractPolicyBasedAuthorizer { Assert.assertTrue(fingerprint.length() > 0); } - /** - * An AbstractPolicyBasedAuthorizer that stores everything in memory. - */ - private static final class MemoryPolicyBasedAuthorizer extends AbstractPolicyBasedAuthorizer { + @Test(expected = AuthorizerCreationException.class) + public void testOnConfiguredWhenPoliciesWithSameResourceAndAction() { + User user1 = new User.Builder().identifier("user-id-1").identity("user-1").build(); - private Set groups = new HashSet<>(); - private Set users = new HashSet<>(); - private Set policies = new HashSet<>(); + AccessPolicy policy1 = new AccessPolicy.Builder() + .identifier("policy-id-1") + .resource("resource1") + .action(RequestAction.READ) + .addUser(user1.getIdentifier()) + .build(); - @Override - public Group addGroup(Group group) throws AuthorizationAccessException { - groups.add(group); - return group; - } + AccessPolicy policy2 = new AccessPolicy.Builder() + .identifier("policy-id-2") + .resource("resource1") + .action(RequestAction.READ) + .addUser(user1.getIdentifier()) + .build(); - @Override - public Group getGroup(String identifier) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + Set policies = new LinkedHashSet<>(); + policies.add(policy1); + policies.add(policy2); - @Override - public Group updateGroup(Group group) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + Set users = new LinkedHashSet<>(); + users.add(user1); - @Override - public Group deleteGroup(Group group) throws AuthorizationAccessException { - groups.remove(group); - return group; - } + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(new HashSet<>(), users, policies); + authorizer.onConfigured(context); + } - @Override - public Set getGroups() throws AuthorizationAccessException { - return groups; - } + @Test(expected = AuthorizerCreationException.class) + public void testOnConfiguredWhenUsersWithSameIdentity() { + User user1 = new User.Builder().identifier("user-id-1").identity("user-1").build(); + User user2 = new User.Builder().identifier("user-id-2").identity("user-1").build(); - @Override - public User addUser(User user) throws AuthorizationAccessException { - users.add(user); - return user; - } + Set users = new LinkedHashSet<>(); + users.add(user1); + users.add(user2); - @Override - public User getUser(String identifier) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(new HashSet<>(), users, new HashSet<>()); + authorizer.onConfigured(context); + } - @Override - public User getUserByIdentity(String identity) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + @Test(expected = AuthorizerCreationException.class) + public void testOnConfiguredWhenGroupsWithSameName() { + Group group1 = new Group.Builder().identifier("group-id-1").name("group-1").build(); + Group group2 = new Group.Builder().identifier("group-id-2").name("group-1").build(); - @Override - public User updateUser(User user) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + Set groups = new LinkedHashSet<>(); + groups.add(group1); + groups.add(group2); - @Override - public User deleteUser(User user) throws AuthorizationAccessException { - users.remove(user); - return user; - } + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(groups, new HashSet<>(), new HashSet<>()); + authorizer.onConfigured(context); + } - @Override - public Set getUsers() throws AuthorizationAccessException { - return users; - } + @Test + public void testAddPoliciesWithSameResourceAndAction() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); - @Override - protected AccessPolicy doAddAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { - policies.add(accessPolicy); - return accessPolicy; - } + User user1 = new User.Builder().identifier("user-id-1").identity("user-1").build(); + authorizer.addUser(user1); - @Override - public AccessPolicy getAccessPolicy(String identifier) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + AccessPolicy policy1 = new AccessPolicy.Builder() + .identifier("policy-id-1") + .resource("resource1") + .action(RequestAction.READ) + .addUser(user1.getIdentifier()) + .build(); + authorizer.addAccessPolicy(policy1); - @Override - public AccessPolicy updateAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } + AccessPolicy policy2 = new AccessPolicy.Builder() + .identifier("policy-id-2") + .resource("resource1") + .action(RequestAction.READ) + .addUser(user1.getIdentifier()) + .build(); - @Override - public AccessPolicy deleteAccessPolicy(AccessPolicy policy) throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } - - @Override - public Set getAccessPolicies() throws AuthorizationAccessException { - return policies; - } - - @Override - public UsersAndAccessPolicies getUsersAndAccessPolicies() throws AuthorizationAccessException { - throw new UnsupportedOperationException(); - } - - @Override - public void initialize(AuthorizerInitializationContext initializationContext) throws AuthorizerCreationException { + try { + authorizer.addAccessPolicy(policy2); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { } + } - @Override - public void doOnConfigured(AuthorizerConfigurationContext configurationContext) throws AuthorizerCreationException { + @Test + public void testAddUsersWithSameIdentity() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); + + User user1 = new User.Builder().identifier("user-id-1").identity("user-1").build(); + authorizer.addUser(user1); + + User user2 = new User.Builder().identifier("user-id-2").identity("user-1").build(); + + try { + authorizer.addUser(user2); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { } + } - @Override - public void preDestruction() throws AuthorizerDestructionException { + @Test + public void testAddGroupsWithSameName() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); + + Group group1 = new Group.Builder().identifier("group-id-1").name("group-1").build(); + authorizer.addGroup(group1); + + Group group2 = new Group.Builder().identifier("group-id-2").name("group-1").build(); + + try { + authorizer.addGroup(group2); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { + + } + } + + @Test + public void testAddUsersWithSameIdentityAsGroupName() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); + + Group group1 = new Group.Builder().identifier("group-id-1").name("abc").build(); + authorizer.addGroup(group1); + + User user = new User.Builder().identifier("user-id-2").identity("abc").build(); + + try { + authorizer.addUser(user); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { + + } + } + + @Test + public void testAddGroupWithSameNameAsUserIdentity() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); + + User user = new User.Builder().identifier("user-id-2").identity("abc").build(); + authorizer.addUser(user); + + Group group1 = new Group.Builder().identifier("group-id-1").name("abc").build(); + try { + authorizer.addGroup(group1); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { + + } + } + + @Test + public void testUpdateUserWithSameIdentity() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); + + User user1 = new User.Builder().identifier("user-id-1").identity("abc").build(); + authorizer.addUser(user1); + + User user2 = new User.Builder().identifier("user-id-2").identity("xyz").build(); + authorizer.addUser(user2); + + try { + User user1Updated = new User.Builder().identifier("user-id-1").identity("xyz").build(); + authorizer.updateUser(user1Updated); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { + + } + } + + @Test + public void testUpdateGroupWithSameName() { + AuthorizerConfigurationContext context = Mockito.mock(AuthorizerConfigurationContext.class); + AbstractPolicyBasedAuthorizer authorizer = new MockPolicyBasedAuthorizer(); + authorizer.onConfigured(context); + + Group group1 = new Group.Builder().identifier("group-id-1").name("abc").build(); + authorizer.addGroup(group1); + + Group group2 = new Group.Builder().identifier("group-id-2").name("xyz").build(); + authorizer.addGroup(group2); + + try { + Group group1Updated = new Group.Builder().identifier("group-id-1").name("xyz").build(); + authorizer.updateGroup(group1Updated); + Assert.fail("Should have thrown exception"); + } catch (IllegalStateException e) { } } 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 bfcfb7d3e9..8a4a6de7dc 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 @@ -292,7 +292,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, Autho AbstractPolicyBasedAuthorizer policyBasedAuthorizer = (AbstractPolicyBasedAuthorizer) baseAuthorizer; return new AbstractPolicyBasedAuthorizer() { @Override - public Group addGroup(Group group) throws AuthorizationAccessException { + public Group doAddGroup(Group group) throws AuthorizationAccessException { try (final NarCloseable narCloseable = NarCloseable.withNarLoader()) { return policyBasedAuthorizer.addGroup(group); } @@ -306,7 +306,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, Autho } @Override - public Group updateGroup(Group group) throws AuthorizationAccessException { + public Group doUpdateGroup(Group group) throws AuthorizationAccessException { try (final NarCloseable narCloseable = NarCloseable.withNarLoader()) { return policyBasedAuthorizer.updateGroup(group); } @@ -327,7 +327,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, Autho } @Override - public User addUser(User user) throws AuthorizationAccessException { + public User doAddUser(User user) throws AuthorizationAccessException { try (final NarCloseable narCloseable = NarCloseable.withNarLoader()) { return policyBasedAuthorizer.addUser(user); } @@ -348,7 +348,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, Autho } @Override - public User updateUser(User user) throws AuthorizationAccessException { + public User doUpdateUser(User user) throws AuthorizationAccessException { try (final NarCloseable narCloseable = NarCloseable.withNarLoader()) { return policyBasedAuthorizer.updateUser(user); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java index 3f714d4763..1cd40ea064 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-file-authorizer/src/main/java/org/apache/nifi/authorization/FileAuthorizer.java @@ -688,7 +688,7 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { // ------------------ Groups ------------------ @Override - public synchronized Group addGroup(Group group) throws AuthorizationAccessException { + public synchronized Group doAddGroup(Group group) throws AuthorizationAccessException { if (group == null) { throw new IllegalArgumentException("Group cannot be null"); } @@ -726,7 +726,7 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { } @Override - public synchronized Group updateGroup(Group group) throws AuthorizationAccessException { + public synchronized Group doUpdateGroup(Group group) throws AuthorizationAccessException { if (group == null) { throw new IllegalArgumentException("Group cannot be null"); } @@ -826,7 +826,7 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { // ------------------ Users ------------------ @Override - public synchronized User addUser(final User user) throws AuthorizationAccessException { + public synchronized User doAddUser(final User user) throws AuthorizationAccessException { if (user == null) { throw new IllegalArgumentException("User cannot be null"); } @@ -870,7 +870,7 @@ public class FileAuthorizer extends AbstractPolicyBasedAuthorizer { } @Override - public synchronized User updateUser(final User user) throws AuthorizationAccessException { + public synchronized User doUpdateUser(final User user) throws AuthorizationAccessException { if (user == null) { throw new IllegalArgumentException("User cannot be null"); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml index 8cace45bcb..f46792fac9 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/pom.xml @@ -163,7 +163,6 @@ nifi-mock test - org.spockframework spock-core diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/authorization/MockPolicyBasedAuthorizer.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/authorization/MockPolicyBasedAuthorizer.java new file mode 100644 index 0000000000..9b50b50ed4 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/authorization/MockPolicyBasedAuthorizer.java @@ -0,0 +1,183 @@ +/* + * 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.authorization.exception.AuthorizationAccessException; +import org.apache.nifi.authorization.exception.AuthorizerCreationException; +import org.apache.nifi.authorization.exception.AuthorizerDestructionException; + +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Mock implementation of AbstractPolicyBasedAuthorizer. + */ +public class MockPolicyBasedAuthorizer extends AbstractPolicyBasedAuthorizer { + + private Set groups = new HashSet<>(); + private Set users = new HashSet<>(); + private Set policies = new HashSet<>(); + + public MockPolicyBasedAuthorizer() { + + } + + public MockPolicyBasedAuthorizer(Set groups, Set users, Set policies) { + if (groups != null) { + this.groups.addAll(groups); + } + if (users != null) { + this.users.addAll(users); + } + if (policies != null) { + this.policies.addAll(policies); + } + } + + @Override + public Group doAddGroup(Group group) throws AuthorizationAccessException { + groups.add(group); + return group; + } + + @Override + public Group getGroup(String identifier) throws AuthorizationAccessException { + return groups.stream().filter(g -> g.getIdentifier().equals(identifier)).findFirst().get(); + } + + @Override + public Group doUpdateGroup(Group group) throws AuthorizationAccessException { + deleteGroup(group); + return addGroup(group); + } + + @Override + public Group deleteGroup(Group group) throws AuthorizationAccessException { + groups.remove(group); + return group; + } + + @Override + public Set getGroups() throws AuthorizationAccessException { + return groups; + } + + @Override + public User doAddUser(User user) throws AuthorizationAccessException { + users.add(user); + return user; + } + + @Override + public User getUser(String identifier) throws AuthorizationAccessException { + return users.stream().filter(u -> u.getIdentifier().equals(identifier)).findFirst().get(); + } + + @Override + public User getUserByIdentity(String identity) throws AuthorizationAccessException { + return users.stream().filter(u -> u.getIdentity().equals(identity)).findFirst().get(); + } + + @Override + public User doUpdateUser(User user) throws AuthorizationAccessException { + deleteUser(user); + return addUser(user); + } + + @Override + public User deleteUser(User user) throws AuthorizationAccessException { + users.remove(user); + return user; + } + + @Override + public Set getUsers() throws AuthorizationAccessException { + return users; + } + + @Override + protected AccessPolicy doAddAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { + policies.add(accessPolicy); + return accessPolicy; + } + + @Override + public AccessPolicy getAccessPolicy(String identifier) throws AuthorizationAccessException { + return policies.stream().filter(p -> p.getIdentifier().equals(identifier)).findFirst().get(); + } + + @Override + public AccessPolicy updateAccessPolicy(AccessPolicy accessPolicy) throws AuthorizationAccessException { + deleteAccessPolicy(accessPolicy); + return addAccessPolicy(accessPolicy); + } + + @Override + public AccessPolicy deleteAccessPolicy(AccessPolicy policy) throws AuthorizationAccessException { + policies.remove(policy); + return policy; + } + + @Override + public Set getAccessPolicies() throws AuthorizationAccessException { + return policies; + } + + @Override + public UsersAndAccessPolicies getUsersAndAccessPolicies() throws AuthorizationAccessException { + return new UsersAndAccessPolicies() { + @Override + public AccessPolicy getAccessPolicy(String resourceIdentifier, RequestAction action) { + return null; + } + + @Override + public User getUser(String identity) { + return getUserByIdentity(identity); + } + + @Override + public Set getGroups(String userIdentity) { + User user = getUserByIdentity(userIdentity); + if (user == null) { + return new HashSet<>(); + } else { + return groups.stream() + .filter(g -> g.getUsers().contains(user.getIdentifier())) + .collect(Collectors.toSet()); + } + } + }; + } + + @Override + public void initialize(AuthorizerInitializationContext initializationContext) throws AuthorizerCreationException { + + } + + @Override + public void doOnConfigured(AuthorizerConfigurationContext configurationContext) throws AuthorizerCreationException { + + } + + @Override + public void preDestruction() throws AuthorizerDestructionException { + + } + +} diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java index a2eeb58119..6e3f475fb8 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/test/java/org/apache/nifi/controller/TestFlowController.java @@ -21,6 +21,7 @@ import org.apache.nifi.admin.service.AuditService; import org.apache.nifi.authorization.AbstractPolicyBasedAuthorizer; import org.apache.nifi.authorization.AccessPolicy; import org.apache.nifi.authorization.Group; +import org.apache.nifi.authorization.MockPolicyBasedAuthorizer; import org.apache.nifi.authorization.RequestAction; import org.apache.nifi.authorization.User; import org.apache.nifi.cluster.protocol.DataFlow; @@ -40,17 +41,14 @@ import org.junit.Test; import org.mockito.Mockito; import java.nio.charset.StandardCharsets; -import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class TestFlowController { @@ -58,15 +56,20 @@ public class TestFlowController { private FlowController controller; private AbstractPolicyBasedAuthorizer authorizer; private StandardFlowSynchronizer standardFlowSynchronizer; + private FlowFileEventRepository flowFileEventRepo; + private AuditService auditService; + private StringEncryptor encryptor; + private NiFiProperties properties; + private BulletinRepository bulletinRepo; @Before public void setup() { System.setProperty(NiFiProperties.PROPERTIES_FILE_PATH, "src/test/resources/nifi.properties"); - final FlowFileEventRepository flowFileEventRepo = Mockito.mock(FlowFileEventRepository.class); - final AuditService auditService = Mockito.mock(AuditService.class); - final StringEncryptor encryptor = StringEncryptor.createEncryptor(); - final NiFiProperties properties = NiFiProperties.getInstance(); + flowFileEventRepo = Mockito.mock(FlowFileEventRepository.class); + auditService = Mockito.mock(AuditService.class); + encryptor = StringEncryptor.createEncryptor(); + properties = NiFiProperties.getInstance(); properties.setProperty(NiFiProperties.PROVENANCE_REPO_IMPLEMENTATION_CLASS, MockProvenanceRepository.class.getName()); properties.setProperty("nifi.remote.input.socket.port", ""); properties.setProperty("nifi.remote.input.secure", ""); @@ -107,12 +110,9 @@ public class TestFlowController { policies1.add(policy1); policies1.add(policy2); - authorizer = Mockito.mock(AbstractPolicyBasedAuthorizer.class); - when(authorizer.getGroups()).thenReturn(groups1); - when(authorizer.getUsers()).thenReturn(users1); - when(authorizer.getAccessPolicies()).thenReturn(policies1); + authorizer = new MockPolicyBasedAuthorizer(groups1, users1, policies1); - final BulletinRepository bulletinRepo = Mockito.mock(BulletinRepository.class); + bulletinRepo = Mockito.mock(BulletinRepository.class); controller = FlowController.createStandaloneInstance(flowFileEventRepo, properties, authorizer, auditService, encryptor, bulletinRepo); standardFlowSynchronizer = new StandardFlowSynchronizer(StringEncryptor.createEncryptor()); @@ -132,10 +132,7 @@ public class TestFlowController { controller.synchronize(standardFlowSynchronizer, proposedDataFlow); - // had a problem verifying the call to inheritFingerprint didn't happen, so just verify none of the add methods got called - verify(authorizer, times(0)).addUser(any(User.class)); - verify(authorizer, times(0)).addGroup(any(Group.class)); - //verify(authorizer, times(0)).addAccessPolicy(any(AccessPolicy.class)); + assertEquals(authFingerprint, authorizer.getFingerprint()); } @Test(expected = UninheritableFlowException.class) @@ -146,6 +143,7 @@ public class TestFlowController { when(proposedDataFlow.getAuthorizerFingerprint()).thenReturn(authFingerprint.getBytes(StandardCharsets.UTF_8)); controller.synchronize(standardFlowSynchronizer, proposedDataFlow); + assertNotEquals(authFingerprint, authorizer.getFingerprint()); } @Test(expected = UninheritableFlowException.class) @@ -163,13 +161,13 @@ public class TestFlowController { final DataFlow proposedDataFlow = Mockito.mock(DataFlow.class); when(proposedDataFlow.getAuthorizerFingerprint()).thenReturn(authFingerprint.getBytes(StandardCharsets.UTF_8)); - // reset the authorizer so it returns empty fingerprint - when(authorizer.getUsers()).thenReturn(new HashSet()); - when(authorizer.getGroups()).thenReturn(new HashSet()); - when(authorizer.getAccessPolicies()).thenReturn(new HashSet()); + authorizer = new MockPolicyBasedAuthorizer(); + assertNotEquals(authFingerprint, authorizer.getFingerprint()); + controller.shutdown(true); + controller = FlowController.createStandaloneInstance(flowFileEventRepo, properties, authorizer, auditService, encryptor, bulletinRepo); controller.synchronize(standardFlowSynchronizer, proposedDataFlow); - verify(authorizer, times(1)).inheritFingerprint(authFingerprint); + assertEquals(authFingerprint, authorizer.getFingerprint()); } @Test diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java index 41051e1a1e..a4be613325 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAO.java @@ -53,7 +53,7 @@ public class StandardPolicyBasedAuthorizerDAO implements AccessPolicyDAO, UserGr } else { this.authorizer = new AbstractPolicyBasedAuthorizer() { @Override - public Group addGroup(final Group group) throws AuthorizationAccessException { + public Group doAddGroup(final Group group) throws AuthorizationAccessException { throw new IllegalStateException(MSG_NON_ABSTRACT_POLICY_BASED_AUTHORIZER); } @@ -63,7 +63,7 @@ public class StandardPolicyBasedAuthorizerDAO implements AccessPolicyDAO, UserGr } @Override - public Group updateGroup(final Group group) throws AuthorizationAccessException { + public Group doUpdateGroup(final Group group) throws AuthorizationAccessException { throw new IllegalStateException(MSG_NON_ABSTRACT_POLICY_BASED_AUTHORIZER); } @@ -78,7 +78,7 @@ public class StandardPolicyBasedAuthorizerDAO implements AccessPolicyDAO, UserGr } @Override - public User addUser(final User user) throws AuthorizationAccessException { + public User doAddUser(final User user) throws AuthorizationAccessException { throw new IllegalStateException(MSG_NON_ABSTRACT_POLICY_BASED_AUTHORIZER); } @@ -93,7 +93,7 @@ public class StandardPolicyBasedAuthorizerDAO implements AccessPolicyDAO, UserGr } @Override - public User updateUser(final User user) throws AuthorizationAccessException { + public User doUpdateUser(final User user) throws AuthorizationAccessException { throw new IllegalStateException(MSG_NON_ABSTRACT_POLICY_BASED_AUTHORIZER); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy index 3a98c70ef6..340f6f9bcc 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/groovy/org/apache/nifi/web/dao/impl/StandardPolicyBasedAuthorizerDAOSpec.groovy @@ -250,13 +250,16 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { noExceptionThrown() then: - 1 * authorizer.addGroup(userGroup) >> userGroup + 1 * authorizer.getUsers() >> users + 1 * authorizer.getGroups() >> groups + 1 * authorizer.doAddGroup(userGroup) >> userGroup 0 * _ result?.equals userGroup where: - userGroup | _ - new Group.Builder().identifier('user-group-id-1').name('user-group-id-1').addUser('user-id-1').build() | _ + userGroup | users | groups + new Group.Builder().identifier('user-group-id-1') + .name('user-group-id-1').addUser('user-id-1').build() | [] as Set | [] as Set } @Unroll @@ -324,13 +327,16 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { then: 1 * authorizer.getGroup(requestDTO.id) >> userGroup - 1 * authorizer.updateGroup(userGroup) >> userGroup + 1 * authorizer.getUsers() >> users + 1 * authorizer.getGroups() >> groups + 1 * authorizer.doUpdateGroup(userGroup) >> userGroup 0 * _ result?.equals(userGroup) where: - userGroup | _ - new Group.Builder().identifier('user-group-id-1').name('user-group-id-1').addUser('user-id-1').build() | _ + userGroup | users | groups + new Group.Builder().identifier('user-group-id-1') + .name('user-group-id-1').addUser('user-id-1').build() | [] as Set | [] as Set } @Unroll @@ -417,13 +423,16 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { noExceptionThrown() then: - 1 * authorizer.addUser(user) >> user + 1 * authorizer.getUsers() >> users + 1 * authorizer.getGroups() >> groups + 1 * authorizer.doAddUser(user) >> user 0 * _ result?.equals user where: - user | _ - new User.Builder().identifier('user-id-1').identity('user identity').build() | _ + user | users | groups + new User.Builder().identifier('user-id-1') + .identity('user identity').build() | [] as Set | [] as Set } @Unroll @@ -491,13 +500,16 @@ class StandardPolicyBasedAuthorizerDAOSpec extends Specification { then: 1 * authorizer.getUser(requestDTO.id) >> user - 1 * authorizer.updateUser(user) >> user + 1 * authorizer.getUsers() >> users + 1 * authorizer.getGroups() >> groups + 1 * authorizer.doUpdateUser(user) >> user 0 * _ result?.equals(user) where: - user | _ - new User.Builder().identifier('user-id-1').identity('user identity').build() | _ + user | users | groups + new User.Builder().identifier('user-id-1') + .identity('user identity').build() | [] as Set | [] as Set } @Unroll diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/users/nf-users-table.js b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/users/nf-users-table.js index 0d4a1b08a5..b6a73f7c99 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/users/nf-users-table.js +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-ui/src/main/webapp/js/nf/users/nf-users-table.js @@ -365,7 +365,7 @@ nf.UsersTable = (function () { contentType: 'application/json' }).done(function (groupEntity) { nf.UsersTable.loadUsersTable(); - }); + }).fail(nf.Common.handleAjaxError); }; /**