Merge pull request #1376 from GeoWebCache/backport-1364-to-1.26.x

[Backport 1.26.x] [GWC-1363] Support Environment Parametrization for WMSLayer Credentials

Author: groldan

geowebcache/core/pom.xml

@@ -205,6 +205,12 @@
       <artifactId>awaitility</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <!-- used for tests that require environment variables -->
+      <groupId>com.github.stefanbirkner</groupId>
+      <artifactId>system-rules</artifactId>
+      <scope>test</scope>
+    </dependency>
 
     <!-- Thijs Brentjens: for security fixes, OWASP library-->
     <dependency>

geowebcache/core/src/main/java/org/geowebcache/GeoWebCacheEnvironment.java

@@ -23,7 +23,6 @@
 import org.springframework.beans.factory.config.PlaceholderConfigurerSupport;
 import org.springframework.core.Constants;
 import org.springframework.util.PropertyPlaceholderHelper;
-import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver;
 
 /**
  * Utility class uses to process GeoWebCache configuration workflow through external environment variables.
@@ -46,7 +45,7 @@
 public class GeoWebCacheEnvironment {
 
     /** logger */
-    public static Logger LOGGER = Logging.getLogger(GeoWebCacheEnvironment.class.getName());
+    public static final Logger LOGGER = Logging.getLogger(GeoWebCacheEnvironment.class.getName());
 
     private static final Constants constants = new Constants(PlaceholderConfigurerSupport.class);
 
