Skip to content

Spring Security 6.3.0 SecurityContextHolder thread error #15796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vsTianhao opened this issue Sep 12, 2024 · 3 comments
Closed

Spring Security 6.3.0 SecurityContextHolder thread error #15796

vsTianhao opened this issue Sep 12, 2024 · 3 comments
Assignees
Labels
status: invalid An issue that we don't feel is valid type: bug A general bug

Comments

@vsTianhao
Copy link

like #13866, But there seems to be a difference.

WebSecurityConfig:

@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
  return http.securityContext(c -> c.requireExplicitSave(true)).formLogin(e -> e.loginPage("/login").permitAll())
	.logout(e -> e.invalidateHttpSession(true).logoutUrl("/user/logout").logoutSuccessUrl("/logout/success").permitAll())
	.exceptionHandling(e -> e.accessDeniedPage("/defied").authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint("/login")))
	.with(customAuthenticationDsl, Customizer.withDefaults())
	.csrf(CsrfConfigurer::disable).cors(CorsConfigurer::disable)
	.authenticationProvider(customAuthenticationProvider).build();
}

@Bean
static SecurityContextHolderStrategy securityContextHolderStrategy() {
  SecurityContextHolder.setStrategyName(MODE_INHERITABLETHREADLOCAL);//The effect is the same as MODE_THREADLOCAL
  return SecurityContextHolder.getContextHolderStrategy();
}

@Bean
static MethodSecurityExpressionHandler methodSecurityExpressionHandler() {
  return new DefaultMethodSecurityExpressionHandler();
}

customAuthenticationProvider:
I'm debugging here to get the thread name

@Service
public class CustomAuthenticationProvider implements AuthenticationProvider {

	@Override
	public Authentication authenticate(Authentication authentication) throws AuthenticationException {
		SecurityContextHolder.getContextHolderStrategy().setContext(new SecurityContextImpl(authentication));
		// current thread: Thread[http-nio-8080-exec-4,5,main]
		return new UsernamePasswordAuthenticationToken("test", authentication.getCredentials().toString(), null);
	}
}

I continued to debug backwards (making sure it was the same HTTP request), Reach this class: org.springframework.security.web.access.intercept.AuthorizationFilter

	@Override
	public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain chain)
			throws ServletException, IOException {

		HttpServletRequest request = (HttpServletRequest) servletRequest;
		HttpServletResponse response = (HttpServletResponse) servletResponse;

		if (this.observeOncePerRequest && isApplied(request)) {
			chain.doFilter(request, response);
			return;
		}

		if (skipDispatch(request)) {
			chain.doFilter(request, response);
			return;
		}

		String alreadyFilteredAttributeName = getAlreadyFilteredAttributeName();
		request.setAttribute(alreadyFilteredAttributeName, Boolean.TRUE);
		try {
			AuthorizationDecision decision = this.authorizationManager.check(this::getAuthentication, request);
			//DEBUG: getAuthentication() is null
			//DEBUG: current thread: Thread[http-nio-8080-exec-2,5,main]
			this.eventPublisher.publishAuthorizationEvent(this::getAuthentication, request, decision);
			if (decision != null && !decision.isGranted()) {
				throw new AuthorizationDeniedException("Access Denied", decision);
			}
			chain.doFilter(request, response);
		}
		finally {
			request.removeAttribute(alreadyFilteredAttributeName);
		}
	}

Here is the problem, a request is executed by two different threads:

  • CustomAuthenticationProvider: Thread[http-nio-8080-exec-4,5,main] SecurityContextHolder has authentication
  • AuthorizationFilter: Thread[http-nio-8080-exec-2,5,main] SecurityContextHolder authentication is null

These two threads are on the same level, there is no hierarchical relationship, so the InheritableThreadLocalSecurityContextHolderStrategy is also the same as the ThreadLocalSecurityContextHolderStrategy, also gets null in AuthorizationFilter as well.

Rightfully so, it goes to ExceptionTranslationFilter and the login fails

There was no such problem in spring 5, but now I can't achieve any login.

@vsTianhao vsTianhao added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Sep 12, 2024
@vsTianhao
Copy link
Author

In the process of upgrading from spring 5 to 6, I didn't change the logical code or set different configuration values, I just followed the changes in spring 6 and changed the way it was called, like .csrf().disable() => .csrf(CsrfConfigurer::disable)

@vsTianhao
Copy link
Author

vsTianhao commented Sep 12, 2024

I seem to have solved the problem, but I don't know if it's proper.

set SecurityContextRepository:

@Bean
public SecurityContextRepository securityContextRepository() {
return new HttpSessionSecurityContextRepository();
}
//in HttpSecurity config:
http.securityContext(c -> c.requireExplicitSave(true).securityContextRepository(securityContextRepository()))

Save the SecurityContextHolder after setting it.

@Autowired
private SecurityContextRepository securityContextRepository;
SecurityContextImpl securityContext = new SecurityContextImpl(auth);
SecurityContextHolder.getContextHolderStrategy().setContext(securityContext);
securityContextRepository.saveContext(securityContext, request, response);

But before I upgraded spring6, there was SecurityContextHolder automatically, no need to manually write SecurityContextHolder.getContextHolderStrategy().setContext(securityContext);, now I have to set these up manually

I think the problem is solved.

@marcusdacoregio
Copy link
Contributor

Hi @vsTianhao. I'm glad that you figured out what happened. Spring Security 6 requires explicit save of the SecurityContext.

@marcusdacoregio marcusdacoregio added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 13, 2024
@marcusdacoregio marcusdacoregio self-assigned this Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants