From a59b3f6031e16f69e31f2013ee32df029d71565d Mon Sep 17 00:00:00 2001 From: gandhiUH Date: Tue, 12 Oct 2021 11:04:33 -0500 Subject: [PATCH] added X-forward-Host and X-Forward-Proto Updated LoadBalancerProperties and AutoConfiguration LoadBalancerXtranformerTests added PR Changes Implemented Changes Implemented as per PR comments Updated XForwadedHeadersTransformer name, test classes as per PR Commit on Oct,25, 2021 Addressed PR comments to modify tests with better Assertions Changed the enabledXforwarded property name to just enabled in Xforwarded class as per PR comments --- .../loadbalancer/LoadBalancerProperties.java | 31 ++++++++ .../LoadBalancerClientConfiguration.java | 10 +++ .../XForwardedHeadersTransformer.java | 55 +++++++++++++ ...ngLoadBalancerClientAutoConfiguration.java | 9 +++ .../core/XForwardedHeadersTransformer.java | 55 +++++++++++++ .../XForwardedHeadersTransformerTests.java | 77 ++++++++++++++++++ .../XForwardedHeadersTransformerTests.java | 78 +++++++++++++++++++ 7 files changed, 315 insertions(+) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformer.java create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformer.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformerTests.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformerTests.java diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java index 644970c7..04aa2ce9 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java @@ -35,6 +35,7 @@ import org.springframework.util.LinkedCaseInsensitiveMap; * annotation. * * @author Olga Maciaszek-Sharma + * @author Gandhimathi Velusamy * @since 2.2.1 */ public class LoadBalancerProperties { @@ -107,6 +108,19 @@ public class LoadBalancerProperties { this.hintHeaderName = hintHeaderName; } + /** + * Enabling X-Forwarded Host and Proto Headers. + */ + private XForwarded xForwarded = new XForwarded(); + + public void setxForwarded(XForwarded xForwarded) { + this.xForwarded = xForwarded; + } + + public XForwarded getxForwarded() { + return xForwarded; + } + public static class StickySession { /** @@ -138,6 +152,23 @@ public class LoadBalancerProperties { } + public static class XForwarded { + + /** + * To Enable X-Forwarded Headers. + */ + private boolean enabled = false; + + public boolean isEnabled() { + return enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + } + public static class HealthCheck { /** diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java index 3fc711c3..ac822695 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java @@ -30,10 +30,12 @@ import org.springframework.cloud.client.ConditionalOnReactiveDiscoveryEnabled; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; import org.springframework.cloud.client.discovery.ReactiveDiscoveryClient; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.core.ReactorLoadBalancer; import org.springframework.cloud.loadbalancer.core.RetryAwareServiceInstanceListSupplier; import org.springframework.cloud.loadbalancer.core.RoundRobinLoadBalancer; import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier; +import org.springframework.cloud.loadbalancer.core.XForwardedHeadersTransformer; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; @@ -92,6 +94,14 @@ public class LoadBalancerClientConfiguration { .build(context); } + @Bean + @ConditionalOnBean(XForwardedHeadersTransformer.class) + @ConditionalOnMissingBean + @ConditionalOnProperty(value = "spring.cloud.loadbalancer.xForwarded.enabledXforwarded", havingValue = "true") + public XForwardedHeadersTransformer xForwarderHeadersTransformer(LoadBalancerProperties properties) { + return new XForwardedHeadersTransformer(properties.getxForwarded()); + } + @Bean @ConditionalOnBean({ ReactiveDiscoveryClient.class, WebClient.Builder.class }) @ConditionalOnMissingBean diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformer.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformer.java new file mode 100644 index 00000000..8e5e25a7 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformer.java @@ -0,0 +1,55 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.blocking; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRequestTransformer; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpRequest; + +/** + * To add X-Forwarded-Host and X-Forwarded-Proto Headers. + * + * @author Gandhimathi Velusamy + * @since 3.1.0 + */ + +public class XForwardedHeadersTransformer implements LoadBalancerRequestTransformer { + + private final LoadBalancerProperties.XForwarded XForwardedHeaders; + + public XForwardedHeadersTransformer(LoadBalancerProperties.XForwarded XForwardedHeaders) { + this.XForwardedHeaders = XForwardedHeaders; + } + + @Override + public HttpRequest transformRequest(HttpRequest request, ServiceInstance instance) { + if (instance == null) { + return request; + } + if (XForwardedHeaders.isEnabled()) { + HttpHeaders headers = request.getHeaders(); + String xForwardedHost = request.getURI().getHost(); + String xforwardedProto = request.getURI().getScheme(); + headers.add("X-Forwarded-Host", xForwardedHost); + headers.add("X-Forwarded-Proto", xforwardedProto); + } + return request; + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java index 2da800e2..52f5a0bf 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java @@ -31,6 +31,7 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancerClientsProperti import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; +import org.springframework.cloud.loadbalancer.blocking.XForwardedHeadersTransformer; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.cloud.loadbalancer.blocking.retry.BlockingLoadBalancedRetryFactory; import org.springframework.cloud.loadbalancer.core.LoadBalancerServiceInstanceCookieTransformer; @@ -44,6 +45,7 @@ import org.springframework.web.client.RestTemplate; * An autoconfiguration for {@link BlockingLoadBalancerClient}. * * @author Olga Maciaszek-Sharma + * @author Gandhimathi Velusamy * @since 2.1.3 */ @Configuration(proxyBeanMethods = false) @@ -71,6 +73,13 @@ public class BlockingLoadBalancerClientAutoConfiguration { return new LoadBalancerServiceInstanceCookieTransformer(properties.getStickySession()); } + @Bean + @ConditionalOnProperty(value = "spring.cloud.loadbalancer.xforwarded.enabledXforwarded", havingValue = "true") + @ConditionalOnMissingBean(XForwardedHeadersTransformer.class) + public XForwardedHeadersTransformer xForwarderHeadersTransformer(LoadBalancerProperties properties) { + return new XForwardedHeadersTransformer(properties.getxForwarded()); + } + @Configuration @ConditionalOnClass(RetryTemplate.class) @EnableConfigurationProperties(LoadBalancerClientsProperties.class) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformer.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformer.java new file mode 100644 index 00000000..cf632098 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformer.java @@ -0,0 +1,55 @@ +/* + * Copyright 2012-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerClientRequestTransformer; +import org.springframework.http.HttpHeaders; +import org.springframework.web.reactive.function.client.ClientRequest; + +/** + * To add X-Forwarded-Host and X-Forwarded-Proto Headers. + * + * @author Gandhimathi Velusamy + * @since 3.1.0 + */ + +public class XForwardedHeadersTransformer implements LoadBalancerClientRequestTransformer { + + private final LoadBalancerProperties.XForwarded XForwardedHeaders; + + public XForwardedHeadersTransformer(LoadBalancerProperties.XForwarded XForwardedHeaders) { + this.XForwardedHeaders = XForwardedHeaders; + } + + @Override + public ClientRequest transformRequest(ClientRequest request, ServiceInstance instance) { + if (instance == null) { + return request; + } + if (XForwardedHeaders.isEnabled()) { + HttpHeaders headers = request.headers(); + String xForwardedHost = request.url().getHost(); + String xForwardedProto = request.url().getScheme(); + headers.add("X-Forwarded-Host", xForwardedHost); + headers.add("X-Forwarded-Proto", xForwardedProto); + } + return request; + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformerTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformerTests.java new file mode 100644 index 00000000..12084036 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/XForwardedHeadersTransformerTests.java @@ -0,0 +1,77 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.blocking; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; + +import static java.net.URI.create; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Tests for XForwadededHeadersTransformer. + * + * @author Gandhimathi Velusamy + */ + +class XForwardedHeadersTransformerTests { + + private final LoadBalancerProperties.XForwarded xForwarded = new LoadBalancerProperties().getxForwarded(); + + private final ServiceInstance serviceInstance = mock(DefaultServiceInstance.class); + + private final HttpRequest request = mock(HttpRequest.class); + + @BeforeEach + void setUp() { + when(serviceInstance.getInstanceId()).thenReturn("test1"); + when(request.getMethod()).thenReturn(HttpMethod.GET); + when(request.getURI()).thenReturn(create("https://google.com")); + when(request.getHeaders()).thenReturn(new HttpHeaders()); + } + + @Test + void shouldAppendXforwardHeaderIfEnabledXforward() throws NullPointerException { + XForwardedHeadersTransformer transformer = new XForwardedHeadersTransformer(xForwarded); + xForwarded.setEnabled(true); + + HttpRequest newRequest = transformer.transformRequest(request, serviceInstance); + + assertThat(newRequest.getHeaders()).containsKey("X-Forwarded-Host"); + assertThat(newRequest.getHeaders().getFirst("X-Forwarded-Host")).isEqualTo("google.com"); + assertThat(newRequest.getHeaders()).containsKey("X-Forwarded-Proto"); + assertThat(newRequest.getHeaders().getFirst("X-Forwarded-Proto")).isEqualTo("https"); + } + + @Test + void shouldNotAppendXforwardedHeaderIfDefault() { + XForwardedHeadersTransformer transformer = new XForwardedHeadersTransformer(xForwarded); + HttpRequest newRequest = transformer.transformRequest(request, serviceInstance); + assertThat(newRequest.getHeaders()).doesNotContainKey("X-Forwarded-Host"); + assertThat(newRequest.getHeaders()).doesNotContainKey("X-Forwarded-Proto"); + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformerTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformerTests.java new file mode 100644 index 00000000..591601c6 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/XForwardedHeadersTransformerTests.java @@ -0,0 +1,78 @@ +/* + * Copyright 2012-2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.net.URI; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.web.reactive.function.client.ClientRequest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Tests for XForwadedHeadersTransformer . + * + * @author Gandhimathi Velusamy + */ + +class XForwardedHeadersTransformerTests { + + private final LoadBalancerProperties.XForwarded xForwarded = new LoadBalancerProperties().getxForwarded(); + + private final ServiceInstance serviceInstance = mock(DefaultServiceInstance.class); + + private final ClientRequest request = mock(ClientRequest.class); + + @BeforeEach + void setUp() { + when(serviceInstance.getInstanceId()).thenReturn("test1"); + when(request.method()).thenReturn(HttpMethod.GET); + when(request.url()).thenReturn(URI.create("https://spring.io")); + when(request.headers()).thenReturn(new HttpHeaders()); + } + + @Test + void shouldAppendXforwardedHeaderIfEnabledXforward() throws NullPointerException { + xForwarded.setEnabled(true); + XForwardedHeadersTransformer transformer = new XForwardedHeadersTransformer(xForwarded); + + ClientRequest newRequest = transformer.transformRequest(request, serviceInstance); + + assertThat(newRequest.headers()).containsKey("X-Forwarded-Host"); + assertThat(newRequest.headers().getFirst("X-Forwarded-Host")).isEqualTo("spring.io"); + assertThat(newRequest.headers()).containsKey("X-Forwarded-Proto"); + assertThat(newRequest.headers().getFirst("X-Forwarded-Proto")).isEqualTo("https"); + } + + @Test + void shouldNotAppendXforwardedHeaderIfDefault() { + XForwardedHeadersTransformer transformer = new XForwardedHeadersTransformer(xForwarded); + ClientRequest newRequest = transformer.transformRequest(request, serviceInstance); + assertThat(newRequest.headers()).doesNotContainKey("X-Forwarded-Host"); + assertThat(newRequest.headers()).doesNotContainKey("X-Forwarded-Proto"); + } + +}