From 944f565c161877800f8bd119b86c027d8543a028 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 21 Jun 2022 16:42:47 -0600 Subject: [PATCH] Use SecurityContextHolderStrategy for Remember-me Issue gh-11060 Isuse gh-11061 --- .../web/configurers/RememberMeConfigurer.java | 1 + .../http/AuthenticationConfigBuilder.java | 7 +-- .../http/RememberMeBeanDefinitionParser.java | 9 +++- .../RememberMeConfigurerTests.java | 29 ++++++++++--- .../config/http/RememberMeConfigTests.java | 13 ++++++ ...ests-WithSecurityContextHolderStrategy.xml | 43 +++++++++++++++++++ .../RememberMeAuthenticationFilter.java | 27 +++++++++--- 7 files changed, 114 insertions(+), 15 deletions(-) create mode 100644 config/src/test/resources/org/springframework/security/config/http/RememberMeConfigTests-WithSecurityContextHolderStrategy.xml diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java index 24f57580e1..ad4e1c082b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurer.java @@ -293,6 +293,7 @@ public final class RememberMeConfigurer> if (this.authenticationSuccessHandler != null) { rememberMeFilter.setAuthenticationSuccessHandler(this.authenticationSuccessHandler); } + rememberMeFilter.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); rememberMeFilter = postProcess(rememberMeFilter); http.addFilter(rememberMeFilter); } diff --git a/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java b/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java index a09e36cf08..87be60a3a8 100644 --- a/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java +++ b/config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java @@ -228,7 +228,7 @@ final class AuthenticationConfigBuilder { this.portResolver = portResolver; this.csrfLogoutHandler = csrfLogoutHandler; createAnonymousFilter(authenticationFilterSecurityContextHolderStrategyRef); - createRememberMeFilter(authenticationManager); + createRememberMeFilter(authenticationManager, authenticationFilterSecurityContextHolderStrategyRef); createBasicFilter(authenticationManager, authenticationFilterSecurityContextHolderStrategyRef); createBearerTokenAuthenticationFilter(authenticationManager); createFormLoginFilter(sessionStrategy, authenticationManager, @@ -245,7 +245,8 @@ final class AuthenticationConfigBuilder { createExceptionTranslationFilter(authenticationFilterSecurityContextHolderStrategyRef); } - void createRememberMeFilter(BeanReference authenticationManager) { + void createRememberMeFilter(BeanReference authenticationManager, + BeanMetadataElement authenticationFilterSecurityContextHolderStrategyRef) { // Parse remember me before logout as RememberMeServices is also a LogoutHandler // implementation. Element rememberMeElt = DomUtils.getChildElementByTagName(this.httpElt, Elements.REMEMBER_ME); @@ -255,7 +256,7 @@ final class AuthenticationConfigBuilder { key = createKey(); } RememberMeBeanDefinitionParser rememberMeParser = new RememberMeBeanDefinitionParser(key, - authenticationManager); + authenticationManager, authenticationFilterSecurityContextHolderStrategyRef); this.rememberMeFilter = rememberMeParser.parse(rememberMeElt, this.pc); this.rememberMeServicesId = rememberMeParser.getRememberMeServicesId(); createRememberMeProvider(key); diff --git a/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java index 85ac360cf6..bd23e29424 100644 --- a/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/RememberMeBeanDefinitionParser.java @@ -20,6 +20,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.w3c.dom.Element; +import org.springframework.beans.BeanMetadataElement; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanReference; import org.springframework.beans.factory.config.RuntimeBeanReference; @@ -70,11 +71,15 @@ class RememberMeBeanDefinitionParser implements BeanDefinitionParser { private final BeanReference authenticationManager; + private final BeanMetadataElement authenticationFilterSecurityContextHolderStrategyRef; + private String rememberMeServicesId; - RememberMeBeanDefinitionParser(String key, BeanReference authenticationManager) { + RememberMeBeanDefinitionParser(String key, BeanReference authenticationManager, + BeanMetadataElement authenticationFilterSecurityContextHolderStrategyRef) { this.key = key; this.authenticationManager = authenticationManager; + this.authenticationFilterSecurityContextHolderStrategyRef = authenticationFilterSecurityContextHolderStrategyRef; } @Override @@ -175,6 +180,8 @@ class RememberMeBeanDefinitionParser implements BeanDefinitionParser { } filter.addConstructorArgValue(this.authenticationManager); filter.addConstructorArgReference(servicesName); + filter.addPropertyValue("securityContextHolderStrategy", + this.authenticationFilterSecurityContextHolderStrategyRef); pc.popAndRegisterContainingComponent(); return filter.getBeanDefinition(); } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java index 56d0a41330..b9a7580e26 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RememberMeConfigurerTests.java @@ -30,12 +30,14 @@ import org.springframework.mock.web.MockHttpSession; import org.springframework.security.authentication.RememberMeAuthenticationToken; import org.springframework.security.authentication.dao.DaoAuthenticationProvider; import org.springframework.security.config.annotation.ObjectPostProcessor; +import org.springframework.security.config.annotation.SecurityContextChangedListenerConfig; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.core.userdetails.PasswordEncodedUser; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetailsService; @@ -55,6 +57,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -100,7 +103,8 @@ public class RememberMeConfigurerTests { @Test public void configureWhenRegisteringObjectPostProcessorThenInvokedOnRememberMeAuthenticationFilter() { this.spring.register(ObjectPostProcessorConfig.class).autowire(); - verify(ObjectPostProcessorConfig.objectPostProcessor).postProcess(any(RememberMeAuthenticationFilter.class)); + verify(this.spring.getContext().getBean(ObjectPostProcessor.class)) + .postProcess(any(RememberMeAuthenticationFilter.class)); } @Test @@ -131,6 +135,21 @@ public class RememberMeConfigurerTests { this.mvc.perform(request).andExpect(remembermeAuthentication); } + @Test + public void rememberMeWhenCustomSecurityContextHolderStrategyThenUses() throws Exception { + this.spring.register(UserDetailsServiceBeanConfig.class, SecurityContextChangedListenerConfig.class).autowire(); + MvcResult mvcResult = this.mvc.perform(post("/login").with(csrf()).param("username", "user") + .param("password", "password").param("remember-me", "true")).andReturn(); + Cookie rememberMeCookie = mvcResult.getResponse().getCookie("remember-me"); + // @formatter:off + MockHttpServletRequestBuilder request = get("/abc").cookie(rememberMeCookie); + SecurityMockMvcResultMatchers.AuthenticatedMatcher remembermeAuthentication = authenticated() + .withAuthentication((auth) -> assertThat(auth).isInstanceOf(RememberMeAuthenticationToken.class)); + // @formatter:on + this.mvc.perform(request).andExpect(remembermeAuthentication); + verify(this.spring.getContext().getBean(SecurityContextHolderStrategy.class), atLeastOnce()).getContext(); + } + @Test public void loginWhenRememberMeTrueThenRespondsWithRememberMeCookie() throws Exception { this.spring.register(RememberMeConfig.class).autowire(); @@ -315,14 +334,14 @@ public class RememberMeConfigurerTests { @EnableWebSecurity static class ObjectPostProcessorConfig extends WebSecurityConfigurerAdapter { - static ObjectPostProcessor objectPostProcessor = spy(ReflectingObjectPostProcessor.class); + ObjectPostProcessor objectPostProcessor = spy(ReflectingObjectPostProcessor.class); @Override protected void configure(HttpSecurity http) throws Exception { // @formatter:off http .rememberMe() - .userDetailsService(new AuthenticationManagerBuilder(objectPostProcessor).getDefaultUserDetailsService()); + .userDetailsService(new AuthenticationManagerBuilder(this.objectPostProcessor).getDefaultUserDetailsService()); // @formatter:on } @@ -335,8 +354,8 @@ public class RememberMeConfigurerTests { } @Bean - static ObjectPostProcessor objectPostProcessor() { - return objectPostProcessor; + ObjectPostProcessor objectPostProcessor() { + return this.objectPostProcessor; } } diff --git a/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java b/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java index 86a4c213df..14d328c047 100644 --- a/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/RememberMeConfigTests.java @@ -29,6 +29,7 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.security.TestDataSource; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.core.userdetails.User; import org.springframework.security.core.userdetails.UserDetailsService; import org.springframework.security.web.authentication.rememberme.AbstractRememberMeServices; @@ -219,6 +220,18 @@ public class RememberMeConfigTests { () -> this.spring.configLocations(xml("NegativeTokenValidityWithPersistentRepository")).autowire()); } + @Test + public void rememberMeWhenCustomSecurityContextHolderStrategyThenUses() throws Exception { + this.spring.configLocations(xml("WithSecurityContextHolderStrategy")).autowire(); + MvcResult result = rememberAuthentication("user", "password").andReturn(); + Cookie cookie = rememberMeCookie(result); + // @formatter:off + this.mvc.perform(get("/authenticated").cookie(cookie)) + .andExpect(status().isOk()); + // @formatter:on + verify(this.spring.getContext().getBean(SecurityContextHolderStrategy.class), atLeastOnce()).getContext(); + } + @Test public void requestWithRememberMeWhenUsingCustomUserDetailsServiceThenInvokesThisUserDetailsService() throws Exception { diff --git a/config/src/test/resources/org/springframework/security/config/http/RememberMeConfigTests-WithSecurityContextHolderStrategy.xml b/config/src/test/resources/org/springframework/security/config/http/RememberMeConfigTests-WithSecurityContextHolderStrategy.xml new file mode 100644 index 0000000000..69b32f0490 --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/RememberMeConfigTests-WithSecurityContextHolderStrategy.xml @@ -0,0 +1,43 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java index aea8c80419..292ddb8bf6 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/RememberMeAuthenticationFilter.java @@ -34,6 +34,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.core.context.SecurityContextHolderStrategy; import org.springframework.security.web.authentication.AuthenticationSuccessHandler; import org.springframework.security.web.authentication.RememberMeServices; import org.springframework.security.web.context.NullSecurityContextRepository; @@ -67,6 +68,9 @@ import org.springframework.web.filter.GenericFilterBean; */ public class RememberMeAuthenticationFilter extends GenericFilterBean implements ApplicationEventPublisherAware { + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + private ApplicationEventPublisher eventPublisher; private AuthenticationSuccessHandler successHandler; @@ -99,10 +103,10 @@ public class RememberMeAuthenticationFilter extends GenericFilterBean implements private void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - if (SecurityContextHolder.getContext().getAuthentication() != null) { + if (this.securityContextHolderStrategy.getContext().getAuthentication() != null) { this.logger.debug(LogMessage .of(() -> "SecurityContextHolder not populated with remember-me token, as it already contained: '" - + SecurityContextHolder.getContext().getAuthentication() + "'")); + + this.securityContextHolderStrategy.getContext().getAuthentication() + "'")); chain.doFilter(request, response); return; } @@ -112,16 +116,16 @@ public class RememberMeAuthenticationFilter extends GenericFilterBean implements try { rememberMeAuth = this.authenticationManager.authenticate(rememberMeAuth); // Store to SecurityContextHolder - SecurityContext context = SecurityContextHolder.createEmptyContext(); + SecurityContext context = this.securityContextHolderStrategy.createEmptyContext(); context.setAuthentication(rememberMeAuth); - SecurityContextHolder.setContext(context); + this.securityContextHolderStrategy.setContext(context); onSuccessfulAuthentication(request, response, rememberMeAuth); this.logger.debug(LogMessage.of(() -> "SecurityContextHolder populated with remember-me token: '" - + SecurityContextHolder.getContext().getAuthentication() + "'")); + + this.securityContextHolderStrategy.getContext().getAuthentication() + "'")); this.securityContextRepository.saveContext(context, request, response); if (this.eventPublisher != null) { this.eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent( - SecurityContextHolder.getContext().getAuthentication(), this.getClass())); + this.securityContextHolderStrategy.getContext().getAuthentication(), this.getClass())); } if (this.successHandler != null) { this.successHandler.onAuthenticationSuccess(request, response, rememberMeAuth); @@ -196,4 +200,15 @@ public class RememberMeAuthenticationFilter extends GenericFilterBean implements this.securityContextRepository = securityContextRepository; } + /** + * Sets the {@link SecurityContextHolderStrategy} to use. The default action is to use + * the {@link SecurityContextHolderStrategy} stored in {@link SecurityContextHolder}. + * + * @since 5.8 + */ + public void setSecurityContextHolderStrategy(SecurityContextHolderStrategy securityContextHolderStrategy) { + Assert.notNull(securityContextHolderStrategy, "securityContextHolderStrategy cannot be null"); + this.securityContextHolderStrategy = securityContextHolderStrategy; + } + }