Skip to content

Commit 0e058cd

Browse files
groldangithub-actions[bot]
authored andcommitted
[GWC-1363] Support Environment Parametrization for WMSLayer Credentials
This commit enhances security and configurability by enabling dynamic runtime resolution of HTTP Basic Authentication credentials for WMS layers. Credentials can now be injected from environment variables, reducing the need to hardcode sensitive values. This improves code maintainability, supports secure multi- environment deployments, and simplifies testing through dynamic configuration. 1. **Dynamic Environment Parametrization**: - Introduced `GeoWebCacheEnvironment#isAllowEnvParametrization()` to replace the static `ALLOW_ENV_PARAMETRIZATION` field, allowing runtime toggling. 2. **Environment Variable Resolution Refactor**: - Replaced direct static field checks with method calls. - Updated `resolveValue()` and related methods to use environment variables dynamically. 3. **WMS Credentials Management Update**: - Added `getResolvedHttpUsername()` and `getResolvedHttpPassword()` in `WMSHttpHelper`. - Created `setGeoWebCacheEnvironment()` for dependency injection. 4. **Testing Enhancements**: - Integrated the `system-rules` library for environment variable manipulation. - Added tests to cover default, custom, and parameterized credentials. 5. **Code Improvements**: - Replaced unsafe casts in `resolveValue()`. - Improved exception handling by switching from `Throwable` to `RuntimeException`. - Added better logging and documentation for credential handling.
1 parent 69b9d82 commit 0e058cd

File tree

13 files changed

+578
-45
lines changed

13 files changed

+578
-45
lines changed

geowebcache/core/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@
205205
<artifactId>awaitility</artifactId>
206206
<scope>test</scope>
207207
</dependency>
208+
<dependency>
209+
<!-- used for tests that require environment variables -->
210+
<groupId>com.github.stefanbirkner</groupId>
211+
<artifactId>system-rules</artifactId>
212+
<scope>test</scope>
213+
</dependency>
208214

209215
<!-- Thijs Brentjens: for security fixes, OWASP library-->
210216
<dependency>

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.springframework.beans.factory.config.PlaceholderConfigurerSupport;
2424
import org.springframework.core.Constants;
2525
import org.springframework.util.PropertyPlaceholderHelper;
26-
import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver;
2726

2827
/**
2928
* Utility class uses to process GeoWebCache configuration workflow through external environment variables.
@@ -46,7 +45,7 @@
4645
public class GeoWebCacheEnvironment {
4746

4847
/** logger */
49-
public static Logger LOGGER = Logging.getLogger(GeoWebCacheEnvironment.class.getName());
48+
public static final Logger LOGGER = Logging.getLogger(GeoWebCacheEnvironment.class.getName());
5049

5150
private static final Constants constants = new Constants(PlaceholderConfigurerSupport.class);
5251

