From 850f484e5a0ab91101666fef1bc91f2b4c3a74ba Mon Sep 17 00:00:00 2001 From: agazzarini Date: Thu, 8 Jul 2021 08:39:23 +0200 Subject: [PATCH] [ SEARCH-2985 ] Safe getCommsMethod() + Unit Tests (cherry picked from commit cea9eafb2082fdc1444a468894495d586c3610c6) --- .../SecretSharedPropertyCollector.java | 91 +++------- .../security/SecretSharedPropertyHelper.java | 85 +++++++++ .../SecretSharedPropertyCollectorTest.java | 165 ++++++++++++++++++ 3 files changed, 270 insertions(+), 71 deletions(-) create mode 100644 search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java create mode 100644 search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java index 559684572..f37ec1067 100644 --- a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java @@ -26,23 +26,13 @@ package org.alfresco.solr.security; -import java.io.FileReader; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.HashSet; -import java.util.List; -import java.util.Objects; -import java.util.Properties; -import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; - import org.alfresco.httpclient.HttpClientFactory; import org.alfresco.solr.AlfrescoSolrDataModel; import org.alfresco.solr.config.ConfigUtil; -import org.apache.solr.core.SolrResourceLoader; + +import java.util.Objects; +import java.util.Properties; +import java.util.Set; /** * Provides property values for Alfresco Communication using "secret" method: @@ -58,12 +48,12 @@ public class SecretSharedPropertyCollector public final static String SECRET_SHARED_METHOD_KEY = "secret"; // Property names for "secret" communication method - private static String SECURE_COMMS_PROPERTY = "alfresco.secureComms"; - private static String SHARED_SECRET = "alfresco.secureComms.secret"; - private static String SHARED_SECRET_HEADER = "alfresco.secureComms.secret.header"; + static final String SECURE_COMMS_PROPERTY = "alfresco.secureComms"; + private final static String SHARED_SECRET = "alfresco.secureComms.secret"; + private final static String SHARED_SECRET_HEADER = "alfresco.secureComms.secret.header"; // Save communication method as static value in order to improve performance - private static String commsMethod; + static String commsMethod; /** * Check if communications method is "secret" @@ -79,9 +69,8 @@ public class SecretSharedPropertyCollector * Get communication method from environment variables, shared properties or core properties. * @return Communication method: none, https, secret */ - private static String getCommsMethod() + static String getCommsMethod() { - if (commsMethod == null) { @@ -96,14 +85,23 @@ public class SecretSharedPropertyCollector if (commsMethod == null) { // Get configuration from deployed SOLR Cores - Set secureCommsSet = getCommsFromCores(); + Set secureCommsSet = SecretSharedPropertyHelper.getCommsFromCores(); + + // In case of multiple cores, *all* of them must have the same secureComms value. + // From that perspective, you may find the second clause in the conditional statement + // below not strictly necessary. The reason is that the check below is in charge to make + // sure a consistent configuration about the secret shared property has been defined in all cores. if (secureCommsSet.size() > 1 && secureCommsSet.contains(SECRET_SHARED_METHOD_KEY)) { throw new RuntimeException( "No valid secure comms values: all the cores must be using \"secret\" communication method but found: " + secureCommsSet); } - commsMethod = secureCommsSet.stream().findFirst().get(); + + return commsMethod = + secureCommsSet.isEmpty() + ? null + : secureCommsSet.iterator().next(); } } @@ -113,55 +111,6 @@ public class SecretSharedPropertyCollector } - /** - * Read different values of "alfresco.secureComms" property from every "solrcore.properties" files. - * @return List of different communication methods declared in SOLR Cores. - */ - private static Set getCommsFromCores() - { - - Set secureCommsSet = new HashSet<>(); - - try (Stream walk = Files.walk(Paths.get(SolrResourceLoader.locateSolrHome().toString()))) - { - - List coreFiles = walk.map(x -> x.toString()) - .filter(f -> f.contains("solrcore.properties") && !f.contains("templates")) - .collect(Collectors.toList()); - - coreFiles.forEach(coreFile -> { - - Properties props = new Properties(); - try - { - props.load(new FileReader(coreFile)); - } - catch (Exception e) - { - throw new RuntimeException(e); - } - String prop = props.getProperty(SECURE_COMMS_PROPERTY); - if (prop != null) - { - secureCommsSet.add(prop); - } - else - { - secureCommsSet.add("none"); - } - - }); - - } - catch (IOException e) - { - throw new RuntimeException(e); - } - - return secureCommsSet; - - } - /** * Read "secret" word from Java environment variable "alfresco.secureComms.secret" * diff --git a/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java new file mode 100644 index 000000000..785e931e5 --- /dev/null +++ b/search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java @@ -0,0 +1,85 @@ +/* + * #%L + * Alfresco Search Services + * %% + * 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.solr.security; + +import org.apache.solr.core.SolrResourceLoader; + +import java.io.FileReader; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Properties; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Stream; + +import static java.util.stream.Collectors.toList; +import static java.util.stream.Collectors.toSet; +import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECURE_COMMS_PROPERTY; + +class SecretSharedPropertyHelper +{ + private final static Function toProperties = + file -> { + Properties props = new Properties(); + try + { + props.load(new FileReader(file)); + } + catch (Exception e) + { + throw new RuntimeException(e); + } + return props; + }; + + /** + * Read different values of "alfresco.secureComms" property from every "solrcore.properties" files. + * + * @return List of different communication methods declared in SOLR Cores. + */ + static Set getCommsFromCores() + { + try (Stream walk = Files.walk(Paths.get(SolrResourceLoader.locateSolrHome().toString()))) + { + var solrCorePropertiesFiles = + walk.map(Path::toString) + .filter(path -> path.contains("solrcore.properties") + && !path.contains("templates")) + .collect(toList()); + + return solrCorePropertiesFiles.stream() + .map(toProperties) + .map(properties -> properties.getProperty(SECURE_COMMS_PROPERTY, "none")) + .collect(toSet()); + } + catch (IOException e) + { + throw new RuntimeException(e); + } + } +} diff --git a/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java b/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java new file mode 100644 index 000000000..27313f364 --- /dev/null +++ b/search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java @@ -0,0 +1,165 @@ +/* + * #%L + * Alfresco Search Services + * %% + * 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.solr.security; + +import org.alfresco.solr.AlfrescoSolrDataModel; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; + +import java.util.Properties; +import java.util.Set; + +import static java.util.Collections.emptySet; +import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY; +import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECURE_COMMS_PROPERTY; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mockStatic; + +public class SecretSharedPropertyCollectorTest +{ + private final static String A_COMMS_METHOD = "aCommsMethod"; + private final static String SET_THROUGH_SYSTEM_PROPERTY = "aCommsMethod_SetThroughSystemProperty"; + private final static String SET_THROUGH_ALFRESCO_COMMON_CONFIG = "aCommsMethod_SetThroughAlfrescoCommonConfig"; + private final static String COMMS_METHOD_FROM_SOLRCORE = "aCommsMethod_FromSolrCore"; + + @Before + public void setUp() + { + SecretSharedPropertyCollector.commsMethod = null; + assertNull(System.getProperty(SECURE_COMMS_PROPERTY)); + assertNull(AlfrescoSolrDataModel.getCommonConfig().getProperty(SECURE_COMMS_PROPERTY)); + } + + @After + public void tearDown() + { + System.clearProperty(SECURE_COMMS_PROPERTY); + AlfrescoSolrDataModel.getCommonConfig().remove(SECURE_COMMS_PROPERTY); + } + + @Test + public void commsMethodIsNotNull_shouldReturnThatValue() + { + SecretSharedPropertyCollector.commsMethod = A_COMMS_METHOD; + assertEquals(A_COMMS_METHOD, SecretSharedPropertyCollector.getCommsMethod()); + + assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); + } + + @Test + public void commsMethodIsNotNullAndIsSecret_shouldReturnThatValue() + { + SecretSharedPropertyCollector.commsMethod = SECRET_SHARED_METHOD_KEY; + assertEquals(SECRET_SHARED_METHOD_KEY, SecretSharedPropertyCollector.getCommsMethod()); + + assertTrue(SecretSharedPropertyCollector.isCommsSecretShared()); + } + + @Test + public void commsMethodThroughSystemProperty() + { + System.setProperty(SECURE_COMMS_PROPERTY, SET_THROUGH_SYSTEM_PROPERTY); + assertEquals(SET_THROUGH_SYSTEM_PROPERTY, SecretSharedPropertyCollector.getCommsMethod()); + + assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); + } + + @Test + public void commsMethodSetToSecretThroughSystemProperty() + { + System.setProperty(SECURE_COMMS_PROPERTY, SECRET_SHARED_METHOD_KEY); + assertEquals(SECRET_SHARED_METHOD_KEY, SecretSharedPropertyCollector.getCommsMethod()); + + assertTrue(SecretSharedPropertyCollector.isCommsSecretShared()); + } + + @Test + public void commsMethodThroughAlfrescoProperties() + { + try(MockedStatic mock = mockStatic(AlfrescoSolrDataModel.class)) + { + var alfrescoCommonConfig = new Properties(); + alfrescoCommonConfig.setProperty(SECURE_COMMS_PROPERTY, SET_THROUGH_ALFRESCO_COMMON_CONFIG); + + mock.when(AlfrescoSolrDataModel::getCommonConfig).thenReturn(alfrescoCommonConfig); + assertEquals(SET_THROUGH_ALFRESCO_COMMON_CONFIG, SecretSharedPropertyCollector.getCommsMethod()); + + assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); + } + } + + @Test + public void commsMethodThroughSolrCores() + { + try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) + { + mock.when(SecretSharedPropertyHelper::getCommsFromCores).thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE)); + assertEquals(COMMS_METHOD_FROM_SOLRCORE, SecretSharedPropertyCollector.getCommsMethod()); + + assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); + } + } + + /** + * In case no core has been defined in the Solr instance, no comms method + * can be defined (assuming that value cannot be retrieved from configuration + * or system properties). + * + * @see SEARCH-2985 + */ + @Test + public void commsMethodThroughSolrCoresReturnEmptySet() + { + try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) + { + mock.when(SecretSharedPropertyHelper::getCommsFromCores).thenReturn(emptySet()); + assertNull(SecretSharedPropertyCollector.getCommsMethod()); + + assertFalse(SecretSharedPropertyCollector.isCommsSecretShared()); + } + } + + /** + * When multiple solr cores are defined and they are using the shared secret, + * they are all supposed to use the same communication method. + */ + @Test(expected = RuntimeException.class) + public void commsMethodThroughSolrCoresReturnsMoreThanOneValue() + { + try(MockedStatic mock = mockStatic(SecretSharedPropertyHelper.class)) + { + mock.when(SecretSharedPropertyHelper::getCommsFromCores) + .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE, SECRET_SHARED_METHOD_KEY)); + + SecretSharedPropertyCollector.getCommsMethod(); + } + } +}