From 5b0f6013e859b2b72e4746bdbb48de16dfb0620d Mon Sep 17 00:00:00 2001 From: Daniel Lavoie Date: Mon, 8 Aug 2016 23:05:10 +0200 Subject: [PATCH 01/11] Expose security context to any Hystrix command. Fix issue gh-1054 --- .../main/asciidoc/spring-cloud-netflix.adoc | 2 + spring-cloud-netflix-core/pom.xml | 5 ++ .../HystrixSecurityAutoConfiguration.java | 87 +++++++++++++++++++ .../SecurityContextConcurrencyStrategy.java | 78 +++++++++++++++++ .../netflix/zuul/filters/ZuulProperties.java | 2 +- .../main/resources/META-INF/spring.factories | 1 + .../netflix/hystrix/HystrixOnlyTests.java | 40 +++++++-- .../security/HystrixSecurityApplication.java | 32 +++++++ .../security/HystrixSecurityTests.java | 87 +++++++++++++++++++ .../app/CustomConcurrenyStrategy.java | 23 +++++ .../security/app/ProxyUsernameController.java | 36 ++++++++ .../hystrix/security/app/TestInterceptor.java | 40 +++++++++ .../hystrix/security/app/UsernameClient.java | 30 +++++++ .../security/app/UsernameController.java | 33 +++++++ .../zuul/filters/ZuulPropertiesTests.java | 7 +- .../src/test/resources/application.yml | 8 ++ 16 files changed, 501 insertions(+), 10 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityAutoConfiguration.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/SecurityContextConcurrencyStrategy.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityApplication.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityTests.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/CustomConcurrenyStrategy.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/ProxyUsernameController.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/TestInterceptor.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameClient.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameController.java diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 31b6d208..41d0e2f9 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -507,6 +507,8 @@ If you want some thread local context to propagate into a `@HystrixCommand` the The same thing applies if you are using `@SessionScope` or `@RequestScope`. You will know when you need to do this because of a runtime exception that says it can't find the scoped context. +You also have the option to set the `hystrix.shareSecurityContext` property to `true`. Doing so will auto configure an Hystrix concurrency strategy plugin hook who will transfer the `SecurityContext` from your main thread to the one used by the Hystrix command. Hystrix does not allow multiple hystrix concurrency strategy to be registered so an extension mechanism is available by declaring your own `HystrixConcurrencyStrategy` as a Spring bean. Spring Cloud will lookup for your implementation within the Spring context and wrap it inside its own plugin. + ### Health Indicator The state of the connected circuit breakers are also exposed in the diff --git a/spring-cloud-netflix-core/pom.xml b/spring-cloud-netflix-core/pom.xml index 91b38f2a..45c1e2de 100644 --- a/spring-cloud-netflix-core/pom.xml +++ b/spring-cloud-netflix-core/pom.xml @@ -34,6 +34,11 @@ spring-boot-starter-actuator true + + org.springframework.boot + spring-boot-starter-security + true + org.springframework.boot spring-boot-starter-web diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityAutoConfiguration.java new file mode 100644 index 00000000..41442c9a --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityAutoConfiguration.java @@ -0,0 +1,87 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security; + +import javax.annotation.PostConstruct; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.condition.AllNestedConditions; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.netflix.hystrix.security.HystrixSecurityAutoConfiguration.HystrixSecurityCondition; +import org.springframework.context.annotation.Conditional; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.core.context.SecurityContext; + +import com.netflix.hystrix.Hystrix; +import com.netflix.hystrix.strategy.HystrixPlugins; +import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy; +import com.netflix.hystrix.strategy.eventnotifier.HystrixEventNotifier; +import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook; +import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher; +import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy; + +/** + * @author Daniel Lavoie + */ +@Configuration +@Conditional(HystrixSecurityCondition.class) +@ConditionalOnClass({ Hystrix.class, SecurityContext.class }) +public class HystrixSecurityAutoConfiguration { + @Autowired(required = false) + private HystrixConcurrencyStrategy existingConcurrencyStrategy; + + @PostConstruct + public void init() { + // Keeps references of existing Hystrix plugins. + HystrixEventNotifier eventNotifier = HystrixPlugins.getInstance() + .getEventNotifier(); + HystrixMetricsPublisher metricsPublisher = HystrixPlugins.getInstance() + .getMetricsPublisher(); + HystrixPropertiesStrategy propertiesStrategy = HystrixPlugins.getInstance() + .getPropertiesStrategy(); + HystrixCommandExecutionHook commandExecutionHook = HystrixPlugins.getInstance() + .getCommandExecutionHook(); + + HystrixPlugins.reset(); + + // Registers existing plugins excepts the Concurrent Strategy plugin. + HystrixPlugins.getInstance().registerConcurrencyStrategy( + new SecurityContextConcurrencyStrategy(existingConcurrencyStrategy)); + HystrixPlugins.getInstance().registerEventNotifier(eventNotifier); + HystrixPlugins.getInstance().registerMetricsPublisher(metricsPublisher); + HystrixPlugins.getInstance().registerPropertiesStrategy(propertiesStrategy); + HystrixPlugins.getInstance().registerCommandExecutionHook(commandExecutionHook); + } + + static class HystrixSecurityCondition extends AllNestedConditions { + + public HystrixSecurityCondition() { + super(ConfigurationPhase.REGISTER_BEAN); + } + + @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) + static class HystrixEnabled { + + } + + @ConditionalOnProperty(name = "hystrix.shareSecurityContext") + static class ShareSecurityContext { + + } + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/SecurityContextConcurrencyStrategy.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/SecurityContextConcurrencyStrategy.java new file mode 100644 index 00000000..48079e74 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/security/SecurityContextConcurrencyStrategy.java @@ -0,0 +1,78 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.Callable; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; + +import org.springframework.security.concurrent.DelegatingSecurityContextCallable; + +import com.netflix.hystrix.HystrixThreadPoolKey; +import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy; +import com.netflix.hystrix.strategy.concurrency.HystrixRequestVariable; +import com.netflix.hystrix.strategy.concurrency.HystrixRequestVariableLifecycle; +import com.netflix.hystrix.strategy.properties.HystrixProperty; + +/** + * @author daniellavoie + */ +public class SecurityContextConcurrencyStrategy extends HystrixConcurrencyStrategy { + private HystrixConcurrencyStrategy existingConcurrencyStrategy; + + public SecurityContextConcurrencyStrategy( + HystrixConcurrencyStrategy existingConcurrencyStrategy) { + this.existingConcurrencyStrategy = existingConcurrencyStrategy; + } + + @Override + public BlockingQueue getBlockingQueue(int maxQueueSize) { + return existingConcurrencyStrategy != null + ? existingConcurrencyStrategy.getBlockingQueue(maxQueueSize) + : super.getBlockingQueue(maxQueueSize); + } + + @Override + public HystrixRequestVariable getRequestVariable( + HystrixRequestVariableLifecycle rv) { + return existingConcurrencyStrategy != null + ? existingConcurrencyStrategy.getRequestVariable(rv) + : super.getRequestVariable(rv); + } + + @Override + public ThreadPoolExecutor getThreadPool(HystrixThreadPoolKey threadPoolKey, + HystrixProperty corePoolSize, + HystrixProperty maximumPoolSize, + HystrixProperty keepAliveTime, TimeUnit unit, + BlockingQueue workQueue) { + return existingConcurrencyStrategy != null + ? existingConcurrencyStrategy.getThreadPool(threadPoolKey, corePoolSize, + maximumPoolSize, keepAliveTime, unit, workQueue) + : super.getThreadPool(threadPoolKey, corePoolSize, maximumPoolSize, + keepAliveTime, unit, workQueue); + } + + @Override + public Callable wrapCallable(Callable callable) { + return existingConcurrencyStrategy != null + ? existingConcurrencyStrategy + .wrapCallable(new DelegatingSecurityContextCallable(callable)) + : super.wrapCallable(new DelegatingSecurityContextCallable(callable)); + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index debf0d10..0d49e12b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -48,7 +48,7 @@ public class ZuulProperties { * duplicated if the proxy and the backend are secured with Spring. By default they * are added to the ignored headers if Spring Security is present. */ - private static final List SECURITY_HEADERS = Arrays.asList("Pragma", + public static final List SECURITY_HEADERS = Arrays.asList("Pragma", "Cache-Control", "X-Frame-Options", "X-Content-Type-Options", "X-XSS-Protection", "Expires"); diff --git a/spring-cloud-netflix-core/src/main/resources/META-INF/spring.factories b/spring-cloud-netflix-core/src/main/resources/META-INF/spring.factories index 709e2987..09988567 100644 --- a/spring-cloud-netflix-core/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-netflix-core/src/main/resources/META-INF/spring.factories @@ -5,6 +5,7 @@ org.springframework.cloud.netflix.feign.FeignAutoConfiguration,\ org.springframework.cloud.netflix.feign.encoding.FeignAcceptGzipEncodingAutoConfiguration,\ org.springframework.cloud.netflix.feign.encoding.FeignContentGzipEncodingAutoConfiguration,\ org.springframework.cloud.netflix.hystrix.HystrixAutoConfiguration,\ +org.springframework.cloud.netflix.hystrix.security.HystrixSecurityAutoConfiguration,\ org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration,\ org.springframework.cloud.netflix.rx.RxJavaAutoConfiguration,\ org.springframework.cloud.netflix.metrics.servo.ServoMetricsAutoConfiguration diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixOnlyTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixOnlyTests.java index 1e3478d5..799c8ac0 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixOnlyTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixOnlyTests.java @@ -16,6 +16,11 @@ package org.springframework.cloud.netflix.hystrix; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Base64; import java.util.Map; import org.junit.Test; @@ -30,6 +35,9 @@ import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.cloud.client.circuitbreaker.EnableCircuitBreaker; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.web.bind.annotation.RequestMapping; @@ -37,10 +45,6 @@ import org.springframework.web.bind.annotation.RestController; import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - /** * @author Spencer Gibb */ @@ -51,6 +55,12 @@ public class HystrixOnlyTests { @Value("${local.server.port}") private int port; + + @Value("${security.user.username}") + private String username; + + @Value("${security.user.password}") + private String password; @Test public void testNormalExecution() { @@ -82,9 +92,27 @@ public class HystrixOnlyTests { map.containsKey("discovery")); } + + private Map getHealth() { - return new TestRestTemplate().getForObject("http://localhost:" + this.port - + "/admin/health", Map.class); + return new TestRestTemplate().exchange( + "http://localhost:" + this.port + "/admin/health", HttpMethod.GET, + new HttpEntity(createBasicAuthHeader(username, password)), + Map.class).getBody(); + } + + public static HttpHeaders createBasicAuthHeader(final String username, + final String password) { + return new HttpHeaders() { + private static final long serialVersionUID = 1766341693637204893L; + + { + String auth = username + ":" + password; + byte[] encodedAuth = Base64.getEncoder().encode(auth.getBytes()); + String authHeader = "Basic " + new String(encodedAuth); + this.set("Authorization", authHeader); + } + }; } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityApplication.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityApplication.java new file mode 100644 index 00000000..7d146703 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityApplication.java @@ -0,0 +1,32 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security; + +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.hystrix.security.app.UsernameClient; +import org.springframework.context.annotation.Configuration; + +/** + * @author Daniel Lavoie + */ +@Configuration +@SpringBootApplication +@EnableFeignClients(clients = UsernameClient.class) +public class HystrixSecurityApplication { + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityTests.java new file mode 100644 index 00000000..6645a905 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/HystrixSecurityTests.java @@ -0,0 +1,87 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security; + +import java.util.Base64; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; +import org.springframework.cloud.netflix.hystrix.security.app.CustomConcurrenyStrategy; +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.client.RestTemplate; + +/** + * Tests that a secured web service returning values using a feign client properly access + * the security context from a hystrix command. + * @author Daniel Lavoie + */ +@RunWith(SpringJUnit4ClassRunner.class) +@DirtiesContext +@SpringBootTest(classes = HystrixSecurityApplication.class, webEnvironment = WebEnvironment.RANDOM_PORT, properties = "username.ribbon.listOfServers=localhost:${local.server.port}") +public class HystrixSecurityTests { + @Autowired + private CustomConcurrenyStrategy customConcurrenyStrategy; + + @Value("${local.server.port}") + private String serverPort; + + @Value("${security.user.username}") + private String username; + + @Value("${security.user.password}") + private String password; + + @Test + public void testFeignHystrixSecurity() { + HttpHeaders headers = HystrixSecurityTests.createBasicAuthHeader(username, + password); + + String usernameResult = new RestTemplate() + .exchange("http://localhost:" + serverPort + "/proxy-username", + HttpMethod.GET, new HttpEntity(headers), String.class) + .getBody(); + + Assert.assertTrue("Username should have been intercepted by feign interceptor.", + username.equals(usernameResult)); + + Assert.assertTrue("Custom hook should have been called.", + customConcurrenyStrategy.isHookCalled()); + } + + public static HttpHeaders createBasicAuthHeader(final String username, + final String password) { + return new HttpHeaders() { + private static final long serialVersionUID = 1766341693637204893L; + + { + String auth = username + ":" + password; + byte[] encodedAuth = Base64.getEncoder().encode(auth.getBytes()); + String authHeader = "Basic " + new String(encodedAuth); + this.set("Authorization", authHeader); + } + }; + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/CustomConcurrenyStrategy.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/CustomConcurrenyStrategy.java new file mode 100644 index 00000000..674adf67 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/CustomConcurrenyStrategy.java @@ -0,0 +1,23 @@ +package org.springframework.cloud.netflix.hystrix.security.app; + +import java.util.concurrent.Callable; + +import org.springframework.stereotype.Component; + +import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy; + +@Component +public class CustomConcurrenyStrategy extends HystrixConcurrencyStrategy { + private boolean hookCalled; + + @Override + public Callable wrapCallable(Callable callable) { + this.hookCalled = true; + + return super.wrapCallable(callable); + } + + public boolean isHookCalled() { + return hookCalled; + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/ProxyUsernameController.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/ProxyUsernameController.java new file mode 100644 index 00000000..2f30427c --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/ProxyUsernameController.java @@ -0,0 +1,36 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security.app; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * @author Daniel Lavoie + */ +@RestController +@RequestMapping("/proxy-username") +public class ProxyUsernameController { + @Autowired + private UsernameClient usernameClient; + + @RequestMapping + public String getUsername() { + return usernameClient.getUsername(); + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/TestInterceptor.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/TestInterceptor.java new file mode 100644 index 00000000..c904de7e --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/TestInterceptor.java @@ -0,0 +1,40 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security.app; + +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Component; + +import feign.RequestInterceptor; +import feign.RequestTemplate; + +/** + * This interceptor should be called from an Hyxtrix command execution thread. It is + * access the SecurityContext and settings an http header from the authentication details. + * + * @author Daniel Lavoie + */ +@Component +public class TestInterceptor implements RequestInterceptor { + + @Override + public void apply(RequestTemplate template) { + if (SecurityContextHolder.getContext().getAuthentication() != null) + template.header("username", + SecurityContextHolder.getContext().getAuthentication().getName()); + } +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameClient.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameClient.java new file mode 100644 index 00000000..b630f839 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameClient.java @@ -0,0 +1,30 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security.app; + +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.web.bind.annotation.RequestMapping; + +/** + * @author Daniel Lavoie + */ +@FeignClient("username") +public interface UsernameClient { + + @RequestMapping("/username") + public String getUsername(); +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameController.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameController.java new file mode 100644 index 00000000..54605b8e --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/security/app/UsernameController.java @@ -0,0 +1,33 @@ +/* + * Copyright 2013-2016 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 + * + * http://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.netflix.hystrix.security.app; + +import org.springframework.web.bind.annotation.RequestHeader; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * @author Daniel Lavoie + */ +@RestController +@RequestMapping("/username") +public class UsernameController { + @RequestMapping + public String getUsername(@RequestHeader String username){ + return username; + } +} 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 5a1270bb..60e25406 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 @@ -46,7 +46,8 @@ public class ZuulPropertiesTests { @Test public void defaultIgnoredHeaders() { - assertTrue(this.zuul.getIgnoredHeaders().isEmpty()); + assertTrue(this.zuul.getIgnoredHeaders() + .containsAll(ZuulProperties.SECURITY_HEADERS)); } @Test @@ -60,8 +61,8 @@ public class ZuulPropertiesTests { ZuulRoute route = new ZuulRoute("foo"); this.zuul.getRoutes().put("foo", route); assertTrue(this.zuul.getRoutes().get("foo").getSensitiveHeaders().isEmpty()); - assertTrue(this.zuul.getSensitiveHeaders().containsAll( - Arrays.asList("Cookie", "Set-Cookie", "Authorization"))); + assertTrue(this.zuul.getSensitiveHeaders() + .containsAll(Arrays.asList("Cookie", "Set-Cookie", "Authorization"))); } @Test diff --git a/spring-cloud-netflix-core/src/test/resources/application.yml b/spring-cloud-netflix-core/src/test/resources/application.yml index 777e546f..dd7d5dea 100644 --- a/spring-cloud-netflix-core/src/test/resources/application.yml +++ b/spring-cloud-netflix-core/src/test/resources/application.yml @@ -53,7 +53,15 @@ zuul: stores: url: http://localhost:8081 path: /stores/** +hystrix: + shareSecurityContext: true feignClient: localappName: localapp methodLevelRequestMappingPath: /hello2 myPlaceholderHeader: myPlaceholderHeaderValue +security: + basic: + path: /proxy-username + user: + username: user + password: password \ No newline at end of file From 36ae029c181821c83eaf0313568951292fab6a6b Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 9 Aug 2016 11:44:01 -0400 Subject: [PATCH 02/11] circleci updates --- circle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/circle.yml b/circle.yml index 466be43d..6386b1d8 100644 --- a/circle.yml +++ b/circle.yml @@ -12,7 +12,7 @@ dependencies: - ./mvnw -s .settings.xml -U --fail-never dependency:go-offline || true test: override: - - ./mvnw -s .settings.xml clean install -U -P sonar -nsu --batch-mode -Dmaven.test.redirectTestOutputToFile=true -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn + - ./mvnw -s .settings.xml clean install org.jacoco:jacoco-maven-plugin:prepare-agent install -U -P sonar -nsu --batch-mode -Dmaven.test.redirectTestOutputToFile=true -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn post: - find . -type f -regex ".*/spring-cloud-*.*/target/*.*" | cpio -pdm $CIRCLE_ARTIFACTS - mkdir -p $CIRCLE_TEST_REPORTS/junit/ From a4ba11eabb01617686b5c269162d5a1a7784541b Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 9 Aug 2016 14:31:13 -0400 Subject: [PATCH 03/11] Updated sonar profile configuration --- pom.xml | 43 ++++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/pom.xml b/pom.xml index ba18ea10..bbca6b01 100644 --- a/pom.xml +++ b/pom.xml @@ -177,52 +177,45 @@ sonar - - false - org.jacoco jacoco-maven-plugin - 0.7.4.201502262128 - - ${sonar.jacoco.reportPath} - true - - agent + pre-unit-test prepare-agent + + surefireArgLine + ${project.build.directory}/jacoco.exec + + + + post-unit-test + test + + report + + + + ${project.build.directory}/jacoco.exec + org.apache.maven.plugins maven-surefire-plugin - - ${surefire.plugin.version} - - - listener - org.sonar.java.jacoco.JUnitListener - - + + ${surefireArgLine} - - - org.sonarsource.java - sonar-jacoco-listeners - 3.8 - test - - From ec22c23bc560a39e0b99b682259f8f5e3298ef56 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 9 Aug 2016 16:45:26 -0400 Subject: [PATCH 04/11] Added CircleCI and Codecov badges --- docs/src/main/asciidoc/README.adoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/src/main/asciidoc/README.adoc b/docs/src/main/asciidoc/README.adoc index fa878819..933f7a0f 100644 --- a/docs/src/main/asciidoc/README.adoc +++ b/docs/src/main/asciidoc/README.adoc @@ -1,4 +1,6 @@ -image::https://travis-ci.org/spring-cloud/spring-cloud-netflix.svg?branch=master[Build Status, link=https://travis-ci.org/spring-cloud/spring-cloud-netflix] +image::https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master.svg?style=svg["CircleCI", link="https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master"] +image::https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master/graph/badge.svg["Codecov", link="https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master"] + include::intro.adoc[] From 306b43624bf8501bffcb7b5c80c7f2a8a73cf86a Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 9 Aug 2016 14:51:19 -0600 Subject: [PATCH 05/11] update readme --- README.adoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.adoc b/README.adoc index 6d9556ac..e0597541 100644 --- a/README.adoc +++ b/README.adoc @@ -1,6 +1,8 @@ // Do not edit this file (e.g. go instead to src/main/asciidoc) -image::https://travis-ci.org/spring-cloud/spring-cloud-netflix.svg?branch=master[Build Status, link=https://travis-ci.org/spring-cloud/spring-cloud-netflix] +image::https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master.svg?style=svg["CircleCI", link="https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master"] +image::https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master/graph/badge.svg["Codecov", link="https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master"] + This project provides Netflix OSS integrations for Spring Boot apps through autoconfiguration and binding to the Spring Environment and other Spring programming model idioms. With a few From 909a51000fd9762a2212eca77bc0aced343479b8 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 9 Aug 2016 16:50:57 -0600 Subject: [PATCH 06/11] Remove @Ignore fixes gh-1157 --- .../cloud/netflix/zuul/FormZuulProxyApplicationTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java index f2d786c1..d0bc21d8 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/FormZuulProxyApplicationTests.java @@ -23,7 +23,6 @@ import java.io.InputStream; import java.util.Map; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Value; @@ -91,7 +90,6 @@ public class FormZuulProxyApplicationTests { assertEquals("Posted! {foo=[bar]}", result.getBody()); } - @Ignore // TODO: fix this test @Test public void postWithMultipartForm() { MultiValueMap form = new LinkedMultiValueMap(); From b09b0bcdbd29841cb129efd2fd417d862aa929a9 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 10 Aug 2016 10:36:34 -0400 Subject: [PATCH 07/11] Added Codacy badge to README --- README.adoc | 1 + docs/src/main/asciidoc/README.adoc | 1 + 2 files changed, 2 insertions(+) diff --git a/README.adoc b/README.adoc index e0597541..2e14bffb 100644 --- a/README.adoc +++ b/README.adoc @@ -2,6 +2,7 @@ image::https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master.svg?style=svg["CircleCI", link="https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master"] image::https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master/graph/badge.svg["Codecov", link="https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master"] +image::https://api.codacy.com/project/badge/Grade/a6885a06921e4f72a0df0b7aabd6d118["Codacy code quality", link="https://www.codacy.com/app/Spring-Cloud/spring-cloud-netflix?utm_source=github.com&utm_medium=referral&utm_content=spring-cloud/spring-cloud-netflix&utm_campaign=Badge_Grade"] This project provides Netflix OSS integrations for Spring Boot apps through autoconfiguration diff --git a/docs/src/main/asciidoc/README.adoc b/docs/src/main/asciidoc/README.adoc index 933f7a0f..2d490236 100644 --- a/docs/src/main/asciidoc/README.adoc +++ b/docs/src/main/asciidoc/README.adoc @@ -1,5 +1,6 @@ image::https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master.svg?style=svg["CircleCI", link="https://circleci.com/gh/spring-cloud/spring-cloud-netflix/tree/master"] image::https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master/graph/badge.svg["Codecov", link="https://codecov.io/gh/spring-cloud/spring-cloud-netflix/branch/master"] +image::https://api.codacy.com/project/badge/Grade/a6885a06921e4f72a0df0b7aabd6d118["Codacy code quality", link="https://www.codacy.com/app/Spring-Cloud/spring-cloud-netflix?utm_source=github.com&utm_medium=referral&utm_content=spring-cloud/spring-cloud-netflix&utm_campaign=Badge_Grade"] include::intro.adoc[] From 5e698278a83b254aefd2be2dd9f4f16395127c90 Mon Sep 17 00:00:00 2001 From: Scott Oster Date: Wed, 10 Aug 2016 13:10:30 -0400 Subject: [PATCH 08/11] Fixed double-encoding issue in RibbonClientConfiguration fixes gh-1241 --- .../ribbon/RibbonClientConfiguration.java | 2 +- .../RibbonClientConfigurationTests.java | 70 +++++++++++++++++-- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index efc4433e..3420d4f9 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -200,7 +200,7 @@ public class RibbonClientConfiguration { public URI reconstructURIWithServer(Server server, URI original) { String scheme = original.getScheme(); if (!"https".equals(scheme) && this.serverIntrospector.isSecure(server)) { - original = UriComponentsBuilder.fromUri(original).scheme("https").build() + original = UriComponentsBuilder.fromUri(original).scheme("https").build(true) .toUri(); } return super.reconstructURIWithServer(server, original); 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 101121b5..31544a78 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 @@ -16,26 +16,46 @@ package org.springframework.cloud.netflix.ribbon; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertThat; +import java.net.URI; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration.OverrideRestClient; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; -import org.junit.Test; +import com.netflix.loadbalancer.Server; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.when; /** * @author Spencer Gibb */ public class RibbonClientConfigurationTests { - @Test - public void restClientInitCalledOnce() { - CountingConfig config = new CountingConfig(); + private CountingConfig config; + + @Mock + private ServerIntrospector inspector; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + config = new CountingConfig(); config.setProperty(CommonClientConfigKey.ConnectTimeout, "1"); config.setProperty(CommonClientConfigKey.ReadTimeout, "1"); config.setProperty(CommonClientConfigKey.MaxHttpConnectionsPerHost, "1"); config.setClientName("testClient"); + } + + @Test + public void restClientInitCalledOnce() { new TestRestClient(config); assertThat(config.count, is(equalTo(1))); } @@ -44,7 +64,43 @@ public class RibbonClientConfigurationTests { int count = 0; } - static class TestRestClient extends RibbonClientConfiguration.OverrideRestClient { + @Test + public void testSecureUriFromClientConfig() throws Exception { + Server server = new Server("foo", 7777); + when(inspector.isSecure(server)).thenReturn(true); + + OverrideRestClient overrideRestClient = new OverrideRestClient(this.config, + inspector); + URI uri = overrideRestClient.reconstructURIWithServer(server, + new URI("http://foo/")); + assertThat(uri, is(new URI("https://foo:7777/"))); + } + + @Test + public void testInSecureUriFromClientConfig() throws Exception { + Server server = new Server("foo", 7777); + when(inspector.isSecure(server)).thenReturn(false); + + OverrideRestClient overrideRestClient = new OverrideRestClient(this.config, + inspector); + URI uri = overrideRestClient.reconstructURIWithServer(server, + new URI("http://foo/")); + assertThat(uri, is(new URI("http://foo:7777/"))); + } + + @Test + public void testNotDoubleEncodedWhenSecure() throws Exception { + Server server = new Server("foo", 7777); + when(inspector.isSecure(server)).thenReturn(true); + + OverrideRestClient overrideRestClient = new OverrideRestClient(this.config, + inspector); + URI uri = overrideRestClient.reconstructURIWithServer(server, + new URI("http://foo/%20bar")); + assertThat(uri, is(new URI("https://foo:7777/%20bar"))); + } + + static class TestRestClient extends OverrideRestClient { private TestRestClient(IClientConfig ncc) { super(ncc, new DefaultServerIntrospector()); From 519be4db06329658633477c4fb046895ca177c30 Mon Sep 17 00:00:00 2001 From: libetl Date: Wed, 10 Aug 2016 07:54:06 +0200 Subject: [PATCH 09/11] MetricsInterceptorConfiguration modifies potenially unmodifiable list. fixes gh-1240 --- .../MetricsInterceptorConfiguration.java | 14 +++++-- ...ricsClientHttpRequestInterceptorTests.java | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java index 8b316bba..e48f68b3 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsInterceptorConfiguration.java @@ -13,6 +13,8 @@ package org.springframework.cloud.netflix.metrics; +import java.util.ArrayList; + import org.aspectj.lang.JoinPoint; import org.springframework.beans.BeansException; import org.springframework.beans.factory.config.BeanPostProcessor; @@ -25,6 +27,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.web.client.RestTemplate; import org.springframework.web.servlet.config.annotation.InterceptorRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; @@ -80,8 +83,8 @@ public class MetricsInterceptorConfiguration { return new MetricsInterceptorPostProcessor(); } - private static class MetricsInterceptorPostProcessor implements - BeanPostProcessor, ApplicationContextAware { + private static class MetricsInterceptorPostProcessor + implements BeanPostProcessor, ApplicationContextAware { private ApplicationContext context; private MetricsClientHttpRequestInterceptor interceptor; @@ -97,7 +100,12 @@ public class MetricsInterceptorConfiguration { this.interceptor = this.context .getBean(MetricsClientHttpRequestInterceptor.class); } - ((RestTemplate) bean).getInterceptors().add(interceptor); + RestTemplate restTemplate = (RestTemplate) bean; + // create a new list as the old one may be unmodifiable (ie Arrays.asList()) + ArrayList interceptors = new ArrayList<>(); + interceptors.add(interceptor); + interceptors.addAll(restTemplate.getInterceptors()); + restTemplate.setInterceptors(interceptors); } return bean; } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java index 816b1cac..db5ac7b1 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java @@ -15,8 +15,11 @@ package org.springframework.cloud.netflix.metrics; import static org.junit.Assert.assertEquals; +import java.util.Arrays; + import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.ImportAutoConfiguration; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; @@ -28,6 +31,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -58,6 +62,9 @@ public class MetricsClientHttpRequestInterceptorTests { @Autowired RestTemplate restTemplate; + @Autowired + RestTemplate restTemplateWithFakeInterceptorsList; + @Test public void metricsGatheredWhenSuccessful() { MockRestServiceServer mockServer = MockRestServiceServer.createServer(restTemplate); @@ -79,6 +86,28 @@ public class MetricsClientHttpRequestInterceptorTests { mockServer.verify(); } + + @Test + public void restTemplateWithFakeInterceptorList() { + MockRestServiceServer mockServer = MockRestServiceServer.createServer(restTemplateWithFakeInterceptorsList); + mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) + .andExpect(MockRestRequestMatchers.method(HttpMethod.GET)) + .andRespond(MockRestResponseCreators.withSuccess("OK", MediaType.APPLICATION_JSON)); + String s = restTemplate.getForObject("/test/{id}", String.class, 123); + + MonitorConfig.Builder builder = new MonitorConfig.Builder("metricName") + .withTag("method", "GET") + .withTag("uri", "_test_-id-") + .withTag("status", "200") + .withTag("clientName", "none"); + + BasicTimer timer = servoMonitorCache.getTimer(builder.build()); + + assertEquals(2L, (long) timer.getCount()); + assertEquals("OK", s); + + mockServer.verify(); + } } @Configuration @@ -95,7 +124,15 @@ class MetricsRestTemplateTestConfig { @Configuration class MetricsRestTemplateRestTemplateConfig { @Bean + @Primary RestTemplate restTemplate() { return new RestTemplate(); } + + @Bean(name="restTemplateWithFakeInterceptorsList") + RestTemplate restTemplateWithFakeInterceptorsList() { + RestTemplate restTemplate = new RestTemplate(); + restTemplate.setInterceptors(Arrays.asList(Mockito.mock(ClientHttpRequestInterceptor.class))); + return restTemplate; + } } \ No newline at end of file From f37b513f21325233345f24b9f9260e8aeee6bf65 Mon Sep 17 00:00:00 2001 From: Mike Liu Date: Tue, 9 Aug 2016 21:20:50 -0700 Subject: [PATCH 10/11] Add @Qualifier customization for @FeignClient Changed test to use @Qualifier. Add documentation to reference the new attribute. --- .../src/main/asciidoc/spring-cloud-netflix.adoc | 7 ++++++- .../cloud/netflix/feign/FeignClient.java | 5 +++++ .../netflix/feign/FeignClientsRegistrar.java | 17 +++++++++++++++++ .../netflix/feign/beans/FeignClientTests.java | 2 ++ .../netflix/feign/beans/extra/TestClient.java | 2 +- 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 31b6d208..141f986a 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -790,7 +790,12 @@ In the `@FeignClient` annotation the String value ("stores" above) is an arbitrary client name, which is used to create a Ribbon load balancer (see <>). You can also specify a URL using the `url` attribute -(absolute value or just a hostname). The name of the bean in the application context is the fully qualified name of the interface. An alias is also created which is the 'name' attribute plus 'FeignClient'. For the example above, `@Qualifier("storesFeignClient")` could be used to reference the bean. +(absolute value or just a hostname). The name of the bean in the +application context is the fully qualified name of the interface. +An alias is also created which is the 'name' attribute plus 'FeignClient'. +For the example above, `@Qualifier("storesFeignClient")` could be used to +reference the bean. If you want to change the default `@Qualifier` value, +this can be done with the `qualifier` value in `@FeignClient`. The Ribbon client above will want to discover the physical addresses for the "stores" service. If your application is a Eureka client then diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java index 2aaaeca1..8c2e361f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClient.java @@ -59,6 +59,11 @@ public @interface FeignClient { */ @AliasFor("value") String name() default ""; + + /** + * Sets the @Qualifier value for the feign client. + */ + String qualifier() default ""; /** * An absolute URL or resolvable hostname (the protocol is optional). diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java index ff1a6728..9d31319b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsRegistrar.java @@ -184,6 +184,12 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, String alias = name + "FeignClient"; AbstractBeanDefinition beanDefinition = definition.getBeanDefinition(); beanDefinition.setPrimary(true); + + String qualifier = getQualifier(attributes); + if (StringUtils.hasText(qualifier)) { + alias = qualifier; + } + BeanDefinitionHolder holder = new BeanDefinitionHolder(beanDefinition, className, new String[] { alias }); BeanDefinitionReaderUtils.registerBeanDefinition(holder, registry); @@ -317,6 +323,17 @@ class FeignClientsRegistrar implements ImportBeanDefinitionRegistrar, } return basePackages; } + + private String getQualifier(Map client) { + if (client == null) { + return null; + } + String qualifier = (String) client.get("qualifier"); + if (StringUtils.hasText(qualifier)) { + return qualifier; + } + return null; + } private String getClientName(Map client) { if (client == null) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/FeignClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/FeignClientTests.java index fc5182e2..35406a42 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/FeignClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/FeignClientTests.java @@ -26,6 +26,7 @@ import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.builder.SpringApplicationBuilder; @@ -65,6 +66,7 @@ public class FeignClientTests { @Autowired private ApplicationContext context; + @Qualifier("uniquequalifier") @Autowired private org.springframework.cloud.netflix.feign.beans.extra.TestClient extraClient; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/extra/TestClient.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/extra/TestClient.java index 0f239aa9..42676f68 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/extra/TestClient.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/beans/extra/TestClient.java @@ -21,7 +21,7 @@ import org.springframework.cloud.netflix.feign.beans.FeignClientTests.Hello; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; -@FeignClient(value = "otherapp") +@FeignClient(value = "otherapp", qualifier = "uniquequalifier") public interface TestClient { @RequestMapping(method = RequestMethod.GET, value = "/hello") Hello getHello(); From 674e45f473bf71b766b324fcca3173923ac5a22f Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 10 Aug 2016 14:13:28 -0600 Subject: [PATCH 11/11] Add Client to list of feign objects that can be customized. --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index eeb71b9f..648b4084 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -847,6 +847,9 @@ Spring Cloud Netflix provides the following beans by default for feign (`BeanTyp * `Logger` feignLogger: `Slf4jLogger` * `Contract` feignContract: `SpringMvcContract` * `Feign.Builder` feignBuilder: `HystrixFeign.Builder` +* `Client` feignClient: if Ribbon is enabled it is a `LoadBalancerFeignClient`, otherwise the default feign client is used. + +The OkHttpClient and ApacheHttpClient feign clients can be used by setting `feign.okhttp.enabled` or `feign.httpclient.enabled` to `true`, respectively, and having them on the classpath. Spring Cloud Netflix _does not_ provide the following beans by default for feign, but still looks up beans of these types from the application context to create the feign client: