Hi all. i am using the following libs and versions:
compile('org.springframework.boot:spring-boot-starter-actuator')
compile('org.springframework.cloud:spring-cloud-starter-config')
compile('org.springframework.cloud:spring-cloud-starter-eureka')
compile('org.springframework.cloud:spring-cloud-starter-ribbon')
compile('org.springframework.cloud:spring-cloud-starter-hystrix')
compile('org.springframework.cloud:spring-cloud-netflix-hystrix-stream')
compile('org.springframework.cloud:spring-cloud-starter-stream-rabbit')
compile('org.springframework.cloud:spring-cloud-starter-feign')
compile('org.springframework.boot:spring-boot-starter-web')
compile('org.springframework.cloud:spring-cloud-starter-oauth2')
mavenBom 'org.springframework.cloud:spring-cloud-dependencies:Camden.BUILD-SNAPSHOT'
i try to get hystrix working in the THREAD isolation strategy mode relaying AccessTokens, therefore i tried both, Oauth2RestTemplate and Feign.
// Defining Beans
@LoadBalanced
@Bean
public OAuth2RestTemplate loadBalancedOauth2RestTemplate(OAuth2ProtectedResourceDetails resource,
OAuth2ClientContext oauth2Context) {
return new OAuth2RestTemplate(resource, oauth2Context);
}
@Bean
public RequestInterceptor oAuth2RequestInterceptor(OAuth2ProtectedResourceDetails resource,
OAuth2ClientContext oauth2Context) {
return new OAuth2FeignRequestInterceptor(oauth2Context, resource);
}
I also set the feign.hystrix.enabled property to true and also hystrix.shareSecurityContext to true but still getting the "no thread bound" exception when making the service to service call. it is just working with setting the SEMAPHORE strategy.
Any ideas? @daniellavoie
Comment From: daniellavoie
Can you share a project where I can have a look ?
Comment From: daniellavoie
BTW, I don't see Spring Security Starter in your gradle. The hystrix.shareSecurityContext will be effective if Spring Security is present in the classpath. Share a sample project and I'll see if there is any means to adapt the mechanism specially for Spring Cloud OAuth without depending on Spring Security.
Comment From: dennyh89
hi. i have some effort to share the project on github, therefore i will needs some hours, but would like to answer your question. org.springframework.cloud:spring-cloud-starter-oauth2 depends on spring-cloud-starter-security and spring-security-core and so on, therefore i think i have all dependencies in. i can also debug through the autoconfiguration HystrixSecurityAutoConfiguration where the "existingConcurrencyStrategy" is null and gets wrapped into the SecurityContextConcurrencyStrategy. but then the exception occurs in the called OAuth2FeignRequestInterceptor at OAuth2AccessToken accessToken = oAuth2ClientContext.getAccessToken();
Comment From: bilak
@daniellavoie I think this is the same issue as mine. The problem is that RequestContextHolder doesn't hold the attributes for child threads.
Comment From: daniellavoie
I fear that the DelegatingSecurityContextCallable does not transfer the attributes you are expecting. The hystrix.shareSecurityContext was implemented to transfer the Principal holder used by classic Spring Security. A custom one might be needed for the specific requirements of Spring Cloud OAuth. I'll dig into it and see what's what.
Comment From: dennyh89
you can see my code within this project and/or the other Org Projects there: https://github.com/dennyh89-spring-cloud-ref/microservice-two
the config is a bit mixed between config-repo and bootstrap.yml as i have tried around a little bit but it should be easy to follow.
Comment From: daniellavoie
This project does not even run. I am not sure you understand how OAuth2FeignRequestInterceptor works. The OAuth2ProtectedResourceDetails is not something that is available in the container on startup that can be injected on bootstrap. Please provide a unit test that reproduce the exact problem you are encountering.
Comment From: dennyh89
it is now runnable (i forgot to checkin gradle directory with it - gradle bootRun) AFAIK the OAuth2ProtectedResourceDetails are able to be injected as they are always the same since bootstrap in this context. they are just filled with the clientId that is somehow mandatory for generating this Object and it is not needed at all for forwarding the Oauth2 Tokens. The requestInterceptor is using the oAuth2ClientContext to get the Access Token out of the request context and there the problem occurs when using the THREAD strategy which i was expecting to be eliminated with your commit on the HystrixConcurrencyStrategy and the SecurityContextCallable!?
Comment From: s4s0l
My small input maybe you don't realize it. I tried to solve it myself, and maybe I don't understand it fully but there is few places where HystrixConcurrencyStrategy is tweaked: user can inject a bean - it will be picked by HystrixSecurityAutoConfiguration and wrap it with something that wraps callable. There is also SleuthHystrixConcurrencyStrategy which takes HystrixConcurrencyStrategy from HystrixPlugins.getInstance().getConcurrencyStrategy(). If it executes after HystrixSecurityAutoConfiguration will wrap already wrapped strategy, but if order will be different then Sleuth will replace plugin and then will be overriden by HystrixSecurityAutoConfiguration (sorry for syntax, I don't know how to explain it in polish even). I'm not sure what the order will be, will test it maybe later but my suggestion: - because all these strategies do is wrap a callable - there is a obvious need for another wrapper for oauth token propagation - probably users will also want to wrap this callable to fulfill their needs - HystrixConcurrencyStrategy has 2 responsibilities: concurency itself and means to wrap call in callables (it realizes 2 different needs i think)
Maybe it would be nice to introduce Strategy that could be a general wrapper for some 'callablewrapperproviders' that could be chained. It would simplify what has to be done to extend it. Code in HystrixSecurityAutoConfiguration and SleuthHystrixConcurrencyStrategy it quite complicated (and similar) and this way it would be as simple as registering new 'histrixcallablewrapperprovider' bean or something. Making this strategy as default would not break existing code even. Because currently to do it by myself I Have to almost copy code from one of the above classes:/
Comment From: alwaysastudent
There is a fundamental flaw on the implementation of org.springframework.cloud.security.oauth2.client.feign.OAuth2FeignRequestInterceptor.getToken() https://github.com/spring-cloud/spring-cloud-security/blob/v1.2.1.RELEASE/spring-cloud-security/src/main/java/org/springframework/cloud/security/oauth2/client/feign/OAuth2FeignRequestInterceptor.java#L124
The OAuth2FeignRequestInterceptor should be getting the token value from the
((OAuth2AuthenticationDetails) org.springframework.security.core.context.SecurityContextHolder
.getContext().getAuthentication().getDetails()).getTokenValue()
instead of going after the
org.springframework.security.oauth2.client.OAuth2ClientContext.getAccessToken()
This should be the case, because we are dealing with the SecurityContext which gets associated to the current thread of execution on org.springframework.security.concurrent.DelegatingSecurityContextCallable.call(), which gets wrapped from org.springframework.cloud.netflix.hystrix.security.SecurityContextConcurrencyStrategy.wrapCallable()
cc. @dsyer; @miguelsauza; @joaoevangelista
Comment From: alwaysastudent
I was able to work around this by doing the below
@Bean
public OAuth2ClientContext oAuth2ClientContext() {
return new DefaultOAuth2ClientContext() {
@Override
public OAuth2AccessToken getAccessToken() {
return Optional.ofNullable(super.getAccessToken())
.orElse(new DefaultOAuth2AccessToken(
((OAuth2AuthenticationDetails) SecurityContextHolder
.getContext().getAuthentication().getDetails())
.getTokenValue()));
}
};
}
@Bean
public RequestInterceptor oauth2FeignRequestInterceptor(
OAuth2ClientContext oAuth2ClientContext,
OAuth2ProtectedResourceDetails resource) {
return new OAuth2FeignRequestInterceptor(oAuth2ClientContext, resource);
}
hystrix:
shareSecurityContext: true
feign:
hystrix:
enabled: true
cc. @bilak @dennyh89
Comment From: alwaysastudent
I can issue a PR, if you agree with this approach.
Comment From: dsyer
Actually, I don't think that is the right approach. The OAuth2ClientContext knows about the refresh token and how to use it, but the SecurityContext doesn't. So what you need to do is transfer the OAuth context to the hystrix thread, not the generic security context.
Also, the code you linked to is in a different project. Maybe you should open an issue or pull request there instead?
Comment From: alwaysastudent
@dsyer - I can create another issue, give you the github project and work with you for a PR
But if I understand you correctly, you are acknowledging that there is an issue and you are saying it would be better to change the implementation of SecurityContextConcurrencyStrategy.wrapCallable(Callable<T>) - to wrap the commands with a custom delegator similar to DelegatingSecurityContextCallable which would transfer an OAuth2ClientContext instead of SecurityContext on some ThreadLocal carrier. Did I get you right ?
Currently the issue happens within this method, OAuth2FeignRequestInterceptor.getToken() , who craps out trying to weave the oAuth2ClientContext.
And if I step through with my debugger I can grab the detailed message as
org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'scopedTarget.oauth2ClientContext': Scope 'request' is not active for the current thread; consider defining a scoped proxy for this bean if you intend to refer to it from a singleton; nested exception is java.lang.IllegalStateException: No thread-bound request found: Are you referring to request attributes outside of an actual web request, or processing a request outside of the originally receiving thread? If you are actually operating within a web request and still receive this message, your code is probably running outside of DispatcherServlet/DispatcherPortlet: In this case, use RequestContextListener or RequestContextFilter to expose the current request.
Stacktrace
java.lang.IllegalStateException: No thread-bound request found: Are you referring to request attributes outside of an actual web request, or processing a request outside of the originally receiving thread? If you are actually operating within a web request and still receive this message, your code is probably running outside of DispatcherServlet/DispatcherPortlet: In this case, use RequestContextListener or RequestContextFilter to expose the current request.
at org.springframework.web.context.request.RequestContextHolder.currentRequestAttributes(RequestContextHolder.java:131) ~[spring-web-4.3.9.RELEASE.jar:4.3.9.RELEASE]
at org.springframework.web.context.request.SessionScope.get(SessionScope.java:91) ~[spring-web-4.3.9.RELEASE.jar:4.3.9.RELEASE]
at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:340) ~[spring-beans-4.3.9.RELEASE.jar:4.3.9.RELEASE]
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:197) ~[spring-beans-4.3.9.RELEASE.jar:4.3.9.RELEASE]
at org.springframework.aop.target.SimpleBeanTargetSource.getTarget(SimpleBeanTargetSource.java:35) ~[spring-aop-4.3.9.RELEASE.jar:4.3.9.RELEASE]
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:192) ~[spring-aop-4.3.9.RELEASE.jar:4.3.9.RELEASE]
at com.sun.proxy.$Proxy103.getAccessToken(Unknown Source) ~[na:na]
at org.springframework.cloud.security.oauth2.client.feign.OAuth2FeignRequestInterceptor.getToken(OAuth2FeignRequestInterceptor.java:124) ~[spring-cloud-security-1.2.0.RELEASE.jar:1.2.0.RELEASE]
Comment From: dsyer
Your debugging probably suggests that the solution could be in spring-cloud-netflix (supporting @Scope("request") in a HystrixCommand - i.e. nothing to do with the SecurityContext).
Comment From: alwaysastudent
@dsyer - Thank you for clarifying. I got your coordinates now. And I was able to get the request attributes propagated to the OAuth2FeignRequestInterceptor using a custom HystrixConcurrencyStrategy which mimics the same pattern as SecurityContextConcurrencyStrategy. So that solved the weaving issue within the OAuth2FeignRequestInterceptor.getToken() and I am able to get the oauth2ClientContext resolved there along with the token.
I have an auto configuration HystrixRequestScopeAutoConfiguration that is placed after the HystrixSecurityAutoConfiguration. Currently it pivots on the same conditions as the later, but will set up a RequestScopeConcurrencyStrategy that is decorated with any existing strategy if found.
class RequestScopeConcurrencyStrategy{
...
@Override
public <T> Callable<T> wrapCallable(Callable<T> callable) {
return delegate != null
? delegate.wrapCallable(new DelegatingRequestContextCallable<T>(callable))
: super.wrapCallable(new DelegatingRequestContextCallable<T>(callable));
}
...
}
The DelegatingRequestContextCallable* which is wrapping the original Callable as a delegate, and would set the request attributes before invoking the delegate similar to the DelegatingSecurityContextCallable
If you like this approach, I can issue a PR.
Comment From: alwaysastudent
I think it is better to collapse the logic, while enabling hystrix.shareSecurityContext, to transfer both the SecurityContext and the RequestAttributes at one place, since they complement each other.
Also should not be looking for the presence of feign.hystrix.enabled, to dictate these transfers to happen, as this issue is not in particular to feign alone and can happen even if you wrap the OAuth2RestTemplate call within a @HystrixCommand
Comment From: michaeltecourt
We had already written our token relay implementation based on the SecurityContext, and we found out that we only needed to propagate the security context in threads spawned from the request thread :
SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL);
We put that in a @PostConstruct method somewhere in our @Configuration classes.
It works with stuff wrapped in HystrixCommands like @FeignClient calls, just like @Async methods.
@daniellavoie do you see any pitfall in this approach ? It seems far too simple compared to what you did in 5b0f6013.
@dsyer would it be a bad idea to suggest that Spring Boot could auto-configure the SecurityContextHolder strategy through a property like security.context.strategy=MODE_INHERITABLETHREADLOCAL ?
I wonder why it's not the default strategy ?
Comment From: dsyer
That won't work with refresh tokens, but neither will anything else in this project (that's more of a spring-cloud-security thing) so it seems like a reasonable solution if it works for you. I'm kind of surprised it works though because InheritableThreadLocal doesn't usually have any impact on work done in a thread pool. I guess it depends on how the thread pool is constructed.
Comment From: dsyer
@alwaysastudent I don't think we want to link security and request scope together in a single class (they come from different dependencies). I think Spring Cloud Netflix allows you to stack Hystrix strategies together (but I don't recall if that applies to the concurrency strategy - if not we'd have to see if we can extend something). If you are still working on a PR, please try and split it up so it works when Spring Security is not in use.
Comment From: michaeltecourt
OK that's the catch. I was using a FeignClient within an @Async method, which seems to change everything and I'm not sure why :
- Within an
@Asyncmethod, I only need to set the security context holder strategy toMODE_INHERITABLETHREADLOCAL. The SecurityContext is available even ifhystrix.shareSecurityContextisfalse. Is it because the default TaskExecutor configured by@EnableAsyncspawns new threads instead of using a thread pool ? - In a normal setup, I only need
hystrix.shareSecurityContext: true.MODE_INHERITABLETHREADLOCALhas no effect like @dsyer predicted
Comment From: dsyer
That's because @Async by default doesn't use a thread pool. OK, so we are all on the same page. Good.
Comment From: alwaysastudent
@alwaysastudent I don't think we want to link security and request scope together in a single class (they come from different dependencies). I think Spring Cloud Netflix allows you to stack Hystrix strategies together (but I don't recall if that applies to the concurrency strategy - if not we'd have to see if we can extend something). If you are still working on a PR, please try and split it up so it works when Spring Security is not in use.
@dsyer - Makes sense. I am thinking to have a RequestScopeConcurrencyStrategy that can be independently configurable using hystrix.shareRequestContext or if you have the hystrix.shareSecurityContext turned on, since it complements the entire request cycle. I have been busy, but hope I'll be ready to issue a pull this weekend or worse next week.
Comment From: OlgaMaciaszek
Closing as Hystrix is no longer supported within Spring Cloud Netflix.