@@ -55,9 +54,13 @@ public class GeoWebCacheEnvironment {
      * placeholders translation.
      *
      * <p>Default to FALSE
+     *
+     * @deprecated a static final variable does prevents change during runtime and hinders testing. Use
+     *     {@link #isAllowEnvParametrization()} instead.
      */
+    @Deprecated(forRemoval = true)
     public static final boolean ALLOW_ENV_PARAMETRIZATION =
-            Boolean.valueOf(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
+            Boolean.parseBoolean(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
 
     private static final String nullValue = "null";
 
@@ -67,10 +70,17 @@ public class GeoWebCacheEnvironment {
             constants.asString("DEFAULT_VALUE_SEPARATOR"),
             true);
 
-    private final PlaceholderResolver resolver = placeholderName -> resolvePlaceholder(placeholderName);
-
     private Properties props;
 
+    /**
+     * Determines if the {@code ALLOW_ENV_PARAMETRIZATION} environment variable is set to {@code true} and hence
+     * variable variable substitution of configuration parameters using <code>${}</code> place holders can be performed
+     * through {@link #resolveValue(Object)}.
+     */
+    public boolean isAllowEnvParametrization() {
+        return Boolean.parseBoolean(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
+    }
+
     /**
      * Internal "props" getter method.
      *
@@ -107,7 +117,7 @@ protected String resolveSystemProperty(String key) {
                 value = System.getenv(key);
             }
             return value;
-        } catch (Throwable ex) {
+        } catch (RuntimeException ex) {
             if (LOGGER.isLoggable(Level.FINE)) {
                 LOGGER.fine("Could not access system property '" + key + "': " + ex);
             }
@@ -116,7 +126,7 @@ protected String resolveSystemProperty(String key) {
     }
 
     protected String resolveStringValue(String strVal) throws BeansException {
-        String resolved = this.helper.replacePlaceholders(strVal, this.resolver);
+        String resolved = this.helper.replacePlaceholders(strVal, this::resolvePlaceholder);
 
         return (resolved.equals(nullValue) ? null : resolved);
     }
@@ -127,19 +137,17 @@ protected String resolveStringValue(String strVal) throws BeansException {
      * <p>The method first looks for System variables which take precedence on local ones, then into internal props
      * injected through the applicationContext.
      */
-    public Object resolveValue(Object value) {
-        if (value != null) {
-            if (value instanceof String) {
-                return resolveStringValue((String) value);
-            }
+    @SuppressWarnings("unchecked")
+    public <T> T resolveValue(T value) {
+        if (value instanceof String) {
+            return (T) resolveStringValue((String) value);
         }
 
         return value;
     }
 
     private String resolveValueIfEnabled(String value) {
-        if (ALLOW_ENV_PARAMETRIZATION) return (String) resolveValue(value);
-        else return value;
+        return isAllowEnvParametrization() ? resolveValue(value) : value;
     }
 
     private boolean validateBoolean(String value) {

geowebcache/core/src/main/java/org/geowebcache/config/XMLConfiguration.java

@@ -53,6 +53,7 @@
 import javax.xml.validation.SchemaFactory;
 import javax.xml.validation.Validator;
 import org.geotools.util.logging.Logging;
+import org.geowebcache.GeoWebCacheEnvironment;
 import org.geowebcache.GeoWebCacheException;
 import org.geowebcache.GeoWebCacheExtensions;
 import org.geowebcache.config.ContextualConfigurationProvider.Context;
@@ -282,6 +283,8 @@ public void setDefaultValues(TileLayer layer) {
                 sourceHelper = new WMSHttpHelper(null, null, proxyUrl);
                 log.fine("Not using HTTP credentials for " + wl.getName());
             }
+            GeoWebCacheEnvironment gwcEnv = GeoWebCacheExtensions.bean(GeoWebCacheEnvironment.class);
+            sourceHelper.setGeoWebCacheEnvironment(gwcEnv);
 
             wl.setSourceHelper(sourceHelper);
             wl.setLockProvider(getGwcConfig().getLockProvider());

geowebcache/core/src/main/java/org/geowebcache/layer/wms/WMSHttpHelper.java

@@ -13,6 +13,7 @@
  */
 package org.geowebcache.layer.wms;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UnsupportedEncodingException;
@@ -29,14 +30,17 @@
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
+import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicNameValuePair;
 import org.geotools.util.logging.Logging;
+import org.geowebcache.GeoWebCacheEnvironment;
 import org.geowebcache.GeoWebCacheException;
+import org.geowebcache.GeoWebCacheExtensions;
 import org.geowebcache.io.Resource;
 import org.geowebcache.layer.TileResponseReceiver;
 import org.geowebcache.mime.ErrorMime;
@@ -48,17 +52,39 @@
 import org.geowebcache.util.URLs;
 import org.springframework.util.Assert;
 
-/** This class is a wrapper for HTTP interaction with WMS backend */
+/**
+ * Helper class for HTTP interactions of {@link WMSLayer} with a WMS backend.
+ *
+ * <p>HTTP Basic Auth username and password supplied to the constructor support environment parametrization.
+ *
+ * @see GeoWebCacheEnvironment#isAllowEnvParametrization()
+ * @see GeoWebCacheEnvironment#resolveValue(Object)
+ */
 public class WMSHttpHelper extends WMSSourceHelper {
     private static final Logger log = Logging.getLogger(WMSHttpHelper.class.getName());
 
+    /**
+     * Used by {@link #getResolvedHttpUsername()} and {@link #getResolvedHttpPassword()} to
+     * {@link GeoWebCacheEnvironment#resolveValue resolve} the actual values against environment variables when
+     * {@link GeoWebCacheEnvironment#isAllowEnvParametrization() ALLOW_ENV_PARAMETRIZATION} is enabled.
+     */
+    protected GeoWebCacheEnvironment gwcEnv;
+
     private final URL proxyUrl;
 
+    /**
+     * HTTP Basic Auth username, might be de-referenced using environment variable substitution. Always access it
+     * through {@link #getResolvedHttpUsername()}
+     */
     private final String httpUsername;
 
+    /**
+     * HTTP Basic Auth password, might be de-referenced using environment variable substitution. Always access it
+     * through {@link #getResolvedHttpUsername()}
+     */
     private final String httpPassword;
 
-    protected volatile HttpClient client;
+    protected HttpClient client;
 
     public WMSHttpHelper() {
         this(null, null, null);
@@ -71,23 +97,75 @@ public WMSHttpHelper(String httpUsername, String httpPassword, URL proxyUrl) {
         this.proxyUrl = proxyUrl;
     }
 
-    HttpClient getHttpClient() {
-        if (client == null) {
-            synchronized (this) {
-                if (client != null) {
-                    return client;
-                }
+    /**
+     * Used by {@link #executeRequest}
+     *
+     * @return the actual http username to use when executing requests
+     */
+    public String getResolvedHttpUsername() {
+        return resolvePlaceHolders(httpUsername);
+    }
+
+    /**
+     * Used by {@link #executeRequest}
+     *
+     * @return the actual http password to use when executing requests
+     */
+    public String getResolvedHttpPassword() {
+        return resolvePlaceHolders(httpPassword);
+    }
+
+    /**
+     * Assigns the environment variable {@link GeoWebCacheEnvironment#resolveValue resolver} to perform variable
+     * substitution against the configured http username and password during {@link #executeRequest}
+     *
+     * <p>When unset, a bean of this type will be looked up through {@link GeoWebCacheExtensions#bean(Class)}
+     */
+    public void setGeoWebCacheEnvironment(GeoWebCacheEnvironment gwcEnv) {
+        this.gwcEnv = gwcEnv;
+    }
 
-                HttpClientBuilder builder = new HttpClientBuilder(
-                        null, getBackendTimeout(), httpUsername, httpPassword, proxyUrl, getConcurrency());
+    private String resolvePlaceHolders(String value) {
+        GeoWebCacheEnvironment env = getEnvironment();
+        return env != null && env.isAllowEnvParametrization() ? env.resolveValue(value) : value;
+    }
 
-                client = builder.buildClient();
-            }
+    private GeoWebCacheEnvironment getEnvironment() {
+        GeoWebCacheEnvironment env = this.gwcEnv;
+        if (env == null) {
+            env = GeoWebCacheExtensions.bean(GeoWebCacheEnvironment.class);
+            this.gwcEnv = env;
         }
+        return env;
+    }
 
+    /**
+     * Note: synchronizing the entire method avoids double-checked locking altogether. Modern JVMs optimize this and
+     * reduce the overhead of synchronization. Otherwise PMD complains with a `Double checked locking is not thread safe
+     * in Java.` error.
+     */
+    synchronized HttpClient getHttpClient() {
+        if (client == null) {
+            int backendTimeout = getBackendTimeout();
+            String user = getResolvedHttpUsername();
+            String password = getResolvedHttpPassword();
+            URL proxy = proxyUrl;
+            int concurrency = getConcurrency();
+            client = buildHttpClient(backendTimeout, user, password, proxy, concurrency);
+        }
         return client;
     }
 
+    @VisibleForTesting
+    HttpClient buildHttpClient(int backendTimeout, String username, String password, URL proxy, int concurrency) {
+
+        URL serverUrl = null;
+        HttpClientBuilder builder =
+                new HttpClientBuilder(serverUrl, backendTimeout, username, password, proxy, concurrency);
+
+        return builder.buildClient();
+    }
+
     /** Loops over the different backends, tries the request */
     @Override
     protected void makeRequest(
@@ -319,7 +397,13 @@ public HttpResponse executeRequest(
         if (log.isLoggable(Level.FINER)) {
             log.finer(method.toString());
         }
-        return getHttpClient().execute(method);
+        HttpClient httpClient = getHttpClient();
+        return execute(httpClient, method);
+    }
+
+    @VisibleForTesting
+    HttpResponse execute(HttpClient httpClient, HttpRequestBase method) throws IOException, ClientProtocolException {
+        return httpClient.execute(method);
     }
 
     private String processRequestParameters(Map<String, String> parameters) throws UnsupportedEncodingException {

geowebcache/core/src/test/java/org/geowebcache/GeoWebCacheEnvironmentTest.java

@@ -58,7 +58,7 @@ public void testEnvironment() {
         Assert.assertEquals(1, extensions.size());
         Assert.assertTrue(extensions.contains(genv));
 
-        Assert.assertTrue(GeoWebCacheEnvironment.ALLOW_ENV_PARAMETRIZATION);
+        Assert.assertTrue(genv.isAllowEnvParametrization());
     }
 
     @Test