Skip to content

Commit

Permalink
[GWC-1363] Support Environment Parametrization for WMSLayer Credentials
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
groldan committed Feb 9, 2025
1 parent 9c767e3 commit d28c710
Show file tree
Hide file tree
Showing 12 changed files with 567 additions and 43 deletions.
7 changes: 7 additions & 0 deletions geowebcache/core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,13 @@
<artifactId>awaitility</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<!-- used for tests that require environment variables -->
<groupId>com.github.stefanbirkner</groupId>
<artifactId>system-rules</artifactId>
<version>1.19.0</version>
<scope>test</scope>
</dependency>

<!-- Thijs Brentjens: for security fixes, OWASP library-->
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);

Expand All @@ -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";

Expand All @@ -67,10 +70,12 @@ public class GeoWebCacheEnvironment {
constants.asString("DEFAULT_VALUE_SEPARATOR"),
true);

private final PlaceholderResolver resolver = placeholderName -> resolvePlaceholder(placeholderName);

private Properties props;

public boolean isAllowEnvParametrization() {
return Boolean.parseBoolean(GeoWebCacheExtensions.getProperty("ALLOW_ENV_PARAMETRIZATION"));
}

/**
* Internal "props" getter method.
*
Expand Down Expand Up @@ -107,7 +112,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);
}
Expand All @@ -116,7 +121,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);
}
Expand All @@ -127,19 +132,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,19 @@
import org.apache.commons.io.IOUtils;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
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;
Expand All @@ -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);
Expand All @@ -71,23 +97,74 @@ public WMSHttpHelper(String httpUsername, String httpPassword, URL proxyUrl) {
this.proxyUrl = proxyUrl;
}

/**
* Used by {@link #executeRequest}
*
* @return the actual http username to use when executing requests
*/
public String getResolvedHttpUsername() {
return resolve(httpUsername);
}

/**
* Used by {@link #executeRequest}
*
* @return the actual http password to use when executing requests
*/
public String getResolvedHttpPassword() {
return resolve(httpPassword);
}

/**
* Assigns the environment variable {@link GeoWebCacheEnvironment#resolveValue resolver} to perform variable
* substitution agains 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;
}

private String resolve(String value) {
GeoWebCacheEnvironment env = getEnvironment();
return env != null && env.isAllowEnvParametrization() ? env.resolveValue(value) : value;
}

private GeoWebCacheEnvironment getEnvironment() {
GeoWebCacheEnvironment env = this.gwcEnv;
if (env == null) {
env = GeoWebCacheExtensions.bean(GeoWebCacheEnvironment.class);
this.gwcEnv = env;
}
return env;
}

HttpClient getHttpClient() {
if (client == null) {
synchronized (this) {
if (client != null) {
return client;
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);
}

HttpClientBuilder builder = new HttpClientBuilder(
null, getBackendTimeout(), httpUsername, httpPassword, proxyUrl, getConcurrency());

client = builder.buildClient();
}
}

return client;
}

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(
Expand Down Expand Up @@ -235,7 +312,7 @@ private void connectAndCheckHeaders(
}

// Read the actual data
if (responseCode != 204) {
if (responseCode != HttpStatus.SC_NO_CONTENT) {
try (InputStream inStream = method.getEntity().getContent()) {
if (inStream == null) {
log.severe("No response for " + method);
Expand Down Expand Up @@ -319,7 +396,12 @@ public HttpResponse executeRequest(
if (log.isLoggable(Level.FINER)) {
log.finer(method.toString());
}
return getHttpClient().execute(method);
HttpClient httpClient = getHttpClient();
return execute(httpClient, method);
}

HttpResponse execute(HttpClient httpClient, HttpRequestBase method) throws IOException, ClientProtocolException {
return httpClient.execute(method);
}

private String processRequestParameters(Map<String, String> parameters) throws UnsupportedEncodingException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit d28c710

Please sign in to comment.