@@ -55,9 +54,13 @@ public class GeoWebCacheEnvironment {
5554
* placeholders translation.
5655
*
5756
* <p>Default to FALSE
57+
*
58+
* @deprecated a static final variable does prevents change during runtime and hinders testing. Use
59+
* {@link #isAllowEnvParametrization()} instead.
5860
*/
61+
@Deprecated(forRemoval = true)
5962
public static final boolean ALLOW_ENV_PARAMETRIZATION =
60-
Boolean.valueOf(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
63+
Boolean.parseBoolean(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
6164

6265
private static final String nullValue = "null";
6366

@@ -67,10 +70,17 @@ public class GeoWebCacheEnvironment {
6770
constants.asString("DEFAULT_VALUE_SEPARATOR"),
6871
true);
6972

70-
private final PlaceholderResolver resolver = placeholderName -> resolvePlaceholder(placeholderName);
71-
7273
private Properties props;
7374

75+
/**
76+
* Determines if the {@code ALLOW_ENV_PARAMETRIZATION} environment variable is set to {@code true} and hence
77+
* variable variable substitution of configuration parameters using <code>${}</code> place holders can be performed
78+
* through {@link #resolveValue(Object)}.
79+
*/
80+
public boolean isAllowEnvParametrization() {
81+
return Boolean.parseBoolean(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
82+
}
83+
7484
/**
7585
* Internal "props" getter method.
7686
*
@@ -107,7 +117,7 @@ protected String resolveSystemProperty(String key) {
107117
value = System.getenv(key);
108118
}
109119
return value;
110-
} catch (Throwable ex) {
120+
} catch (RuntimeException ex) {
111121
if (LOGGER.isLoggable(Level.FINE)) {
112122
LOGGER.fine("Could not access system property '" + key + "': " + ex);
113123
}
@@ -116,7 +126,7 @@ protected String resolveSystemProperty(String key) {
116126
}
117127

118128
protected String resolveStringValue(String strVal) throws BeansException {
119-
String resolved = this.helper.replacePlaceholders(strVal, this.resolver);
129+
String resolved = this.helper.replacePlaceholders(strVal, this::resolvePlaceholder);
120130

121131
return (resolved.equals(nullValue) ? null : resolved);
122132
}
@@ -127,19 +137,17 @@ protected String resolveStringValue(String strVal) throws BeansException {
127137
* <p>The method first looks for System variables which take precedence on local ones, then into internal props
128138
* injected through the applicationContext.
129139
*/
130-
public Object resolveValue(Object value) {
131-
if (value != null) {
132-
if (value instanceof String) {
133-
return resolveStringValue((String) value);
134-
}
140+
@SuppressWarnings("unchecked")
141+
public <T> T resolveValue(T value) {
142+
if (value instanceof String) {
143+
return (T) resolveStringValue((String) value);
135144
}
136145

137146
return value;
138147
}
139148

140149
private String resolveValueIfEnabled(String value) {
141-
if (ALLOW_ENV_PARAMETRIZATION) return (String) resolveValue(value);
142-
else return value;
150+
return isAllowEnvParametrization() ? resolveValue(value) : value;
143151
}
144152

145153
private boolean validateBoolean(String value) {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import javax.xml.validation.SchemaFactory;
5454
import javax.xml.validation.Validator;
5555
import org.geotools.util.logging.Logging;
56+
import org.geowebcache.GeoWebCacheEnvironment;
5657
import org.geowebcache.GeoWebCacheException;
5758
import org.geowebcache.GeoWebCacheExtensions;
5859
import org.geowebcache.config.ContextualConfigurationProvider.Context;
@@ -282,6 +283,8 @@ public void setDefaultValues(TileLayer layer) {
282283
sourceHelper = new WMSHttpHelper(null, null, proxyUrl);
283284
log.fine("Not using HTTP credentials for " + wl.getName());
284285
}
286+
GeoWebCacheEnvironment gwcEnv = GeoWebCacheExtensions.bean(GeoWebCacheEnvironment.class);
287+
sourceHelper.setGeoWebCacheEnvironment(gwcEnv);
285288

286289
wl.setSourceHelper(sourceHelper);
287290
wl.setLockProvider(getGwcConfig().getLockProvider());

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

Lines changed: 97 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
*/
1414
package org.geowebcache.layer.wms;
1515

16+
import com.google.common.annotations.VisibleForTesting;
1617
import java.io.IOException;
1718
import java.io.InputStream;
1819
import java.io.UnsupportedEncodingException;
@@ -29,14 +30,17 @@
2930
import org.apache.http.HttpEntity;
3031
import org.apache.http.HttpResponse;
3132
import org.apache.http.NameValuePair;
33+
import org.apache.http.client.ClientProtocolException;
3234
import org.apache.http.client.HttpClient;
3335
import org.apache.http.client.methods.HttpGet;
3436
import org.apache.http.client.methods.HttpPost;
3537
import org.apache.http.client.methods.HttpRequestBase;
3638
import org.apache.http.entity.StringEntity;
3739
import org.apache.http.message.BasicNameValuePair;
3840
import org.geotools.util.logging.Logging;
41+
import org.geowebcache.GeoWebCacheEnvironment;
3942
import org.geowebcache.GeoWebCacheException;
43+
import org.geowebcache.GeoWebCacheExtensions;
4044
import org.geowebcache.io.Resource;
4145
import org.geowebcache.layer.TileResponseReceiver;
4246
import org.geowebcache.mime.ErrorMime;
@@ -48,17 +52,39 @@
4852
import org.geowebcache.util.URLs;
4953
import org.springframework.util.Assert;
5054

51-
/** This class is a wrapper for HTTP interaction with WMS backend */
55+
/**
56+
* Helper class for HTTP interactions of {@link WMSLayer} with a WMS backend.
57+
*
58+
* <p>HTTP Basic Auth username and password supplied to the constructor support environment parametrization.
59+
*
60+
* @see GeoWebCacheEnvironment#isAllowEnvParametrization()
61+
* @see GeoWebCacheEnvironment#resolveValue(Object)
62+
*/
5263
public class WMSHttpHelper extends WMSSourceHelper {
5364
private static final Logger log = Logging.getLogger(WMSHttpHelper.class.getName());
5465

66+
/**
67+
* Used by {@link #getResolvedHttpUsername()} and {@link #getResolvedHttpPassword()} to
68+
* {@link GeoWebCacheEnvironment#resolveValue resolve} the actual values against environment variables when
69+
* {@link GeoWebCacheEnvironment#isAllowEnvParametrization() ALLOW_ENV_PARAMETRIZATION} is enabled.
70+
*/
71+
protected GeoWebCacheEnvironment gwcEnv;
72+
5573
private final URL proxyUrl;
5674

75+
/**
76+
* HTTP Basic Auth username, might be de-referenced using environment variable substitution. Always access it
77+
* through {@link #getResolvedHttpUsername()}
78+
*/
5779
private final String httpUsername;
5880

81+
/**
82+
* HTTP Basic Auth password, might be de-referenced using environment variable substitution. Always access it
83+
* through {@link #getResolvedHttpUsername()}
84+
*/
5985
private final String httpPassword;
6086

61-
protected volatile HttpClient client;
87+
protected HttpClient client;
6288

6389
public WMSHttpHelper() {
6490
this(null, null, null);
@@ -71,23 +97,75 @@ public WMSHttpHelper(String httpUsername, String httpPassword, URL proxyUrl) {
7197
this.proxyUrl = proxyUrl;
7298
}
7399

74-
HttpClient getHttpClient() {
75-
if (client == null) {
76-
synchronized (this) {
77-
if (client != null) {
78-
return client;
79-
}
100+
/**
101+
* Used by {@link #executeRequest}
102+
*
103+
* @return the actual http username to use when executing requests
104+
*/
105+
public String getResolvedHttpUsername() {
106+
return resolvePlaceHolders(httpUsername);
107+
}
108+
109+
/**
110+
* Used by {@link #executeRequest}
111+
*
112+
* @return the actual http password to use when executing requests
113+
*/
114+
public String getResolvedHttpPassword() {
115+
return resolvePlaceHolders(httpPassword);
116+
}
117+
118+
/**
119+
* Assigns the environment variable {@link GeoWebCacheEnvironment#resolveValue resolver} to perform variable
120+
* substitution against the configured http username and password during {@link #executeRequest}
121+
*
122+
* <p>When unset, a bean of this type will be looked up through {@link GeoWebCacheExtensions#bean(Class)}
123+
*/
124+
public void setGeoWebCacheEnvironment(GeoWebCacheEnvironment gwcEnv) {
125+
this.gwcEnv = gwcEnv;
126+
}
80127

81-
HttpClientBuilder builder = new HttpClientBuilder(
82-
null, getBackendTimeout(), httpUsername, httpPassword, proxyUrl, getConcurrency());
128+
private String resolvePlaceHolders(String value) {
129+
GeoWebCacheEnvironment env = getEnvironment();
130+
return env != null && env.isAllowEnvParametrization() ? env.resolveValue(value) : value;
131+
}
83132

84-
client = builder.buildClient();
85-
}
133+
private GeoWebCacheEnvironment getEnvironment() {
134+
GeoWebCacheEnvironment env = this.gwcEnv;
135+
if (env == null) {
136+
env = GeoWebCacheExtensions.bean(GeoWebCacheEnvironment.class);
137+
this.gwcEnv = env;
86138
}
139+
return env;
140+
}
87141

142+
/**
143+
* Note: synchronizing the entire method avoids double-checked locking altogether. Modern JVMs optimize this and
144+
* reduce the overhead of synchronization. Otherwise PMD complains with a `Double checked locking is not thread safe
145+
* in Java.` error.
146+
*/
147+
synchronized HttpClient getHttpClient() {
148+
if (client == null) {
149+
int backendTimeout = getBackendTimeout();
150+
String user = getResolvedHttpUsername();
151+
String password = getResolvedHttpPassword();
152+
URL proxy = proxyUrl;
153+
int concurrency = getConcurrency();
154+
client = buildHttpClient(backendTimeout, user, password, proxy, concurrency);
155+
}
88156
return client;
89157
}
90158

159+
@VisibleForTesting
160+
HttpClient buildHttpClient(int backendTimeout, String username, String password, URL proxy, int concurrency) {
161+
162+
URL serverUrl = null;
163+
HttpClientBuilder builder =
164+
new HttpClientBuilder(serverUrl, backendTimeout, username, password, proxy, concurrency);
165+
166+
return builder.buildClient();
167+
}
168+
91169
/** Loops over the different backends, tries the request */
92170
@Override
93171
protected void makeRequest(
@@ -319,7 +397,13 @@ public HttpResponse executeRequest(
319397
if (log.isLoggable(Level.FINER)) {
320398
log.finer(method.toString());
321399
}
322-
return getHttpClient().execute(method);
400+
HttpClient httpClient = getHttpClient();
401+
return execute(httpClient, method);
402+
}
403+
404+
@VisibleForTesting
405+
HttpResponse execute(HttpClient httpClient, HttpRequestBase method) throws IOException, ClientProtocolException {
406+
return httpClient.execute(method);
323407
}
324408

325409
private String processRequestParameters(Map<String, String> parameters) throws UnsupportedEncodingException {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void testEnvironment() {
5858
Assert.assertEquals(1, extensions.size());
5959
Assert.assertTrue(extensions.contains(genv));
6060

61-
Assert.assertTrue(GeoWebCacheEnvironment.ALLOW_ENV_PARAMETRIZATION);
61+
Assert.assertTrue(genv.isAllowEnvParametrization());
6262
}
6363

6464
@Test

0 commit comments

Comments
 (0)