From ff9be3024bb94ba7beeda04e4d78af95914deecc Mon Sep 17 00:00:00 2001 From: Tom Page Date: Wed, 26 Feb 2020 16:32:52 +0000 Subject: [PATCH 1/2] RM-7119 Fix startup with ACS 5.2.7. --- .../RMMethodSecurityPostProcessor.java | 32 ++++---- ...RMMethodSecurityPostProcessorUnitTest.java | 80 +++++++++++++++++++ 2 files changed, 97 insertions(+), 15 deletions(-) create mode 100644 rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java index 41356e0cca..cdcb5aa89c 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java @@ -35,6 +35,8 @@ import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.PropertyValue; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; @@ -51,7 +53,7 @@ import org.springframework.beans.factory.config.TypedStringValue; */ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor { - private static Log logger = LogFactory.getLog(RMMethodSecurityPostProcessor.class); + private static final Logger LOGGER = LoggerFactory.getLogger(RMMethodSecurityPostProcessor.class); public static final String PROP_OBJECT_DEFINITION_SOURCE = "objectDefinitionSource"; public static final String PROPERTY_PREFIX = "rm.methodsecurity."; @@ -94,10 +96,7 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor { if (beanFactory.containsBeanDefinition(bean)) { - if (logger.isDebugEnabled()) - { - logger.debug("Adding RM method security definitions for " + bean); - } + LOGGER.debug("Adding RM method security definitions for {}", bean); BeanDefinition beanDef = beanFactory.getBeanDefinition(bean); PropertyValue beanValue = beanDef.getPropertyValues().getPropertyValue(PROP_OBJECT_DEFINITION_SOURCE); @@ -134,10 +133,7 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor String securityBeanName = split[index] + SECURITY_BEAN_POSTFIX; if (!securityBeanNameCache.contains(securityBeanName) && beanFactory.containsBean(securityBeanName)) { - if (logger.isDebugEnabled()) - { - logger.debug("Adding " + securityBeanName + " to list from properties."); - } + LOGGER.debug("Adding {} to list from properties.", securityBeanName); securityBeanNameCache.add(securityBeanName); } @@ -166,10 +162,7 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor } else { - if (logger.isWarnEnabled()) - { - logger.warn("Missing RM security definition for method " + key); - } + LOGGER.warn("Missing RM security definition for method {}", key); } } @@ -180,13 +173,22 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor * @param stringValue * @return */ - private Map convertToMap(String stringValue) + protected Map convertToMap(String stringValue) { String[] values = stringValue.trim().split("\n"); Map map = new HashMap(values.length); for (String value : values) { - String[] pair = value.trim().split("="); + String trimmed = value.trim(); + if (trimmed.equals("")) + { + continue; + } + String[] pair = trimmed.split("=", 2); + if (pair.length != 2) + { + LOGGER.error("Error converting string to map: {}", trimmed); + } map.put(pair[0], pair[1]); } return map; diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java new file mode 100644 index 0000000000..998c176047 --- /dev/null +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java @@ -0,0 +1,80 @@ +/* + * #%L + * Alfresco Records Management Module + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * - + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * - + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ +package org.alfresco.module.org_alfresco_module_rm.security; + +import static java.util.Collections.emptyMap; + +import static org.junit.Assert.assertEquals; + +import java.util.Map; + +import com.google.common.collect.ImmutableMap; + +import org.junit.Test; + +/** + * Unit tests for {@link RMMethodSecurityPostProcessor}. + */ +public class RMMethodSecurityPostProcessorUnitTest +{ + private RMMethodSecurityPostProcessor rmMethodSecurityPostProcessor = new RMMethodSecurityPostProcessor(); + + @Test + public void testConvertToMap_emptyString() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap(""); + assertEquals("Unexpectedly included empty string in output.", emptyMap(), actual); + } + + @Test + public void testConvertToMap_normalPairs() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b\nc=d"); + assertEquals("Failed to handle multiline input string.", ImmutableMap.of("a", "b", "c", "d"), actual); + } + + @Test + public void testConvertToMap_stripWhitespace() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap(" \n \t a=b \n \t "); + assertEquals("Failed to strip whitespace.", ImmutableMap.of("a", "b"), actual); + } + + @Test + public void testConvertToMap_ignoreBlankLine() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b\n\nc=d"); + assertEquals("Failed to ignore blank line.", ImmutableMap.of("a", "b", "c", "d"), actual); + } + + @Test + public void testConvertToMap_multipleEquals() + { + Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b=c\nd=e=f"); + assertEquals("Issue with handling of = symbol in value.", ImmutableMap.of("a", "b=c", "d", "e=f"), actual); + } +} From ddae9fbd648cbd9657e58428e7801dde256def53 Mon Sep 17 00:00:00 2001 From: Tom Page Date: Thu, 27 Feb 2020 07:55:40 +0000 Subject: [PATCH 2/2] RM-7119 Throw explicit exception rather than logging message. Also address other code review comments. --- .../security/RMMethodSecurityPostProcessor.java | 12 ++++++++---- .../RMMethodSecurityPostProcessorUnitTest.java | 11 +++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java index cdcb5aa89c..b6eac36746 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessor.java @@ -33,6 +33,7 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import org.alfresco.error.AlfrescoRuntimeException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.slf4j.Logger; @@ -170,8 +171,11 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor } /** - * @param stringValue - * @return + * Convert the lines of a string to a map, separating keys from values by the first "=" sign. + * + * @param stringValue The multi-line string. + * @return The resulting map. + * @throws AlfrescoRuntimeException If a non-blank line does not contain an "=" sign. */ protected Map convertToMap(String stringValue) { @@ -180,14 +184,14 @@ public class RMMethodSecurityPostProcessor implements BeanFactoryPostProcessor for (String value : values) { String trimmed = value.trim(); - if (trimmed.equals("")) + if (trimmed.isEmpty()) { continue; } String[] pair = trimmed.split("=", 2); if (pair.length != 2) { - LOGGER.error("Error converting string to map: {}", trimmed); + throw new AlfrescoRuntimeException("Could not convert string to map " + trimmed); } map.put(pair[0], pair[1]); } diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java index 998c176047..bb996e6c4d 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityPostProcessorUnitTest.java @@ -34,13 +34,17 @@ import java.util.Map; import com.google.common.collect.ImmutableMap; +import org.alfresco.error.AlfrescoRuntimeException; import org.junit.Test; /** * Unit tests for {@link RMMethodSecurityPostProcessor}. + * + * See RM-7119. */ public class RMMethodSecurityPostProcessorUnitTest { + /** The class under test. */ private RMMethodSecurityPostProcessor rmMethodSecurityPostProcessor = new RMMethodSecurityPostProcessor(); @Test @@ -77,4 +81,11 @@ public class RMMethodSecurityPostProcessorUnitTest Map actual = rmMethodSecurityPostProcessor.convertToMap("a=b=c\nd=e=f"); assertEquals("Issue with handling of = symbol in value.", ImmutableMap.of("a", "b=c", "d", "e=f"), actual); } + + /** Check that if a line is missing an equals sign then we get an exception. */ + @Test(expected = AlfrescoRuntimeException.class) + public void testConvertToMap_missingEquals() + { + rmMethodSecurityPostProcessor.convertToMap("a=b\ncd"); + } }