spring-cloud / spring-cloud-circuitbreaker

Spring Cloud Circuit Breaker API and Implementations
Apache License 2.0
329 stars 110 forks source link

circuit-breaker configuration with feign interceptor to share security context #123

Closed bdi-gud closed 2 years ago

bdi-gud commented 2 years ago

Hello! I am having some issues to configure the spring cloud circuit-breaker with resilience4j and the security context. I need to retrieve a token with a feign interceptor. I was on Hystrix and the configuration were simple with property shareSecurityContext.

I understand that the cicruit breaker should not create a new thread in contrary to Hystrix. A new thread is created when we use Bulkhead which should be activated when found in the classpath as explain in the documentation.

In my project, I have included only org.springframework.cloud:spring-cloud-starter-circuitbreaker-resilience4j and the Bulkhead should not be used. During my tests I noted that a new thread is created and the security context is null in the interceptor. I did not see any clue that the Bulkhead was activated. Contrary to Bulkhead I did not find any property to setup the context propagation with the circuit-breaker.

Is it a normal behaviour or is there something that I have missed in the configuration or documentation ?

Thank you in advance

ryanjbaxter commented 2 years ago

Can you provide a complete, minimal, verifiable sample that reproduces the problem? It should be available as a GitHub (or similar) project or attached to this issue as a zip file.

bdi-gud commented 2 years ago

spring-cloud-circuitbreaker-open-feign.zip

Please find in attachment a sample based on the circuit-breaker demo. I added spring cloud open feign and a request interceptor. I also logged the thread id.

koehleru commented 2 years ago

i have the same problem. i set the SecurityContextHolder-strategy

SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL);

but the in the RequestIntercepter the Authentication is still null.

koehleru commented 2 years ago

@ryanjbaxter any news? can you reproduce the problem?

ryanjbaxter commented 2 years ago

Its on my list to look at, things have been busy

koehleru commented 2 years ago

i checked this behavior again and i think it is not a bug. @bdi-gud in your example you have to set SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL); globaly somewhere in a Configuration and then the propagation of SecurityContext to the child thread from the CircuitBreaker works. this copy the SecurityContext to the new thread.

koehleru commented 2 years ago

i found a bettwer way. configure the Resilience4JCircuitBreakerFactory to use a DelegatingSecurityContextExecutorService

@Bean
public Customizer<Resilience4JCircuitBreakerFactory> defaultCustomizer() {
    return factory -> factory.configureExecutorService(new DelegatingSecurityContextExecutorService(Executors.newCachedThreadPool()));
}

so the SecurityContext is propagate to the child thread for the CircuitBreaker. with the SecurityContext strategy SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL); you can get security issues, because the Context is only propagted when a new child thread is created.

bdi-gud commented 2 years ago

@koehleru Your solution works fine. Thank you for your input.

SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL); was problematic in my setup with scheduled tasks.