Merge pull request #1228 from Alfresco/fix/SEARCH-2985_in_2.0.x

[ SEARCH-2985 ] Safe getCommsMethod() + Unit Tests

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<String> secureCommsSet = getCommsFromCores();
+                    Set<String> 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<String> getCommsFromCores()
-    {
-
-        Set<String> secureCommsSet = new HashSet<>();
-
-        try (Stream<Path> walk = Files.walk(Paths.get(SolrResourceLoader.locateSolrHome().toString())))
-        {
-
-            List<String> 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"
      * 

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 <http://www.gnu.org/licenses/>.
+ * #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<String, Properties> 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<String> getCommsFromCores()
+    {
+        try (Stream<Path> 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);
+        }
+    }
+}

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 <http://www.gnu.org/licenses/>.
+ * #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<AlfrescoSolrDataModel> 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<SecretSharedPropertyHelper> 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 <a href="https://alfresco.atlassian.net/browse/SEARCH-2985">SEARCH-2985</a>
+     */
+    @Test
+    public void commsMethodThroughSolrCoresReturnEmptySet()
+    {
+        try(MockedStatic<SecretSharedPropertyHelper> 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<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
+        {
+            mock.when(SecretSharedPropertyHelper::getCommsFromCores)
+                    .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE, SECRET_SHARED_METHOD_KEY));
+
+            SecretSharedPropertyCollector.getCommsMethod();
+        }
+    }
+}