Repository: nifi Updated Branches: refs/heads/master 59970344f -> 5d5edfa89
NIFI-4744 Detect incorrect authorizers config Adds stricter checks in AuthorizerFactoryBean for unique ids within a given type of provider and requires unique providers in composite and composite-configurable user group providers. Failed validation checks cause startup to fail. Adds test cases for these new rules. This closes #2419. Signed-off-by: Bryan Bende <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/nifi/repo Commit: http://git-wip-us.apache.org/repos/asf/nifi/commit/5d5edfa8 Tree: http://git-wip-us.apache.org/repos/asf/nifi/tree/5d5edfa8 Diff: http://git-wip-us.apache.org/repos/asf/nifi/diff/5d5edfa8 Branch: refs/heads/master Commit: 5d5edfa8960bf7785200912959eb69f313c3c181 Parents: 5997034 Author: Kevin Doran <[email protected]> Authored: Fri Jan 19 11:50:36 2018 -0500 Committer: Bryan Bende <[email protected]> Committed: Mon Jan 22 09:45:17 2018 -0500 ---------------------------------------------------------------------- .../nifi-framework/nifi-authorizer/pom.xml | 12 ++++ .../authorization/AuthorizerFactoryBean.java | 74 ++++++++++++-------- .../AuthorizerFactoryBeanSpec.groovy | 72 +++++++++++++++++++ .../CompositeConfigurableUserGroupProvider.java | 14 ++++ .../CompositeUserGroupProvider.java | 4 ++ ...positeConfigurableUserGroupProviderTest.java | 29 ++++++++ .../CompositeUserGroupProviderTest.java | 6 ++ 7 files changed, 183 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/pom.xml ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/pom.xml b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/pom.xml index 6577cb0..697fe9e 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/pom.xml +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/pom.xml @@ -95,6 +95,18 @@ <groupId>org.apache.nifi</groupId> <artifactId>nifi-properties-loader</artifactId> </dependency> + + <!-- Spock testing dependencies--> + <dependency> + <groupId>org.spockframework</groupId> + <artifactId>spock-core</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>cglib</groupId> + <artifactId>cglib-nodep</artifactId> + <scope>test</scope> + </dependency> </dependencies> </project> \ No newline at end of file http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java ---------------------------------------------------------------------- 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 9f17a18..7ece8eb 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 @@ -44,6 +44,7 @@ import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBElement; import javax.xml.bind.JAXBException; import javax.xml.bind.Unmarshaller; +import javax.xml.stream.XMLStreamException; import javax.xml.stream.XMLStreamReader; import javax.xml.transform.stream.StreamSource; import javax.xml.validation.Schema; @@ -56,9 +57,11 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; /** * Factory bean for loading the configured authorizer. @@ -123,6 +126,9 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG // create each user group provider for (final org.apache.nifi.authorization.generated.UserGroupProvider userGroupProvider : authorizerConfiguration.getUserGroupProvider()) { + if (userGroupProviders.containsKey(userGroupProvider.getIdentifier())) { + throw new Exception("Duplicate User Group Provider identifier in Authorizers configuration: " + userGroupProvider.getIdentifier()); + } userGroupProviders.put(userGroupProvider.getIdentifier(), createUserGroupProvider(userGroupProvider.getIdentifier(), userGroupProvider.getClazz())); } @@ -134,6 +140,9 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG // create each access policy provider for (final org.apache.nifi.authorization.generated.AccessPolicyProvider accessPolicyProvider : authorizerConfiguration.getAccessPolicyProvider()) { + if (accessPolicyProviders.containsKey(accessPolicyProvider.getIdentifier())) { + throw new Exception("Duplicate Access Policy Provider identifier in Authorizers configuration: " + accessPolicyProvider.getIdentifier()); + } accessPolicyProviders.put(accessPolicyProvider.getIdentifier(), createAccessPolicyProvider(accessPolicyProvider.getIdentifier(), accessPolicyProvider.getClazz())); } @@ -145,6 +154,9 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG // create each authorizer for (final org.apache.nifi.authorization.generated.Authorizer authorizer : authorizerConfiguration.getAuthorizer()) { + if (authorizers.containsKey(authorizer.getIdentifier())) { + throw new Exception("Duplicate Authorizer identifier in Authorizers configuration: " + authorizer.getIdentifier()); + } authorizers.put(authorizer.getIdentifier(), createAuthorizer(authorizer.getIdentifier(), authorizer.getClazz(),authorizer.getClasspath())); } @@ -184,7 +196,7 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG unmarshaller.setSchema(schema); final JAXBElement<Authorizers> element = unmarshaller.unmarshal(xsr, Authorizers.class); return element.getValue(); - } catch (SAXException | JAXBException e) { + } catch (XMLStreamException | SAXException | JAXBException e) { throw new Exception("Unable to load the authorizer configuration file at: " + authorizersConfigurationFile.getAbsolutePath(), e); } } else { @@ -478,38 +490,44 @@ public class AuthorizerFactoryBean implements FactoryBean, DisposableBean, UserG @Override public void destroy() throws Exception { - if (authorizer != null) { - Exception error = null; + List<Exception> errors = new ArrayList<>(); - try { - authorizer.preDestruction(); - } catch (final Exception e) { - error = e; - } + if (authorizers != null) { + authorizers.forEach((identifier, object) -> { + try { + object.preDestruction(); + } catch (Exception e) { + errors.add(e); + logger.error("Error pre-destructing {}: {}", identifier, e); + } + }); + } - if (authorizer instanceof ManagedAuthorizer) { - final AccessPolicyProvider accessPolicyProvider = ((ManagedAuthorizer) authorizer).getAccessPolicyProvider(); - if (accessPolicyProvider != null) { - try { - accessPolicyProvider.preDestruction(); - } catch (final Exception e) { - error = e; - } + if (accessPolicyProviders != null) { + accessPolicyProviders.forEach((identifier, object) -> { + try { + object.preDestruction(); + } catch (Exception e) { + errors.add(e); + logger.error("Error pre-destructing {}: {}", identifier, e); + } + }); + } - final UserGroupProvider userGroupProvider = accessPolicyProvider.getUserGroupProvider(); - if (userGroupProvider != null) { - try { - userGroupProvider.preDestruction(); - } catch (final Exception e) { - error = e; - } - } + if (userGroupProviders != null) { + userGroupProviders.forEach((identifier, object) -> { + try { + object.preDestruction(); + } catch (Exception e) { + errors.add(e); + logger.error("Error pre-destructing {}: {}", identifier, e); } - } + }); + } - if (error != null) { - throw error; - } + if (!errors.isEmpty()) { + List<String> errorMessages = errors.stream().map(Throwable::toString).collect(Collectors.toList()); + throw new AuthorizerDestructionException("One or more providers encountered a pre-destruction error: " + StringUtils.join(errorMessages, "; "), errors.get(0)); } } http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/groovy/org/apache/nifi/authorization/AuthorizerFactoryBeanSpec.groovy ---------------------------------------------------------------------- diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/groovy/org/apache/nifi/authorization/AuthorizerFactoryBeanSpec.groovy b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/groovy/org/apache/nifi/authorization/AuthorizerFactoryBeanSpec.groovy new file mode 100644 index 0000000..c16e018 --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/test/groovy/org/apache/nifi/authorization/AuthorizerFactoryBeanSpec.groovy @@ -0,0 +1,72 @@ +/* + * 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.resource.ResourceFactory +import org.apache.nifi.util.NiFiProperties +import spock.lang.Specification + +class AuthorizerFactoryBeanSpec extends Specification { + + def mockProperties = Mock(NiFiProperties) + + AuthorizerFactoryBean authorizerFactoryBean + + // runs before every feature method + def setup() { + authorizerFactoryBean = new AuthorizerFactoryBean() + authorizerFactoryBean.setProperties(mockProperties) + } + + // runs after every feature method + def cleanup() {} + + // runs before the first feature method + def setupSpec() {} + + // runs after the last feature method + def cleanupSpec() {} + + def "create default authorizer"() { + + setup: "properties indicate nifi-registry is unsecured" + mockProperties.getProperty(NiFiProperties.WEB_HTTPS_PORT) >> "" + + when: "getAuthorizer() is first called" + def authorizer = (Authorizer) authorizerFactoryBean.getObject() + + then: "the default authorizer is returned" + authorizer != null + + and: "any authorization request made to that authorizer is approved" + def authorizationResult = authorizer.authorize(getTestAuthorizationRequest()) + authorizationResult.result == AuthorizationResult.Result.Approved + + } + + // Helper methods + + private static AuthorizationRequest getTestAuthorizationRequest() { + return new AuthorizationRequest.Builder() + .resource(ResourceFactory.getFlowResource()) + .action(RequestAction.WRITE) + .accessAttempt(false) + .anonymous(true) + .build() + } + +} http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProvider.java ---------------------------------------------------------------------- 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 a262197..91c4186 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 @@ -21,9 +21,12 @@ import org.apache.nifi.authorization.exception.AuthorizerCreationException; import org.apache.nifi.authorization.exception.AuthorizerDestructionException; import org.apache.nifi.authorization.exception.UninheritableAuthorizationsException; import org.apache.nifi.components.PropertyValue; +import org.apache.nifi.util.StringUtils; import java.util.HashSet; +import java.util.Map; import java.util.Set; +import java.util.regex.Matcher; public class CompositeConfigurableUserGroupProvider extends CompositeUserGroupProvider implements ConfigurableUserGroupProvider { @@ -61,6 +64,17 @@ public class CompositeConfigurableUserGroupProvider extends CompositeUserGroupPr throw new AuthorizerCreationException(String.format("The Configurable User Group Provider is not configurable: %s", configurableUserGroupProviderKey)); } + // Ensure that the ConfigurableUserGroupProvider is not also listed as one of the providers for the CompositeUserGroupProvider + for (Map.Entry<String, String> entry : configurationContext.getProperties().entrySet()) { + Matcher matcher = USER_GROUP_PROVIDER_PATTERN.matcher(entry.getKey()); + if (matcher.matches() && !StringUtils.isBlank(entry.getValue())) { + final String userGroupProviderKey = entry.getValue(); + if (userGroupProviderKey.equals(configurableUserGroupProviderKey.getValue())) { + throw new AuthorizerCreationException(String.format("Duplicate provider in Composite Configurable User Group Provider configuration: %s", userGroupProviderKey)); + } + } + } + configurableUserGroupProvider = (ConfigurableUserGroupProvider) userGroupProvider; // configure the CompositeUserGroupProvider http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/CompositeUserGroupProvider.java ---------------------------------------------------------------------- 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 916865b..2bef05e 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 @@ -67,6 +67,10 @@ public class CompositeUserGroupProvider implements UserGroupProvider { throw new AuthorizerCreationException(String.format("Unable to locate the configured User Group Provider: %s", userGroupProviderKey)); } + if (userGroupProviders.contains(userGroupProvider)) { + throw new AuthorizerCreationException(String.format("Duplicate provider in Composite User Group Provider configuration: %s", userGroupProviderKey)); + } + userGroupProviders.add(userGroupProvider); } } http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeConfigurableUserGroupProviderTest.java ---------------------------------------------------------------------- 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 cdef935..6b41d09 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 @@ -20,11 +20,14 @@ import org.apache.nifi.attribute.expression.language.StandardPropertyValue; import org.apache.nifi.authorization.exception.AuthorizerCreationException; import org.junit.Test; +import java.util.HashMap; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; import static org.apache.nifi.authorization.CompositeConfigurableUserGroupProvider.PROP_CONFIGURABLE_USER_GROUP_PROVIDER; +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.assertNull; @@ -64,6 +67,32 @@ public class CompositeConfigurableUserGroupProviderTest extends CompositeUserGro }); } + @Test(expected = AuthorizerCreationException.class) + public void testDuplicateProviders() throws Exception { + + // Mock UserGroupProviderLookup + UserGroupProvider configurableUserGroupProvider = getConfigurableUserGroupProvider(); + final UserGroupProviderLookup ugpLookup = mock(UserGroupProviderLookup.class); + when(ugpLookup.getUserGroupProvider(eq(CONFIGURABLE_USER_GROUP_PROVIDER))).thenReturn(configurableUserGroupProvider); + + // Mock AuthorizerInitializationContext + final AuthorizerInitializationContext initializationContext = mock(AuthorizerInitializationContext.class); + when(initializationContext.getUserGroupProviderLookup()).thenReturn(ugpLookup); + + // Mock AuthorizerConfigurationContext to introduce the duplicate provider ids + final AuthorizerConfigurationContext configurationContext = mock(AuthorizerConfigurationContext.class); + when(configurationContext.getProperty(PROP_CONFIGURABLE_USER_GROUP_PROVIDER)).thenReturn(new StandardPropertyValue(CONFIGURABLE_USER_GROUP_PROVIDER, null)); + Map<String, String> configurationContextProperties = new HashMap<>(); + configurationContextProperties.put(PROP_USER_GROUP_PROVIDER_PREFIX + "1", CONFIGURABLE_USER_GROUP_PROVIDER); + configurationContextProperties.put(PROP_USER_GROUP_PROVIDER_PREFIX + "2", NOT_CONFIGURABLE_USER_GROUP_PROVIDER); + when(configurationContext.getProperties()).thenReturn(configurationContextProperties); + + // configure (should throw exception) + CompositeConfigurableUserGroupProvider provider = new CompositeConfigurableUserGroupProvider(); + provider.initialize(initializationContext); + provider.onConfigured(configurationContext); + } + @Test public void testConfigurableUserGroupProviderOnly() throws Exception { final UserGroupProvider userGroupProvider = initCompositeUserGroupProvider(new CompositeConfigurableUserGroupProvider(), lookup -> { http://git-wip-us.apache.org/repos/asf/nifi/blob/5d5edfa8/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/test/java/org/apache/nifi/authorization/CompositeUserGroupProviderTest.java ---------------------------------------------------------------------- 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 acc351b..0be2d02 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 @@ -62,6 +62,12 @@ public class CompositeUserGroupProviderTest extends CompositeUserGroupProviderTe compositeUserGroupProvider.onConfigured(configurationContext); } + @Test(expected = AuthorizerCreationException.class) + public void testDuplicateProviders() throws Exception { + UserGroupProvider duplicatedUserGroupProvider = getUserGroupProviderOne(); + initCompositeUserGroupProvider(new CompositeUserGroupProvider(), null, null, duplicatedUserGroupProvider, duplicatedUserGroupProvider); + } + @Test public void testOneProvider() throws Exception { final UserGroupProvider userGroupProvider = initCompositeUserGroupProvider(new CompositeUserGroupProvider(), null, null, getUserGroupProviderOne());
