From ea1a4c5add949d16117fffeb5fa3ef5385d4bac8 Mon Sep 17 00:00:00 2001 From: Jacques-Etienne Beaudet Date: Wed, 4 May 2016 13:40:21 -0400 Subject: [PATCH] Sensitive headers set in PreDecorationFilter no longer override previously set ignored headers. Removed the case sensitiveness when the sensitive headers are set. Fixes https://github.com/spring-cloud/spring-cloud-netflix/issues/1003 --- .../netflix/zuul/ZuulProxyConfiguration.java | 6 +- .../zuul/filters/pre/PreDecorationFilter.java | 10 +++- .../filters/pre/PreDecorationFilterTests.java | 59 +++++++++++++++++-- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 3570c8e0..6f81583e 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -85,10 +85,12 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { // pre filters @Bean - public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator) { + public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, + ProxyRequestHelper proxyRequestHelper) { return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), - this.zuulProperties); + this.zuulProperties, + proxyRequestHelper); } // route filters diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 866bb164..e1ff53b7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -43,13 +43,17 @@ public class PreDecorationFilter extends ZuulFilter { private ZuulProperties properties; private UrlPathHelper urlPathHelper = new UrlPathHelper(); + + private ProxyRequestHelper proxyRequestHelper; public PreDecorationFilter(RouteLocator routeLocator, - String dispatcherServletPath, ZuulProperties properties) { + String dispatcherServletPath, ZuulProperties properties, + ProxyRequestHelper proxyRequestHelper) { this.routeLocator = routeLocator; this.properties = properties; this.urlPathHelper.setRemoveSemicolonContent(properties.isRemoveSemicolonContent()); this.dispatcherServletPath = dispatcherServletPath; + this.proxyRequestHelper = proxyRequestHelper; } @Override @@ -81,9 +85,9 @@ public class PreDecorationFilter extends ZuulFilter { ctx.put("requestURI", route.getPath()); ctx.put("proxy", route.getId()); if (route.getSensitiveHeaders().isEmpty()) { - ctx.put(ProxyRequestHelper.IGNORED_HEADERS, this.properties.getSensitiveHeaders()); + proxyRequestHelper.addIgnoredHeaders(this.properties.getSensitiveHeaders().toArray(new String[0])); } else { - ctx.put(ProxyRequestHelper.IGNORED_HEADERS, route.getSensitiveHeaders()); + proxyRequestHelper.addIgnoredHeaders(route.getSensitiveHeaders().toArray(new String[0])); } if (route.getRetryable() != null) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 13245117..016802d1 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -16,7 +16,9 @@ package org.springframework.cloud.netflix.zuul.filters.pre; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -53,6 +55,8 @@ public class PreDecorationFilterTests { private DiscoveryClientRouteLocator routeLocator; private MockHttpServletRequest request = new MockHttpServletRequest(); + + private ProxyRequestHelper proxyRequestHelper = new ProxyRequestHelper(); @Before public void init() { @@ -60,7 +64,7 @@ public class PreDecorationFilterTests { this.properties = new ZuulProperties(); this.routeLocator = new DiscoveryClientRouteLocator("/", this.discovery, this.properties); - this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties); + this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties, proxyRequestHelper); RequestContext ctx = RequestContext.getCurrentContext(); ctx.clear(); ctx.setRequest(this.request); @@ -81,7 +85,7 @@ public class PreDecorationFilterTests { @Test public void skippedIfForwardToSet() throws Exception { - RequestContext.getCurrentContext().set("forward.to", "mycontext"); + RequestContext.getCurrentContext().set("forward.to", "myconteext"); assertEquals(false, this.filter.shouldFilter()); } @@ -189,7 +193,7 @@ public class PreDecorationFilterTests { new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null)); this.filter = new PreDecorationFilter(this.routeLocator, - "/special", this.properties); + "/special", this.properties, proxyRequestHelper); this.request.setRequestURI("/api/bar/1"); @@ -233,7 +237,7 @@ public class PreDecorationFilterTests { this.routeLocator.addRoute( new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null)); this.filter = new PreDecorationFilter(this.routeLocator, - "/special", this.properties); + "/special", this.properties, proxyRequestHelper); this.filter.run(); @@ -258,7 +262,7 @@ public class PreDecorationFilterTests { new ZuulRoute("foo", "/foo/**", null, "forward:/foo", true, null, null)); this.filter = new PreDecorationFilter(this.routeLocator, - "/special", this.properties); + "/special", this.properties, proxyRequestHelper); this.filter.run(); @@ -296,6 +300,51 @@ public class PreDecorationFilterTests { assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar"))); assertFalse("sensitiveHeaders is wrong", sensitiveHeaders.contains("Cookie")); } + + @Test + public void sensitiveHeadersCaseInsensitive() throws Exception { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.properties.setSensitiveHeaders(Collections.singleton("X-bAr")); + this.request.setRequestURI("/api/foo/1"); + this.routeLocator.addRoute("/foo/**", "foo"); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + @SuppressWarnings("unchecked") + Set sensitiveHeaders = (Set) ctx.get(ProxyRequestHelper.IGNORED_HEADERS); + assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-bar"))); + } + + @Test + public void sensitiveHeadersOverrideCaseInsensitive() throws Exception { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.properties.setSensitiveHeaders(Collections.singleton("X-bAr")); + this.request.setRequestURI("/api/foo/1"); + ZuulRoute route = new ZuulRoute("/foo/**", "foo"); + route.setSensitiveHeaders(Collections.singleton("X-Foo")); + this.routeLocator.addRoute(route); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + @SuppressWarnings("unchecked") + Set sensitiveHeaders = (Set) ctx.get(ProxyRequestHelper.IGNORED_HEADERS); + assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Collections.singletonList("x-foo"))); + } + + @Test + public void ignoredHeadersAlreadySetInRequestContextDontGetOverridden() throws Exception { + this.properties.setPrefix("/api"); + this.properties.setStripPrefix(true); + this.properties.setSensitiveHeaders(Collections.singleton("x-bar")); + this.request.setRequestURI("/api/foo/1"); + this.routeLocator.addRoute("/foo/**", "foo"); + RequestContext ctx = RequestContext.getCurrentContext(); + ctx.set(ProxyRequestHelper.IGNORED_HEADERS, new HashSet<>(Arrays.asList("x-foo"))); + this.filter.run(); + @SuppressWarnings("unchecked") + Set sensitiveHeaders = (Set) ctx.get(ProxyRequestHelper.IGNORED_HEADERS); + assertTrue("sensitiveHeaders is wrong", sensitiveHeaders.containsAll(Arrays.asList("x-bar","x-foo"))); + } private Object getHeader(List> headers, String key) { String value = null;