Spring-boot: RequestContextFilter Ordering

[rwinch]

We attempted to fix RequestContextFilter ordering so that Spring Session (and other APIs) have a chance to wrap the HttpServletRequest before RequestContextFilter sets the RequestContextHolder. This is good, but we now have a bit of a chicken and the egg problem. For example, Spring Security OAuth typically uses session scoped beans which require the RequestContextHolder to be populated.

Honestly, at this point I'm not sure what we want to do in this instance, but wanted to ensure we got this logged and discussed.

An example stacktrace:

2015-10-28 10:11:27.033 ERROR 29039 --- [nio-8080-exec-6] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception
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.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.web.context.request.SessionScope.get(SessionScope.java:91) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:339) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    ... 63 common frames omitted
Wrapped by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'scopedTarget.standardGitHubClient': Scope 'session' 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.
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:354) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:196) ~[spring-beans-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.aop.target.SimpleBeanTargetSource.getTarget(SimpleBeanTargetSource.java:35) ~[spring-aop-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:187) ~[spring-aop-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at com.sun.proxy.$Proxy61.getEmails(Unknown Source) ~[na:na]
    at com.gopivotal.cla.security.AdminEmailDomainFilter.isValidAdminUser(AdminEmailDomainFilter.java:58) ~[classes/:na]
    at com.gopivotal.cla.security.AdminEmailDomainFilter.doFilter(AdminEmailDomainFilter.java:50) ~[classes/:na]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.oauth2.client.filter.OAuth2ClientContextFilter.doFilter(OAuth2ClientContextFilter.java:60) ~[spring-security-oauth2-2.0.7.RELEASE.jar:na]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:114) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:122) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:169) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:48) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:120) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.csrf.CsrfFilter.doFilterInternal(CsrfFilter.java:96) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:64) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:91) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:53) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:330) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:213) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:176) ~[spring-security-web-4.0.2.RELEASE.jar:4.0.2.RELEASE]
    at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:346) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:262) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.springframework.web.filter.HttpPutFormContentFilter.doFilterInternal(HttpPutFormContentFilter.java:87) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.springframework.web.filter.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:77) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:85) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.2.RELEASE.jar:4.2.2.RELEASE]
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:217) ~[tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:502) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:142) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:518) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1091) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:673) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1500) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1456) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [na:1.8.0_51]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [na:1.8.0_51]
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) [tomcat-embed-core-8.0.28.jar:8.0.28]
    at java.lang.Thread.run(Thread.java:745) [na:1.8.0_51]

cc @dsyer @philwebb

[wilkinsona]

I attempted to fix this in d9d4cc2, i.e. this is a duplicate of #4270.

[rwinch]

@wilkinsona Thanks for the response.

The problem with d9d4cc2ef5ab40f55e7a0cde68b1a5e78aefb4b3 is that any Filter that is registered after RequestContextFilter will no longer register a wrapped version of the HttpServletRequest and HttpServletResponse. This means anyone accessing the the HttpServletRequest and HttpServletResponse will no longer get a wrapped version.

More concretely, setting the RequestContextHolder too early will break things like Spring Session integration as illustrated in #2637 This happens because Spring Session will wrap the HttpServletRequest to replace the HttpServletRequest.getSession() (and related) methods. If RequestContextHolder is not using the wrapped request, the overridden HttpSession will not be used.

Spring Security also wraps the HttpServletRequest and the HttpServletResponse to provide things like access to the current user on HttpServletRequest.getRemoteUser() and HttpServletRequest.getUserPrincipal().

These are two concrete examples of why Filters that wrap requests must be placed before the RequestContextHolder is populated and why the changes in 88cc883e9499805f90fe2c5ef82ba3816dc25930 were made.

[wilkinsona]

I understand all of that. I (hopefully) chose an order which means that RequestContextFilter goes after all filters that wrap the request but before Spring Security's filter. Note the changes to the tests that I made in the commit which now verify much of the ordering.

What I've neglected to do is to rework the REQUEST_WRAPPER_FILTER_MAX_ORDER constant. Ideally, RequestContextFilter needs to used that order +1 and Spring Security's filter needs to use that order +2 (or more). We can use this issue to do that.

[wilkinsona]

And now I've just re-read this:

Spring Security also wraps the HttpServletRequest and the HttpServletResponse to provide things like access to the current user on HttpServletRequest.getRemoteUser() and HttpServletRequest.getUserPrincipal().

If people want to be able to access Spring Security's wrapped request via RequestContextHolder then I guess we need to register RequestContextFilter with Spring Security's filter chain somehow. Correct?

[rwinch]

If people want to be able to access Spring Security's wrapped request via RequestContextHolder then I guess we need to register RequestContextFilter with Spring Security's filter chain somehow. Correct?

Sort of. More concretely, for the user to be accessible through the RequestContextHolder the RequestContextFilter must be after SecurityContextHolderAwareRequestFilter. That means it could be registered with Spring Security after SecurityContextHolderAwareRequestFilter.

However, I should point out that Spring Security also wraps the HttpServletRequest in other places. For example, RequestCacheAwareFilter uses SavedRequestAwareWrapper. Generally, it should be considered that Spring Security is going to wrap the HttpServletRequest. This means it is likely a better approach to place RequestContextFilter after Spring Security's FilterChainProxy within the servlet container itself.

I think it is important to keep in mind that this is a general problem (i.e. MultipartFilter) and the reason that the constant REQUEST_WRAPPER_FILTER_MAX_ORDER exists. This is why we have a bit of a chicken and the egg problem.

[wilkinsona]

This means it is likely a better approach to place RequestContextFilter after Spring Security's FilterChainProxy within the servlet container itself.

Unfortunately, we can't do that due to OAuth2ClientContextFilter. It runs as part of Spring Security's filter chain and uses RequestContextHolder.

