From c41507d6db1fb03e9df36f94aa43f8a007709f8b Mon Sep 17 00:00:00 2001 From: Gethin James Date: Mon, 26 Sep 2016 10:17:54 +0200 Subject: [PATCH] SEARCH-187: Only permit certain properties for updating --- .../solr/AlfrescoCoreAdminHandler.java | 4 +- .../org/alfresco/solr/HandlerOfResources.java | 48 +++++++++++- .../CoresCreateUpdateDistributedTest.java | 7 ++ .../alfresco/solr/HandlerOfResourcesTest.java | 75 +++++++++++++++++++ 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 search-services/alfresco-solr/src/test/java/org/alfresco/solr/HandlerOfResourcesTest.java diff --git a/search-services/alfresco-solr/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java b/search-services/alfresco-solr/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java index b0faec9ed..6d9bbdd8c 100644 --- a/search-services/alfresco-solr/src/main/java/org/alfresco/solr/AlfrescoCoreAdminHandler.java +++ b/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); diff --git a/search-services/alfresco-solr/src/main/java/org/alfresco/solr/HandlerOfResources.java b/search-services/alfresco-solr/src/main/java/org/alfresco/solr/HandlerOfResources.java index c2f18be6e..2be1af338 100644 --- a/search-services/alfresco-solr/src/main/java/org/alfresco/solr/HandlerOfResources.java +++ b/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 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 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 disallowed. + * @param toCheck + * @param disallowed + * @return + */ + public static boolean allowedProperties(Properties toCheck, List 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 diff --git a/search-services/alfresco-solr/src/test/java/org/alfresco/solr/CoresCreateUpdateDistributedTest.java b/search-services/alfresco-solr/src/test/java/org/alfresco/solr/CoresCreateUpdateDistributedTest.java index c2e4d9268..1a59049fd 100644 --- a/search-services/alfresco-solr/src/test/java/org/alfresco/solr/CoresCreateUpdateDistributedTest.java +++ b/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"); diff --git a/search-services/alfresco-solr/src/test/java/org/alfresco/solr/HandlerOfResourcesTest.java b/search-services/alfresco-solr/src/test/java/org/alfresco/solr/HandlerOfResourcesTest.java new file mode 100644 index 000000000..971f7d33c --- /dev/null +++ b/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 . + */ +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())); + + 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"); + } + +} \ No newline at end of file