ACS-2563 Disallow alfresco.secureComms=none

(cherry picked from commit e26c7f08601e15fcde0ba51df90212d69b9800e1)

e2e-test/helpers/wait-service-to-start.sh

@@ -14,7 +14,12 @@ do
 
    echo "Waiting for Service to start using endpoint: ${endpoint}"
 
-   until [[ "$(curl --output /dev/null -w ''%{http_code}'' --silent --head --fail ${endpoint})" == 200 ]] || [ "$COUNTER" -eq "$TIMEOUT" ]; do
+   additional_args=()
+   if [[ $endpoint == *"solr"* ]]; then
+     additional_args+=(-H "X-Alfresco-Search-Secret: secret")
+   fi
+
+   until [[ "$(curl --output /dev/null -w ''%{http_code}'' "${additional_args[@]}" --silent --head --fail ${endpoint})" == 200 ]] || [ "$COUNTER" -eq "$TIMEOUT" ]; do
       printf '.'
       sleep $WAIT_INTERVAL
       COUNTER=$(($COUNTER+$WAIT_INTERVAL))

search-services/README.md

@@ -158,10 +158,10 @@ $ unzip alfresco-search-services-*.zip
 $ cd alfresco-search-services
 ```
 
-Change default Alfresco Communication protocol to `none`.
+Change default Alfresco Communication protocol to `none`, and set `alfresco.allowUnauthenticatedSolrEndpoint` to `true`:
 
 ```bash
