diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 5768bf6c..53961116 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1305,6 +1305,9 @@ the service-specific prefix from individual routes, e.g. stripPrefix: false ---- +NOTE: `zuul.stripPrefix` only applies to the prefix set in `zuul.prefix`. It does have any effect on prefixes +defined within a given route's `path`. + In this example, requests to "/myusers/101" will be forwarded to "/myusers/101" on the "users" service. The `zuul.routes` entries actually bind to an object of type `ZuulProperties`. If you @@ -1410,6 +1413,8 @@ need to set it unless you want it to be different. N.B. this is new in Spring Cloud Netflix 1.1 (in 1.0 the user had no control over headers and all cookies flow in both directions). +=== Ignored Headers + In addition to the per-route sensitive headers, you can set a global value for `zuul.ignoredHeaders` for values that should be discarded (both request and response) during interactions with downstream @@ -1418,6 +1423,7 @@ classpath, and otherwise they are initialized to a set of well-known "security" headers (e.g. involving caching) as specified by Spring Security. The assumption in this case is that the downstream services might add these headers too, and we want the values from the proxy. +To not discard these well known security headers in case Spring Security is on the classpath you can set `zuul.ignoreSecurityHeaders` to `false`. This can be useful if you disabled the HTTP Security response headers in Spring Security and want the values provided by downstream services === The Routes Endpoint diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java index 711b665b..fb172027 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonUtils.java @@ -1,18 +1,19 @@ package org.springframework.cloud.netflix.ribbon; +import java.net.URI; + +import org.springframework.web.util.UriComponentsBuilder; + import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; -import com.netflix.client.config.IClientConfigKey; import com.netflix.config.ConfigurationManager; import com.netflix.config.DynamicPropertyFactory; import com.netflix.config.DynamicStringProperty; import com.netflix.loadbalancer.Server; -import org.springframework.web.util.UriComponentsBuilder; - -import java.net.URI; /** * @author Spencer Gibb + * @author Jacques-Etienne Beaudet */ public class RibbonUtils { @@ -67,13 +68,19 @@ public class RibbonUtils { * @param server * @return */ - public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector, Server server) { + public static URI updateToHttpsIfNeeded(URI uri, IClientConfig config, ServerIntrospector serverIntrospector, + Server server) { String scheme = uri.getScheme(); if (!"https".equals(scheme) && isSecure(config, serverIntrospector, server)) { - return UriComponentsBuilder.fromUri(uri).scheme("https").build(true) - .toUri(); + UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUri(uri).scheme("https"); + if (uri.getRawQuery() != null) { + // When building the URI, UriComponentsBuilder verify the allowed characters and does not + // support the '+' so we replace it for its equivalent '%20'. + // See issue https://jira.spring.io/browse/SPR-10172 + uriComponentsBuilder.replaceQuery(uri.getRawQuery().replace("+", "%20")); + } + return uriComponentsBuilder.build(true).toUri(); } return uri; } - } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java index daa1e831..b623652a 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java @@ -23,13 +23,11 @@ import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.impl.client.HttpClientBuilder; -import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient; import org.springframework.web.util.UriComponentsBuilder; import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; @@ -42,40 +40,27 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps //TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient public class RibbonLoadBalancingHttpClient extends - AbstractLoadBalancingClient { - private final HttpClient delegate; - private final IClientConfig config; - private final ServerIntrospector serverIntrospector; + AbstractLoadBalancingClient { @Deprecated public RibbonLoadBalancingHttpClient() { - this(new DefaultClientConfigImpl(), new DefaultServerIntrospector()); + super(); } @Deprecated public RibbonLoadBalancingHttpClient(final ILoadBalancer lb) { super(lb); - this.config = new DefaultClientConfigImpl(); - this.delegate = createHttpClient(this.config); - this.serverIntrospector = new DefaultServerIntrospector(); - initWithNiwsConfig(config); } public RibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector) { - this.delegate = createHttpClient(config); - this.config = config; - this.serverIntrospector = serverIntrospector; - initWithNiwsConfig(config); + super(config, serverIntrospector); } public RibbonLoadBalancingHttpClient(HttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector) { - this.delegate = delegate; - this.config = config; - this.serverIntrospector = serverIntrospector; - initWithNiwsConfig(config); + super(delegate, config, serverIntrospector); } - protected HttpClient createHttpClient(IClientConfig config) { + protected HttpClient createDelegate(IClientConfig config) { return HttpClientBuilder.create() // already defaults to 0 in builder, so resetting to 0 won't hurt .setMaxConnTotal(config.getPropertyAsInteger(CommonClientConfigKey.MaxTotalConnections, 0)) @@ -86,10 +71,6 @@ public class RibbonLoadBalancingHttpClient .build(); } - protected HttpClient getDelegate() { - return this.delegate; - } - @Override public RibbonApacheHttpResponse execute(RibbonApacheHttpRequest request, final IClientConfig configOverride) throws Exception { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java index 001e57e2..33851327 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java @@ -19,13 +19,11 @@ package org.springframework.cloud.netflix.ribbon.okhttp; import java.net.URI; import java.util.concurrent.TimeUnit; -import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient; import org.springframework.web.util.UriComponentsBuilder; import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; @@ -40,30 +38,31 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps * @author Spencer Gibb */ public class OkHttpLoadBalancingClient - extends AbstractLoadBalancingClient { - private final OkHttpClient delegate = new OkHttpClient(); - private final IClientConfig config; - private final ServerIntrospector serverIntrospector; + extends AbstractLoadBalancingClient { @Deprecated public OkHttpLoadBalancingClient() { super(); - config = new DefaultClientConfigImpl(); - serverIntrospector = new DefaultServerIntrospector(); } @Deprecated public OkHttpLoadBalancingClient(final ILoadBalancer lb) { super(lb); - config = new DefaultClientConfigImpl(); - serverIntrospector = new DefaultServerIntrospector(); } public OkHttpLoadBalancingClient(IClientConfig config, ServerIntrospector serverIntrospector) { - this.config = config; - this.serverIntrospector = serverIntrospector; - initWithNiwsConfig(config); + super(config, serverIntrospector); + } + + public OkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config, + ServerIntrospector serverIntrospector) { + super(delegate, config, serverIntrospector); + } + + @Override + protected OkHttpClient createDelegate(IClientConfig config) { + return new OkHttpClient(); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java index 8805f216..db376f57 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/support/AbstractLoadBalancingClient.java @@ -17,6 +17,9 @@ package org.springframework.cloud.netflix.ribbon.support; +import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.ServerIntrospector; + import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.IResponse; import com.netflix.client.RequestSpecificRetryHandler; @@ -29,7 +32,7 @@ import com.netflix.loadbalancer.ILoadBalancer; /** * @author Spencer Gibb */ -public abstract class AbstractLoadBalancingClient extends +public abstract class AbstractLoadBalancingClient extends AbstractLoadBalancerAwareClient { protected int connectTimeout; @@ -42,16 +45,47 @@ public abstract class AbstractLoadBalancingClient SECURITY_HEADERS = Arrays.asList("Pragma", "Cache-Control", "X-Frame-Options", "X-Content-Type-Options", @@ -101,6 +102,14 @@ public class ZuulProperties { */ private Set ignoredHeaders = new LinkedHashSet<>(); + /** + * SECURITY_HEADERS are added to ignored headers if spring security is on the classpath and ignoreSecurityHeaders = true + * By setting ignoreSecurityHeaders to false we can switch off this default behaviour. This should be used together with + * disabling the default spring security headers + * see https://docs.spring.io/spring-security/site/docs/current/reference/html/headers.html#default-security-headers + */ + private boolean ignoreSecurityHeaders = true; + /** * Path to install Zuul as a servlet (not part of Spring MVC). The servlet is more * memory efficient for requests with large bodies, e.g. file uploads. @@ -148,7 +157,7 @@ public class ZuulProperties { Set ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders); if (ClassUtils.isPresent( "org.springframework.security.config.annotation.web.WebSecurityConfigurer", - null) && Collections.disjoint(ignoredHeaders, SECURITY_HEADERS)) { + null) && Collections.disjoint(ignoredHeaders, SECURITY_HEADERS) && ignoreSecurityHeaders) { // Allow Spring Security in the gateway to control these headers ignoredHeaders.addAll(SECURITY_HEADERS); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java index e6d05f7c..be0bbe45 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java @@ -127,6 +127,17 @@ public class RibbonClientConfigurationTests { } } + @Test + public void testPlusInQueryStringGetsRewrittenWhenServerIsSecure() throws Exception { + Server server = new Server("foo", 7777); + when(this.inspector.isSecure(server)).thenReturn(true); + + for (AbstractLoadBalancerAwareClient client : clients()) { + URI uri = client.reconstructURIWithServer(server, new URI("http://foo/%20bar?hello=1+2")); + assertThat(uri, is(new URI("https://foo:7777/%20bar?hello=1%202"))); + } + } + private List clients() { ArrayList clients = new ArrayList<>(); clients.add(new OverrideRestClient(this.config, this.inspector)); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java index f8e8204f..53b1bd8d 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonUtilsTests.java @@ -17,20 +17,23 @@ package org.springframework.cloud.netflix.ribbon; -import com.netflix.client.config.CommonClientConfigKey; +import static org.hamcrest.Matchers.is; +import static org.springframework.cloud.netflix.ribbon.RibbonUtils.*; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Map; + import org.junit.Assert; import org.junit.Test; +import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.loadbalancer.Server; -import java.util.Map; - -import static org.hamcrest.Matchers.is; -import static org.springframework.cloud.netflix.ribbon.RibbonUtils.isSecure; - /** * @author Spencer Gibb + * @author Jacques-Etienne Beaudet */ public class RibbonUtilsTests { @@ -77,6 +80,35 @@ public class RibbonUtilsTests { Assert.assertThat("isSecure was wrong", secure, is(false)); } + @Test + public void uriIsNotChangedWhenServerIsNotSecured() throws URISyntaxException { + URI original = new URI("http://foo"); + URI updated = updateToHttpsIfNeeded(original, NON_SECURE_CONFIG, NON_SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("URI should not have been updated since server is not secured.", original, is(updated)); + } + + @Test + public void uriIsNotChangedWhenServerIsSecuredAndUriAlreadyInHttps() throws URISyntaxException { + URI original = new URI("https://foo"); + URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("URI should not have been updated since uri is already in https.", original, is(updated)); + } + + @Test + public void shouldUpgradeUriToHttpsWhenServerIsSecureAndUriNotInHttps() throws URISyntaxException { + URI original = new URI("http://foo"); + URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("URI should have been updated to https.", updated, is(new URI("https://foo"))); + } + + @Test + public void shouldSubstitutePlusInQueryParam() throws URISyntaxException { + URI original = new URI("http://foo/%20bar?hello=1+2"); + URI updated = updateToHttpsIfNeeded(original, SECURE_CONFIG, SECURE_INTROSPECTOR, SERVER); + Assert.assertThat("URI should have had its plus sign replaced in query string.", updated, is(new URI( + "https://foo/%20bar?hello=1%202"))); + } + static DefaultClientConfigImpl getConfig(boolean value) { DefaultClientConfigImpl config = new DefaultClientConfigImpl(); config.setProperty(CommonClientConfigKey.IsSecure, value); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java index 60e25406..9ea87b50 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ZuulPropertiesTests.java @@ -29,6 +29,7 @@ import static org.junit.Assert.assertTrue; /** * @author Dave Syer + * @author Mathias Düsterhöft */ public class ZuulPropertiesTests { @@ -46,10 +47,18 @@ public class ZuulPropertiesTests { @Test public void defaultIgnoredHeaders() { + assertTrue(this.zuul.isIgnoreSecurityHeaders()); assertTrue(this.zuul.getIgnoredHeaders() .containsAll(ZuulProperties.SECURITY_HEADERS)); } + @Test + public void securityHeadersNotIgnored() { + zuul.setIgnoreSecurityHeaders(false); + + assertTrue(this.zuul.getIgnoredHeaders().isEmpty()); + } + @Test public void addIgnoredHeaders() { this.zuul.setIgnoredHeaders(Collections.singleton("x-foo"));