[rwinch]

Unfortunately, we can't do that due to OAuth2ClientContextFilter. It runs as part of Spring Security's filter chain and uses RequestContextHolder.

Agreed. Hence the chicken and the egg problem.

[wilkinsona]

I've added an integration test that verifies the current ordering. Without a change being made to Spring Security OAuth, or a reliable position in Spring Security's filter chain where we know that all the request wrapping will have occurred that's before OAuth2ClientContextFilter, this feels like the best that we can do.

[arcseldon]

Please can we have an update here?

[wilkinsona]

@arcseldon As I said above, we believe we've done the best we can in Spring Boot at this point. I believe that we need a change in Spring Security or Spring Security OAuth to do any better. Have you encountered a specific problem?

[arcseldon]

@wilkinsona - Thanks for the clarification. Yes, i have a specific problem. Written a Spring Security / Spring Boot library for a Business - and we have had reports from users that Spring Session integration does not work.

Took a look today and set up a simple sample (spring boot at that uses spring session and redis as session store) - discovered that indeed in the Spring Security AuthenticatorProvider implementation for some reason we are not getting back the Redis aware session but a standard Session. In Redis, I can see in the hash, all the correct values, it is just that Spring Security is not receiving the wrapped Session - it looks like a Filter ordering issue.

Is there something I can do here? For instance, we do reference RequestContextHolder.currentRequestAttributes(); in that same Provider class but I am less clear if this is cause. I don't mind referencing a Spring Bean declaration for RequestContextFilter etc - just need a work around solution that users can implement when they are going to be using Spring Session.

Makes sense?

[wilkinsona]

IIRC, it should work if all you're using is Spring Session and Spring Security. Can you provide a small sample that mimics your setup and shows the problem you're having?

[arcseldon]

@wilkinsona - Spring Boot, Spring Session, and Spring Security all used together. Thanks for offering to assist here.

Re. code snippet - Sure, here is the library itself which ordinarily works unless Spring Session involved.

The line that link points at is where the failure arises because the incorrect session is being retrieved. I can see the Auth0User value in Redis, but it is incorrectly retrieving from a standard session.

Here is a hacky sample I just put together that is broken because of aforementioned issue. Put some Docker capabilities in there so it is easy to get it running without installing anything locally.

However, bear in mind it integrates with auth0 using oauth2 / openid connect so in src/main/resources/auth0.properties you would need to have legit settings to actually get this running.... if you are really keen - please set up an account!

[arcseldon]

Tried adding RequestContextFilter to the sample config out of desperation having just read several issues related to not doing so - some from @rwinch

He talks about a possible solution of sorts here - not tried it.

[wilkinsona]

@arcseldon I don't think you're suffering from the exact same problem as is described in this issue.

Spring Boot will, by default, auto-configure a RequestContextFilter for you that runs after Spring Session's filter and before Spring Security's filter. This is exactly what you want as you attempt to use the holder within Auth0AuthenticationProvider that's called by Spring Security. However, this doesn't happen in your sample.

The problem is that Auth0Config has registered a RequestContextListener bean. This switches off the auto-configured RequestContextFilter bean. Your attempt at explicitly adding one almost fixed the problem, but it was unordered so it wasn't in the right place in the filter chain. You can fix the problem by using Boot's OrderedRequestContextFilter instead:

@Bean
public OrderedRequestContextFilter requestContextFilter() {
    return new OrderedRequestContextFilter();
}

With this change made to your sample, I can log in.

[arcseldon]

@wilkinsona - Thank you for this - several customers will be happy see this fix :D Genuinely, thanks - and let me know if Auth0 ever appeals as a career option in the future. Interesting times and Spring / Spring Boot is becoming an increasingly important SDK / Library choice for our customers.

[snicoll]

According to the latest status, it looks like this should have been closed. Let me know if I missed anything.

[rupebac]

We were suffering the same problem. Spring Boot registering its own RequestContextFilter at the beginning of the chain (pos 3), but we are wrapping the HttpServletRequest at "Filter_A" (pos 5), and pulling from RequestContextHolder at "Filter_B" (pos 7). We were hoping to have our own RequestContextFilter at pos 6 (that is the order we set in FilterRegistrationBean) updating the new wrapped request from Filter_A, but it was not at that pos (in fact it was not at all in the chain, only the one from Spring Boot), and therefore Filter_B did not see the wrapped HttpServletRequest from Filter_A.

I discovered that I was registering our RequestContextFilter at pos 6 with the filter name "requestContextFilter" (same name as the one from Spring boot). Changing the filter name to "requestContextFilter2" fixed the problem.

Can someone explain why is this happening? Shouldn't it throw a warning at least?

[wilkinsona]

@rupebac It's hard to say what's happening without seeing a complete example. I can say, however, that if you declare your own RequestContextFilter bean, the one that's auto-configured by Spring Boot should back off. There shouldn't be any need for a filter registration bean as you can use Boot's OrderedRequestContextFilter and setOrder(int).

If you're observing behaviour that doesn't match what I've described above and believe you've found a bug, please open a new issue with a complete minimal example of the problem. If you have any follow-up questions, please ask on Stack Overflow or Gitter because, as mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements.

[rupebac]

@wilkinsona if you use OrderedRequestContextFilter, I understand you are moving the autoconfigured one from Spring Boot to a specific position? If I move it too far off, Spring Security will not work right?

[wilkinsona]

There's no mention of Spring Security in your description of your situation or the order that you've assigned to its filter. As I said above, please ask follow-up questions on Stack Overflow or Gitter. If you do so, please take the time to describe everything that's involved (a minimal, complete, and verifiable example is the best way to do that) so that someone can try to help you as efficiently as possible.