-$ sed -i 's/alfresco.secureComms=https/alfresco.secureComms=none/' solrhome/templates/rerank/conf/solrcore.properties
+$ sed -i 's/alfresco.secureComms=https/alfresco.secureComms=none\nalfresco.allowUnauthenticatedSolrEndpoint=true/' solrhome/templates/rerank/conf/solrcore.properties
 ```
 
 *Note* Above line is written in GNU sed, you can use `gsed` from Mac OS X or just edit the file with a Text Editor.

search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedAuthPlugin.java

@@ -2,7 +2,7 @@
  * #%L
  * Alfresco Search Services
  * %%
- * Copyright (C) 2005 - 2020 Alfresco Software Limited
+ * Copyright (C) 2005 - 2022 Alfresco Software Limited
  * %%
  * This file is part of the Alfresco software. 
  * If the software was purchased under a paid Alfresco license, the terms of 
@@ -26,6 +26,8 @@
 
 package org.alfresco.solr.security;
 
+import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECURE_COMMS_PROPERTY;
+
 import java.io.IOException;
 import java.util.Map;
 import java.util.Objects;
@@ -49,6 +51,8 @@ import org.apache.solr.security.AuthenticationPlugin;
 public class SecretSharedAuthPlugin extends AuthenticationPlugin
 {
 
+    private static final String SECURE_COMMS_NONE = "none";
+
     /**
      * Verify that request header includes "secret" word when using "secret" communication method.
      * "alfresco.secureComms.secret" value is expected as Java environment variable.
@@ -69,10 +73,17 @@ public class SecretSharedAuthPlugin extends AuthenticationPlugin
                 return true;
             }
 
-            HttpServletResponse httpResponse = (HttpServletResponse) response;
+            String errorMessage = "Authentication failure: \"" + SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY
-            httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN,
+                + "\" method has been selected, use the right request header with the secret word";
-                        "Authentication failure: \"" + SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY
+            setErrorResponse(response, errorMessage);
-                                    + "\" method has been selected, use the right request header with the secret word");
+            return false;
+        }
+        else if (SECURE_COMMS_NONE.equals(SecretSharedPropertyCollector.getCommsMethod())
+            && !SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint())
+        {
+            String errorMessage = "Authentication failure: \"" + SECURE_COMMS_PROPERTY
+                + "=none\" is no longer supported. Please use \"https\" or \"secret\" instead.";
+            setErrorResponse(response, errorMessage);
             return false;
         }
 
@@ -81,6 +92,12 @@ public class SecretSharedAuthPlugin extends AuthenticationPlugin
 
     }
 
+    private void setErrorResponse(ServletResponse response, String errorMessage) throws IOException
+    {
+        HttpServletResponse httpResponse = (HttpServletResponse) response;
+        httpResponse.sendError(HttpServletResponse.SC_FORBIDDEN, errorMessage);
+    }
+
     @Override
     public void init(Map<String, Object> parameters)
     {

search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyCollector.java

@@ -26,13 +26,21 @@
 
 package org.alfresco.solr.security;
 
+import static java.util.function.Predicate.not;
+
 import org.alfresco.httpclient.HttpClientFactory;
 import org.alfresco.solr.AlfrescoSolrDataModel;
 import org.alfresco.solr.config.ConfigUtil;
 
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiFunction;
+import java.util.function.Consumer;
 
 /**
  * Provides property values for Alfresco Communication using "secret" method:
@@ -50,10 +58,23 @@ public class SecretSharedPropertyCollector
     // Property names for "secret" communication method
     static final String SECURE_COMMS_PROPERTY = "alfresco.secureComms";
     static final String SHARED_SECRET = "alfresco.secureComms.secret";
+    static final String ALLOW_UNAUTHENTICATED_SOLR_PROPERTY = "alfresco.allowUnauthenticatedSolrEndpoint";
     private static final String SHARED_SECRET_HEADER = "alfresco.secureComms.secret.header";
 
-    // Save communication method as static value in order to improve performance 
+    // Memoize read properties to improve performance
-    static String commsMethod;
+    static final Map<String, String> PROPS_CACHE = new ConcurrentHashMap<>();
+    // Ordered list of property location functions
+    private static final ArrayList<BiFunction<String, String, Set<String>>> PROPERTY_LOCATORS = new ArrayList<>();
+
+    static
+    {
+        // Environment variables
+        PROPERTY_LOCATORS.add((name, defaultValue) -> toSet(ConfigUtil.locateProperty(name, null)));
+        // Shared configuration (shared.properties file)
+        PROPERTY_LOCATORS.add((name, defaultValue) -> toSet(AlfrescoSolrDataModel.getCommonConfig().getProperty(name)));
+        // Configuration for each deployed SOLR Core
+        PROPERTY_LOCATORS.add((name, defaultValue) -> SecretSharedPropertyHelper.getPropertyFromCores(name, defaultValue));
+    }
 
     /**
      * Check if communications method is "secret"
@@ -65,50 +86,63 @@ public class SecretSharedPropertyCollector
                     SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY);
     }
 
+    /**
+     * Check if unauthenticated Solr access is allowed
+     * @return true if unauthenticated Solr access is allowed
+     */
+    public static boolean isAllowUnauthenticatedSolrEndpoint()
+    {
+        return Boolean.parseBoolean(PROPS_CACHE.computeIfAbsent(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY,
+            key -> getProperty(key, "false")));
+    }
+
     /**
      * Get communication method from environment variables, shared properties or core properties.
      * @return Communication method: none, https, secret
      */
     static String getCommsMethod()
     {
-        if (commsMethod == null)
+        return PROPS_CACHE.computeIfAbsent(SECURE_COMMS_PROPERTY,
+            key -> getProperty(key, "none", uniqueSecureCommsValidator()));
+    }
+
+    private static String getProperty(String name, String defaultValue)
+    {
+        return getProperty(name, defaultValue, null);
+    }
+
+    private static String getProperty(String name, String defaultValue, Consumer<Set<String>> propertySetValidator)
+    {
+        // Loop orderly through the property locators until the property is found
+        Set<String> propertySet = PROPERTY_LOCATORS.stream()
+            .map(propertyLocator -> propertyLocator.apply(name, defaultValue))
+            .filter(not(Set::isEmpty))
+            .findFirst()
+            .orElse(Set.of());
+
+        if (propertySetValidator != null)
         {
-
+            // Run the propertySetValidator to eg. verify value uniqueness among multiple cores
-            // Environment variable
+            propertySetValidator.accept(propertySet);
-            commsMethod = ConfigUtil.locateProperty(SECURE_COMMS_PROPERTY, null);
-
-            if (commsMethod == null)
-            {
-                // Shared configuration (shared.properties file)
-                commsMethod = AlfrescoSolrDataModel.getCommonConfig().getProperty(SECURE_COMMS_PROPERTY);
-
-                if (commsMethod == null)
-                {
-                    // Get configuration from deployed SOLR Cores
-                    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);
-                    }
-
-                    return commsMethod =
-                            secureCommsSet.isEmpty()
-                                    ? null
-                                    : secureCommsSet.iterator().next();
-
-                }
-            }
         }
 
-        return commsMethod;
+        return propertySet.isEmpty() ? null : propertySet.iterator().next();
+    }
 
+    private static Consumer<Set<String>> uniqueSecureCommsValidator()
+    {
+        // 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.
+        return secureCommsSet -> {
+            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);
+            }
+        };
     }
 
     /**
@@ -168,4 +202,16 @@ public class SecretSharedPropertyCollector
         return properties;
     }
 
+    private static Set<String> toSet(String value)
+    {
+        Set<String> propertySet = new HashSet<>();
+
+        if (value != null)
+        {
+            propertySet.add(value);
+        }
+
+        return propertySet;
+    }
+
 }

search-services/alfresco-search/src/main/java/org/alfresco/solr/security/SecretSharedPropertyHelper.java

@@ -58,11 +58,12 @@ class SecretSharedPropertyHelper
             };
 
     /**
-     * Read different values of "alfresco.secureComms" property from every "solrcore.properties" files.
+     * Read different values of the specified property from every "solrcore.properties" file.
-     *
+     * @param name The name of the property to read
+     * @param defaultValue The default value for the given property
      * @return List of different communication methods declared in SOLR Cores.
      */
-    static Set<String> getCommsFromCores()
+    static Set<String> getPropertyFromCores(String name, String defaultValue)
     {
         try (Stream<Path> walk = Files.walk(Paths.get(SolrResourceLoader.locateSolrHome().toString())))
         {
@@ -74,7 +75,7 @@ class SecretSharedPropertyHelper
 
             return solrCorePropertiesFiles.stream()
                     .map(toProperties)
-                    .map(properties -> properties.getProperty(SECURE_COMMS_PROPERTY, "none"))
+                    .map(properties -> properties.getProperty(name, defaultValue))
                     .collect(toSet());
         }
         catch (IOException e)

search-services/alfresco-search/src/test/java/org/alfresco/solr/security/SecretSharedPropertyCollectorTest.java

@@ -35,6 +35,9 @@ import java.util.Properties;
 import java.util.Set;
 
 import static java.util.Collections.emptySet;
+
+import static org.alfresco.solr.security.SecretSharedPropertyCollector.ALLOW_UNAUTHENTICATED_SOLR_PROPERTY;
+import static org.alfresco.solr.security.SecretSharedPropertyCollector.PROPS_CACHE;
 import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECRET_SHARED_METHOD_KEY;
 import static org.alfresco.solr.security.SecretSharedPropertyCollector.SECURE_COMMS_PROPERTY;
 import static org.alfresco.solr.security.SecretSharedPropertyCollector.SHARED_SECRET;
@@ -51,22 +54,32 @@ public class SecretSharedPropertyCollectorTest
     private static final String SET_THROUGH_ALFRESCO_COMMON_CONFIG = "aCommsMethod_SetThroughAlfrescoCommonConfig";
     private static final String COMMS_METHOD_FROM_SOLRCORE = "aCommsMethod_FromSolrCore";
     private static final String SECRET_VALUE = "my-secret";
+    private static final String SECURE_COMMS_NONE = "none";
+    private static final String TRUE = "true";
+    private static final String FALSE = "false";
+
+    private static final Set<String> PROPS_TO_CLEAR = Set.of(SHARED_SECRET, SECURE_COMMS_PROPERTY, ALLOW_UNAUTHENTICATED_SOLR_PROPERTY);
 
     @Before
     public void setUp()
     {
-        SecretSharedPropertyCollector.commsMethod = null;
+        PROPS_CACHE.clear();
-        assertNull(System.getProperty(SHARED_SECRET));
+
-        assertNull(System.getProperty(SECURE_COMMS_PROPERTY));
+        for (String property : PROPS_TO_CLEAR)
-        assertNull(AlfrescoSolrDataModel.getCommonConfig().getProperty(SECURE_COMMS_PROPERTY));
+        {
+            assertNull(System.getProperty(property));
+            assertNull(AlfrescoSolrDataModel.getCommonConfig().getProperty(property));
+        }
     }
 
     @After
     public void tearDown()
     {
-        System.clearProperty(SHARED_SECRET);
+        for (String property : PROPS_TO_CLEAR)
-        System.clearProperty(SECURE_COMMS_PROPERTY);
+        {
-        AlfrescoSolrDataModel.getCommonConfig().remove(SECURE_COMMS_PROPERTY);
+            System.clearProperty(property);
+            AlfrescoSolrDataModel.getCommonConfig().remove(property);
+        }
     }
 
     @Test
@@ -82,10 +95,87 @@ public class SecretSharedPropertyCollectorTest
         SecretSharedPropertyCollector.getSecret();
     }
 
+    @Test
+    public void allowUnauthenticatedSolrIsNotSet_shouldReturnFalse()
+    {
+        try(MockedStatic<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
+        {
+            mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE))
+                .thenReturn(emptySet());
+            assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+        }
+    }
+
+    @Test
+    public void allowUnauthenticatedSolrIsTrueThroughSystemProperty_shouldReturnTrue()
+    {
+        System.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, TRUE);
+        assertTrue(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+    }
+
+    @Test
+    public void allowUnauthenticatedSolrIsFalseThroughSystemProperty_shouldReturnFalse()
+    {
+        System.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE);
+        assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+    }
+
+    @Test
+    public void allowUnauthenticatedSolrIsTrueThroughAlfrescoProperties_shouldReturnTrue()
+    {
+        try(MockedStatic<AlfrescoSolrDataModel> mock = mockStatic(AlfrescoSolrDataModel.class))
+        {
+            var alfrescoCommonConfig = new Properties();
+            alfrescoCommonConfig.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, TRUE);
+
+            mock.when(AlfrescoSolrDataModel::getCommonConfig).thenReturn(alfrescoCommonConfig);
+
+            assertTrue(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+        }
+    }
+
+    @Test
+    public void allowUnauthenticatedSolrIsFalseThroughAlfrescoProperties_shouldReturnFalse()
+    {
+        try(MockedStatic<AlfrescoSolrDataModel> mock = mockStatic(AlfrescoSolrDataModel.class))
+        {
+            var alfrescoCommonConfig = new Properties();
+            alfrescoCommonConfig.setProperty(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE);
+
+            mock.when(AlfrescoSolrDataModel::getCommonConfig).thenReturn(alfrescoCommonConfig);
+
+            assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+        }
+    }
+
+    @Test
+    public void allowUnauthenticatedSolrIsTrueThroughSolrCores_shouldReturnTrue()
+    {
+        try(MockedStatic<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
+        {
+            mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE))
+                .thenReturn(Set.of(TRUE));
+
+            assertTrue(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+        }
+    }
+
+    @Test
+    public void allowUnauthenticatedSolrIsFalseThroughSolrCores_shouldReturnFalse()
+    {
+        try(MockedStatic<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
+        {
+            mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(ALLOW_UNAUTHENTICATED_SOLR_PROPERTY, FALSE))
+                .thenReturn(Set.of(FALSE));
+
+            assertFalse(SecretSharedPropertyCollector.isAllowUnauthenticatedSolrEndpoint());
+        }
+    }
+
     @Test
     public void commsMethodIsNotNull_shouldReturnThatValue()
     {
-        SecretSharedPropertyCollector.commsMethod = A_COMMS_METHOD;
+        PROPS_CACHE.put(SECURE_COMMS_PROPERTY, A_COMMS_METHOD);
         assertEquals(A_COMMS_METHOD, SecretSharedPropertyCollector.getCommsMethod());
 
         assertFalse(SecretSharedPropertyCollector.isCommsSecretShared());
@@ -94,7 +184,7 @@ public class SecretSharedPropertyCollectorTest
     @Test
     public void commsMethodIsNotNullAndIsSecret_shouldReturnThatValue()
     {
-        SecretSharedPropertyCollector.commsMethod = SECRET_SHARED_METHOD_KEY;
+        PROPS_CACHE.put(SECURE_COMMS_PROPERTY, SECRET_SHARED_METHOD_KEY);
         assertEquals(SECRET_SHARED_METHOD_KEY, SecretSharedPropertyCollector.getCommsMethod());
 
         assertTrue(SecretSharedPropertyCollector.isCommsSecretShared());
@@ -138,7 +228,8 @@ public class SecretSharedPropertyCollectorTest
     {
         try(MockedStatic<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
         {
-            mock.when(SecretSharedPropertyHelper::getCommsFromCores).thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE));
+            mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(SECURE_COMMS_PROPERTY, SECURE_COMMS_NONE))
+                .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE));
             assertEquals(COMMS_METHOD_FROM_SOLRCORE, SecretSharedPropertyCollector.getCommsMethod());
 
             assertFalse(SecretSharedPropertyCollector.isCommsSecretShared());
@@ -157,7 +248,8 @@ public class SecretSharedPropertyCollectorTest
     {
         try(MockedStatic<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
         {
-            mock.when(SecretSharedPropertyHelper::getCommsFromCores).thenReturn(emptySet());
+            mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(SECURE_COMMS_PROPERTY, SECURE_COMMS_NONE))
+                .thenReturn(emptySet());
             assertNull(SecretSharedPropertyCollector.getCommsMethod());
 
             assertFalse(SecretSharedPropertyCollector.isCommsSecretShared());
@@ -173,8 +265,8 @@ public class SecretSharedPropertyCollectorTest
     {
         try(MockedStatic<SecretSharedPropertyHelper> mock = mockStatic(SecretSharedPropertyHelper.class))
         {
-            mock.when(SecretSharedPropertyHelper::getCommsFromCores)
+            mock.when(() -> SecretSharedPropertyHelper.getPropertyFromCores(SECURE_COMMS_PROPERTY, SECURE_COMMS_NONE))
-                    .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE, SECRET_SHARED_METHOD_KEY));
+                .thenReturn(Set.of(COMMS_METHOD_FROM_SOLRCORE, SECRET_SHARED_METHOD_KEY));
 
             SecretSharedPropertyCollector.getCommsMethod();
         }

search-services/packaging/src/docker/search_config_setup.sh

@@ -90,7 +90,6 @@ if [[ ! -z "$SOLR_JAVA_MEM" ]]; then
 fi
 
 # By default Docker Image is using TLS Mutual Authentication (SSL) for communications with Repository
-# Plain HTTP can be enabled by setting ALFRESCO_SECURE_COMMS to 'none'
 # Plain HTTP with a secret word in the request header can be enabled by setting ALFRESCO_SECURE_COMMS to 'secret',
 # the secret word should be defined as a JVM argument like so: JAVA_TOOL_OPTIONS="-Dalfresco.secureComms.secret=my-secret-value"
 case "$ALFRESCO_SECURE_COMMS" in
@@ -103,15 +102,6 @@ case "$ALFRESCO_SECURE_COMMS" in
          sed -i "s/alfresco.secureComms=https/alfresco.secureComms=secret\n/" ${PWD}/solrhome/archive/conf/solrcore.properties
      fi
    ;;
-   none)
-     sed -i "s/alfresco.secureComms=https/alfresco.secureComms=none\n/" $SOLR_RERANK_CORE_FILE $SOLR_NORERANK_CORE_FILE
-     if [[ -f ${PWD}/solrhome/alfresco/conf/solrcore.properties ]]; then
-         sed -i "s/alfresco.secureComms=https/alfresco.secureComms=none\n/" ${PWD}/solrhome/alfresco/conf/solrcore.properties
-     fi
-     if [[ -f ${PWD}/solrhome/archive/conf/solrcore.properties ]]; then
-         sed -i "s/alfresco.secureComms=https/alfresco.secureComms=none\n/" ${PWD}/solrhome/archive/conf/solrcore.properties
-     fi
-   ;;
    https|'')
    ;;
    *)