SEARCH-187: Only permit certain properties for updating

search-services/alfresco-solr/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java

@@ -535,7 +535,7 @@ public class AlfrescoCoreAdminHandler extends CoreAdminHandler
         try {
 
             File config = new File(AlfrescoSolrDataModel.getResourceDirectory(), AlfrescoSolrDataModel.SHARED_PROPERTIES);
-            updatePropertiesFile(params, config);
+            updateSharedProperties(params, config);
 
             coreContainer.getCores().forEach(aCore -> coreContainer.reload(aCore.getName()));
 
@@ -570,7 +570,7 @@ public class AlfrescoCoreAdminHandler extends CoreAdminHandler
 
                 String  configLocaltion = core.getResourceLoader().getConfigDir();
                 File config = new File(configLocaltion, "solrcore.properties");
-                updatePropertiesFile(params, config);
+                updatePropertiesFile(params, config, null);
 
                 coreContainer.reload(coreName);
                 

search-services/alfresco-solr/src/main/java/org/alfresco/solr/HandlerOfResources.java

@@ -22,11 +22,14 @@
 package org.alfresco.solr;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.core.CoreContainer;
 
 import java.io.*;
+import java.util.Arrays;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Properties;
 
 /**
@@ -34,6 +37,10 @@ import java.util.Properties;
  */
 public class HandlerOfResources {
 
+    public static final List<String> DISALLOWED_SHARED_UPDATES = Arrays.asList("alfresco.identifier.property.",
+            "alfresco.suggestable.property.",
+            "alfresco.cross.locale.property.",
+            "alfresco.cross.locale.datatype.");
     /**
      * Opens an InputStream
      * @param solrHome
@@ -74,6 +81,21 @@ public class HandlerOfResources {
         return is;
     }
 
+    /**
+     * Updates a properties file using the SolrParams
+     *
+     * @param params
+     * @param config
+     * @throws IOException
+     */
+    public static void updateSharedProperties(SolrParams params, File config) throws IOException {
+        try {
+            updatePropertiesFile(params,config, DISALLOWED_SHARED_UPDATES);
+        } catch (IllegalArgumentException e) {
+            throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+               "For shared properties you are not allowed to update any of the following "+DISALLOWED_SHARED_UPDATES);
+        }
+    }
 
     /**
      * Updates a properties file using the SolrParams
@@ -82,7 +104,7 @@ public class HandlerOfResources {
      * @param config
      * @throws IOException
      */
-    public static void updatePropertiesFile(SolrParams params, File config) throws IOException {
+    public static void updatePropertiesFile(SolrParams params, File config, List<String> disallowed) throws IOException {
         // fix configuration properties
         Properties properties = new Properties();
         properties.load(new FileInputStream(config));
@@ -91,12 +113,36 @@ public class HandlerOfResources {
         //Allow the properties to be overidden via url params
         if (extraProperties != null && !extraProperties.isEmpty())
         {
+            if (!allowedProperties(extraProperties, disallowed))
+            {
+                throw new IllegalArgumentException("You are not permitted to update these properties.");
+            }
             properties.putAll(extraProperties);
         }
 
         properties.store(new FileOutputStream(config), null);
     }
 
+    /**
+     * Checks a list of properties to see if they are allowed
+     * It actually checks if the property starts with any value in the List<String> disallowed.
+     * @param toCheck
+     * @param disallowed
+     * @return
+     */
+    public static boolean allowedProperties(Properties toCheck, List<String> disallowed)
+    {
+        if (toCheck == null || toCheck.isEmpty() || disallowed == null || disallowed.isEmpty()) return true;
+
+        for (Object key: toCheck.keySet())
+        {
+            for (String prop :disallowed) {
+                if (key.toString().startsWith(prop)) return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * Extracts Custom Properties from SolrParams
      * @param params

search-services/alfresco-solr/src/test/java/org/alfresco/solr/CoresCreateUpdateDistributedTest.java

@@ -21,6 +21,7 @@ package org.alfresco.solr;
 import org.alfresco.service.cmr.repository.StoreRef;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -107,6 +108,12 @@ public class CoresCreateUpdateDistributedTest extends AbstractAlfrescoDistribute
         Properties props = AlfrescoSolrDataModel.getCommonConfig();
         String solrHost = props.getProperty("solr.host");
         assertFalse(props.containsKey("new.property"));
+        try {
+            updateShared(coreAdminHandler,"property.solr.host", "myhost", "property.new.property", "catchup", "property.alfresco.identifier.property.0", "not_this_time");
+            assertFalse(true); //Should not get here
+        } catch (SolrException se) {
+            assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, se.code());
+        }
         updateShared(coreAdminHandler,"property.solr.host", "myhost", "property.new.property", "catchup");
         props = AlfrescoSolrDataModel.getCommonConfig();
         assertEquals(props.getProperty("new.property"), "catchup");

search-services/alfresco-solr/src/test/java/org/alfresco/solr/HandlerOfResourcesTest.java

@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2005 - 2016 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/>.
+ */
+package org.alfresco.solr;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.lang.reflect.Array;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Properties;
+
+import static org.junit.Assert.*;
+
+/**
+ * Tests HandlerOfResources
+ */
+public class HandlerOfResourcesTest {
+
+
+    @Test
+    public void allowedPropertiesTest() throws Exception {
+        assertTrue(HandlerOfResources.allowedProperties(null,null));
+        assertTrue(HandlerOfResources.allowedProperties(new Properties(),null));
+        assertTrue(HandlerOfResources.allowedProperties(new Properties(), new ArrayList<String>()));
+
+        Properties props = new Properties();
+        props.setProperty("king", "kong");
+        props.setProperty("barbie", "doll");
+        assertFalse(HandlerOfResources.allowedProperties(props, Arrays.asList("bar")));
+        assertTrue( HandlerOfResources.allowedProperties(props, Arrays.asList("bark")));
+
+        assertTrue(HandlerOfResources.allowedProperties(props, HandlerOfResources.DISALLOWED_SHARED_UPDATES));
+
+        props.setProperty("solr.host", "me");
+        props.setProperty("solr.port", "233");
+        assertTrue(HandlerOfResources.allowedProperties(props, HandlerOfResources.DISALLOWED_SHARED_UPDATES));
+
+        props.setProperty("alfresco.identifier.property.0", "xy");
+        assertFalse(HandlerOfResources.allowedProperties(props, HandlerOfResources.DISALLOWED_SHARED_UPDATES));
+        props.remove("alfresco.identifier.property.0");
+
+        props.setProperty("alfresco.suggestable.property.1", "xy");
+        assertFalse(HandlerOfResources.allowedProperties(props, HandlerOfResources.DISALLOWED_SHARED_UPDATES));
+        props.remove("alfresco.suggestable.property.1");
+
+        props.setProperty("alfresco.cross.locale.property.0", "xy");
+        assertFalse(HandlerOfResources.allowedProperties(props, HandlerOfResources.DISALLOWED_SHARED_UPDATES));
+        props.remove("alfresco.cross.locale.property.0");
+
+        props.setProperty("alfresco.cross.locale.datatype.2", "xy");
+        assertFalse(HandlerOfResources.allowedProperties(props, HandlerOfResources.DISALLOWED_SHARED_UPDATES));
+        props.remove("alfresco.cross.locale.datatype.2");
+    }
+
+}