Spring Security Calling SecurityContextHolder.setStrategyName(strategy) breaks Spring filters

Calling SecurityContextHolder.setStrategyName(strategy) with any strategy name breaks spring filters because of code like: https://github.com/spring-projects/spring-security/blob/c4485a8909119f88559dd4200cd3506024749529/web/src/main/java/org/springframework/security/web/authentication/AbstractAuthenticationProcessingFilter.java#L118C7-L118C7

Many of our filters use SecurityContextHolder.getContext() directly, which will cause them to use a different strategy instance to look up the thread local context.

This makes changing the strategy difficult in situations where SecurityContextHolder.setStrategyName(strategy) cannot be called before Spring filters are initialized; the only solution is via system properties so that it is not reset in code

Comment From: sjohnr

@ChrisDelahunt Thanks for getting in touch, but it feels like this is a question that would be better suited to Stack Overflow. We prefer to use GitHub issues only for bugs and enhancements. Feel free to update this issue with a link to the re-posted question (so that other people can find it) or add a minimal sample that reproduces this issue if you feel this is a genuine bug.

Having said that, please take a look at the Using SecurityContextHolderStrategy section of the reference, which outlines that you can autowire a strategy via the application context.

I'm going to close this issue for now. As mentioned above, please provide a minimal reproducible example if you feel I am missing something or submit a stackoverflow question if you need further clarification and I'll be happy to take a look.

Comment From: ChrisDelahunt

This was not a question, it is a statement. Spring's use of SecurityContextHolder.getContextHolderStrategy() and then caching the returned ContextHolderStrategy means those filters caching it will be using an entirely different strategy from other filters in the same chain that may be directly using SecurityContextHolder.getContext(), so they will never see the same context.

This is breaking behavior introduced in 5.8+ where our application used an InitialServlet to call SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL); And all filters and logic in that application would automatically then start using the same MODE_INHERITABLETHREADLOCAL strategy from that moment on. Prior to 5.8 (5.7.3 is used), this was working and the same context was used through out.

After 5.8 (testing with 5.8.3), Spring filters will have cached the MODE_THREADLOCAL instance, and so continue using that, getting completely different context object (or null) from other filters still using SecurityContextHolder.getContext() for lookups. This causes NPE issues when SecurityContextHolder.getContext().getAuthorization() is expected to to exist, making 5.8 and up a breaking change.

Nothing within https://docs.spring.io/spring-security/reference/servlet/authentication/session-management.html#use-securitycontextholderstrategy seems to address this problem, or provides a way to set the strategy for use.

The workaround is to use -Dspring.security.strategy=MODE_INHERITABLETHREADLOCAL when starting the server, which is less than ideal to rely on, but is the only way to ensure the value is set before Spring starts caching the strategies without much more involved options and changes

Comment From: sjohnr

is the only way to ensure the value is set before Spring starts caching the strategies without much more involved options and changes

To ensure we are on the same page, can you please provide the details around other options you're referring to? From your earlier comment:

This makes changing the strategy difficult in situations where SecurityContextHolder.setStrategyName(strategy) cannot be called before Spring filters are initialized; the only solution is via system properties so that it is not reset in code

This statement is why I included a link to the docs and mentioned that you can publish an @Bean to change the strategy.

This is breaking behavior introduced in 5.8+

After 5.8 (testing with 5.8.3), Spring filters will have cached the MODE_THREADLOCAL instance, and so continue using that, getting completely different context object (or null) from other filters still using SecurityContextHolder.getContext() for lookups. This causes NPE issues when SecurityContextHolder.getContext().getAuthorization() is expected to to exist, making 5.8 and up a breaking change.

I am happy to re-open this issue and look into this. It would be very helpful if you could provide a minimal reproducible example. Would you be able to provide one?

Comment From: ChrisDelahunt

Thank you [ @sjohnr ] for revisiting.

I can try to set a test case up, but as with everyone, time is limited and all I have access to now is a monolith application that is using a HttpServlet instance to call SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL); during its initialization. Our filters will then get NPEs when calling SecurityContextHolder.getContext().getAuthentication()

Code issue seems straighforward though; and setStrategyName call causes Spring filters to use a different strategy from the static SecurityContextHolder strategy from that point on. It either works in some places, or just doesn't work at all. As the filters are initialized before servlets, it just cannot work for them when they cache the strategy holder, and requires changes on the apps part to ensure the strategy is switched sooner - hence it was a breaking changing. I can test with publishing a bean in the application as a workaround and see if it gets called before the filters are initialized, but that was what our servlet was for.

IMO the strategy switching feature is missing something. The SecurityContextHolder has an initialization counter, but no way for callers using and caching the strategy holders to know it has changed out from under them, or use it in any way to get the latest should they be required to. It fixes one problem, but leaves open this one which according to the docs, is still a valid way to set it with no direction or indications given that the order it should be set in is now important.

Comment From: sjohnr

Hi @ChrisDelahunt, thanks for the follow up.

all I have access to now is a monolith application that is using a HttpServlet instance to call SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL); during its initialization.

This helps me visualize a little bit of what you're trying to do, but providing even a code snippet of how you're setting the strategy (specifically what the servlet init code looks like and/or what method in the servlet spec is being used as a hook to make the call) would be super helpful.

It fixes one problem, but leaves open this one which according to the docs, is still a valid way to set it with no direction or indications given that the order it should be set in is now important.

This is a good point, and part of what we can look into along with a potential fix. In the meantime, can you try a workaround to set the strategyName during application initialization? If you have an @Configuration or any bean that is registered with the application context, add a @PostConstruct as in the following example:

@Configuration
@EnableWebSecurity
public class SecurityConfiguration {

    @PostConstruct
    public void init() {
        System.out.println("Setting strategy in init...");
        SecurityContextHolder.setStrategyName(SecurityContextHolder.MODE_INHERITABLETHREADLOCAL);
    }

    // ...

}

Comment From: spring-projects-issues

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

Comment From: spring-projects-issues

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

Comment From: ChrisDelahunt

I have not had the chance to create a small test case, but this should remain open as it is very obviously still a bug:

The documents state that SecurityContextHolder.setStrategyName is the way to set the strategy name, but using this any point after filter initialization will cause filters that cache the strategy to use different context holder instances from those that don't. Documents don't state any restrictions or guidelines on when SecurityContextHolder.setStrategyName should or should not be called, so this behavior change is breaking backward compatibility. It shouldn't be too difficult to fix either, though I admit I'm not familiar on the problem that drove the strategy holder to be cached within filters. Seems this logic is missing a check back on the SecurityContextHolder for it to allow changing the strategy.

I'm not sure that a bean component would be an acceptable workaround for our application in place of HttpServlet for properties when we are already now using the -Dspring.security.strategy=MODE_INHERITABLETHREADLOCAL option. The Java option might be preferable to any and all loading/initialization issues should Spring internals change yet again.

Comment From: sjohnr

Thanks for following up, @ChrisDelahunt. Please let me know when you have tried the workaround I've mentioned above as it will inform whether I'm understanding your issue correctly or not and how we might go about making any related changes.

Comment From: prettyvoid

@sjohnr I had the same case but the workaround you suggested works fine. Is there any other way to do this instead of initing in postconstruct?

Comment From: gjbaxter

This issue is still live. The joys of singletons! The Filters in question should not be maintaining a hold on the strategy. Maybe the strategy inside the SecurityContextHolder shouldn't be exposed. Regardless, the Filters should request the context, or if necessary the strategy, from the SecurityContextHolder each and every time.