From 83b3bb32096ab3e4f6231e0bb266ad7205eb18c9 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Tue, 21 Jun 2022 16:42:16 -0600 Subject: [PATCH] Add SecurityContextHolderStrategy to Pre-authenticated scenarios Issue gh-11060 Issue gh-11061 --- .../web/configurers/JeeConfigurer.java | 9 ++-- .../web/configurers/X509Configurer.java | 5 ++- .../http/AuthenticationConfigBuilder.java | 14 +++++-- .../web/configurers/X509ConfigurerTests.java | 21 ++++++++++ .../config/http/MiscHttpConfigTests.java | 34 +++++++++++++++ ...ilterWithSecurityContextHolderStrategy.xml | 39 +++++++++++++++++ ...-X509WithSecurityContextHolderStrategy.xml | 42 +++++++++++++++++++ ...tractPreAuthenticatedProcessingFilter.java | 32 ++++++++++---- 8 files changed, 179 insertions(+), 17 deletions(-) create mode 100644 config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-JeeFilterWithSecurityContextHolderStrategy.xml create mode 100644 config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-X509WithSecurityContextHolderStrategy.xml diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/JeeConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/JeeConfigurer.java index 6a16498975..0862b8b02c 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/JeeConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/JeeConfigurer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -198,7 +198,8 @@ public final class JeeConfigurer> extends Abstr @Override public void configure(H http) { - J2eePreAuthenticatedProcessingFilter filter = getFilter(http.getSharedObject(AuthenticationManager.class)); + J2eePreAuthenticatedProcessingFilter filter = getFilter(http.getSharedObject(AuthenticationManager.class), + http); http.addFilter(filter); } @@ -208,12 +209,14 @@ public final class JeeConfigurer> extends Abstr * @param authenticationManager the {@link AuthenticationManager} to use. * @return the {@link J2eePreAuthenticatedProcessingFilter} to use. */ - private J2eePreAuthenticatedProcessingFilter getFilter(AuthenticationManager authenticationManager) { + private J2eePreAuthenticatedProcessingFilter getFilter(AuthenticationManager authenticationManager, H http) { if (this.j2eePreAuthenticatedProcessingFilter == null) { this.j2eePreAuthenticatedProcessingFilter = new J2eePreAuthenticatedProcessingFilter(); this.j2eePreAuthenticatedProcessingFilter.setAuthenticationManager(authenticationManager); this.j2eePreAuthenticatedProcessingFilter .setAuthenticationDetailsSource(createWebAuthenticationDetailsSource()); + this.j2eePreAuthenticatedProcessingFilter + .setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); this.j2eePreAuthenticatedProcessingFilter = postProcess(this.j2eePreAuthenticatedProcessingFilter); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java index 15736e5498..69b85ecd13 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/X509Configurer.java @@ -183,11 +183,11 @@ public final class X509Configurer> @Override public void configure(H http) { - X509AuthenticationFilter filter = getFilter(http.getSharedObject(AuthenticationManager.class)); + X509AuthenticationFilter filter = getFilter(http.getSharedObject(AuthenticationManager.class), http); http.addFilter(filter); } - private X509AuthenticationFilter getFilter(AuthenticationManager authenticationManager) { + private X509AuthenticationFilter getFilter(AuthenticationManager authenticationManager, H http) { if (this.x509AuthenticationFilter == null) { this.x509AuthenticationFilter = new X509AuthenticationFilter(); this.x509AuthenticationFilter.setAuthenticationManager(authenticationManager); @@ -197,6 +197,7 @@ public final class X509Configurer> if (this.authenticationDetailsSource != null) { this.x509AuthenticationFilter.setAuthenticationDetailsSource(this.authenticationDetailsSource); } + this.x509AuthenticationFilter.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); this.x509AuthenticationFilter = postProcess(this.x509AuthenticationFilter); } 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 87be60a3a8..f67c700412 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 @@ -236,8 +236,8 @@ final class AuthenticationConfigBuilder { createOAuth2ClientFilters(sessionStrategy, requestCache, authenticationManager, authenticationFilterSecurityContextRepositoryRef); createSaml2LoginFilter(authenticationManager, authenticationFilterSecurityContextRepositoryRef); - createX509Filter(authenticationManager); - createJeeFilter(authenticationManager); + createX509Filter(authenticationManager, authenticationFilterSecurityContextHolderStrategyRef); + createJeeFilter(authenticationManager, authenticationFilterSecurityContextHolderStrategyRef); createLogoutFilter(authenticationFilterSecurityContextHolderStrategyRef); createSaml2LogoutFilter(); createLoginPageFilterIfNeeded(); @@ -488,7 +488,8 @@ final class AuthenticationConfigBuilder { this.bearerTokenAuthenticationFilter = resourceServerBuilder.parse(resourceServerElt, this.pc); } - void createX509Filter(BeanReference authManager) { + void createX509Filter(BeanReference authManager, + BeanMetadataElement authenticationFilterSecurityContextHolderStrategyRef) { Element x509Elt = DomUtils.getChildElementByTagName(this.httpElt, Elements.X509); RootBeanDefinition filter = null; if (x509Elt != null) { @@ -496,6 +497,8 @@ final class AuthenticationConfigBuilder { .rootBeanDefinition(X509AuthenticationFilter.class); filterBuilder.getRawBeanDefinition().setSource(this.pc.extractSource(x509Elt)); filterBuilder.addPropertyValue("authenticationManager", authManager); + filterBuilder.addPropertyValue("securityContextHolderStrategy", + authenticationFilterSecurityContextHolderStrategyRef); String regex = x509Elt.getAttribute("subject-principal-regex"); if (StringUtils.hasText(regex)) { BeanDefinitionBuilder extractor = BeanDefinitionBuilder @@ -536,7 +539,8 @@ final class AuthenticationConfigBuilder { } } - void createJeeFilter(BeanReference authManager) { + void createJeeFilter(BeanReference authManager, + BeanMetadataElement authenticationFilterSecurityContextHolderStrategyRef) { Element jeeElt = DomUtils.getChildElementByTagName(this.httpElt, Elements.JEE); RootBeanDefinition filter = null; if (jeeElt != null) { @@ -544,6 +548,8 @@ final class AuthenticationConfigBuilder { .rootBeanDefinition(J2eePreAuthenticatedProcessingFilter.class); filterBuilder.getRawBeanDefinition().setSource(this.pc.extractSource(jeeElt)); filterBuilder.addPropertyValue("authenticationManager", authManager); + filterBuilder.addPropertyValue("securityContextHolderStrategy", + authenticationFilterSecurityContextHolderStrategyRef); BeanDefinitionBuilder adsBldr = BeanDefinitionBuilder .rootBeanDefinition(J2eeBasedPreAuthenticatedWebAuthenticationDetailsSource.class); adsBldr.addPropertyValue("userRoles2GrantedAuthoritiesMapper", diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/X509ConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/X509ConfigurerTests.java index e2bc35bbd1..b32dbc344f 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/X509ConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/X509ConfigurerTests.java @@ -28,24 +28,30 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.core.io.ClassPathResource; 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.SecurityContextChangedListener; +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.provisioning.InMemoryUserDetailsManager; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken; import org.springframework.security.web.authentication.preauth.x509.X509AuthenticationFilter; import org.springframework.test.web.servlet.MockMvc; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.springframework.security.config.Customizer.withDefaults; +import static org.springframework.security.config.annotation.SecurityContextChangedListenerArgumentMatchers.setAuthentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.x509; import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.authenticated; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; @@ -90,6 +96,21 @@ public class X509ConfigurerTests { // @formatter:on } + @Test + public void x509WhenCustomSecurityContextHolderStrategyThenUses() throws Exception { + this.spring.register(DefaultsInLambdaConfig.class, SecurityContextChangedListenerConfig.class).autowire(); + X509Certificate certificate = loadCert("rod.cer"); + // @formatter:off + this.mvc.perform(get("/").with(x509(certificate))) + .andExpect(authenticated().withUsername("rod")); + // @formatter:on + SecurityContextHolderStrategy strategy = this.spring.getContext().getBean(SecurityContextHolderStrategy.class); + verify(strategy, atLeastOnce()).getContext(); + SecurityContextChangedListener listener = this.spring.getContext() + .getBean(SecurityContextChangedListener.class); + verify(listener).securityContextChanged(setAuthentication(PreAuthenticatedAuthenticationToken.class)); + } + @Test public void x509WhenSubjectPrincipalRegexInLambdaThenUsesRegexToExtractPrincipal() throws Exception { this.spring.register(SubjectPrincipalRegexInLambdaConfig.class).autowire(); diff --git a/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java b/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java index e1b327d6c8..3e2b98f91f 100644 --- a/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java +++ b/config/src/test/java/org/springframework/security/config/http/MiscHttpConfigTests.java @@ -76,6 +76,7 @@ import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.security.core.authority.AuthorityUtils; 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.core.context.SecurityContextImpl; import org.springframework.security.web.AuthenticationEntryPoint; import org.springframework.security.web.FilterChainProxy; @@ -380,6 +381,19 @@ public class MiscHttpConfigTests { // @formatter:on } + @Test + public void getWhenUsingX509CustomSecurityContextHolderStrategyThenUses() throws Exception { + System.setProperty("subject_principal_regex", "OU=(.*?)(?:,|$)"); + this.spring.configLocations(xml("X509WithSecurityContextHolderStrategy")).autowire(); + RequestPostProcessor x509 = x509( + "classpath:org/springframework/security/config/http/MiscHttpConfigTests-certificate.pem"); + // @formatter:off + this.mvc.perform(get("/protected").with(x509)) + .andExpect(status().isOk()); + // @formatter:on + verify(this.spring.getContext().getBean(SecurityContextHolderStrategy.class), atLeastOnce()).getContext(); + } + @Test public void configureWhenUsingInvalidLogoutSuccessUrlThenThrowsException() { assertThatExceptionOfType(BeanCreationException.class) @@ -652,6 +666,26 @@ public class MiscHttpConfigTests { // @formatter:on } + @Test + public void loginWhenJeeFilterCustomSecurityContextHolderStrategyThenUses() throws Exception { + this.spring.configLocations(xml("JeeFilterWithSecurityContextHolderStrategy")).autowire(); + Principal user = mock(Principal.class); + given(user.getName()).willReturn("joe"); + // @formatter:off + MockHttpServletRequestBuilder rolesRequest = get("/roles") + .principal(user) + .with((request) -> { + request.addUserRole("admin"); + request.addUserRole("user"); + request.addUserRole("unmapped"); + return request; + }); + this.mvc.perform(rolesRequest) + .andExpect(content().string("ROLE_admin,ROLE_user")); + // @formatter:on + verify(this.spring.getContext().getBean(SecurityContextHolderStrategy.class), atLeastOnce()).getContext(); + } + @Test public void loginWhenUsingCustomAuthenticationDetailsSourceRefThenAuthenticationSourcesDetailsAccordingly() throws Exception { diff --git a/config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-JeeFilterWithSecurityContextHolderStrategy.xml b/config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-JeeFilterWithSecurityContextHolderStrategy.xml new file mode 100644 index 0000000000..2b349246e3 --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-JeeFilterWithSecurityContextHolderStrategy.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + + + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-X509WithSecurityContextHolderStrategy.xml b/config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-X509WithSecurityContextHolderStrategy.xml new file mode 100644 index 0000000000..1904e4e67e --- /dev/null +++ b/config/src/test/resources/org/springframework/security/config/http/MiscHttpConfigTests-X509WithSecurityContextHolderStrategy.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java b/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java index 9d4db0287b..3166677c1c 100755 --- a/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java +++ b/web/src/main/java/org/springframework/security/web/authentication/preauth/AbstractPreAuthenticatedProcessingFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,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.WebAttributes; import org.springframework.security.web.authentication.AuthenticationFailureHandler; import org.springframework.security.web.authentication.AuthenticationSuccessHandler; @@ -88,6 +89,9 @@ import org.springframework.web.filter.GenericFilterBean; public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFilterBean implements ApplicationEventPublisherAware { + private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder + .getContextHolderStrategy(); + private ApplicationEventPublisher eventPublisher = null; private AuthenticationDetailsSource authenticationDetailsSource = new WebAuthenticationDetailsSource(); @@ -132,8 +136,8 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi throws IOException, ServletException { if (this.requiresAuthenticationRequestMatcher.matches((HttpServletRequest) request)) { if (logger.isDebugEnabled()) { - logger.debug(LogMessage - .of(() -> "Authenticating " + SecurityContextHolder.getContext().getAuthentication())); + logger.debug(LogMessage.of( + () -> "Authenticating " + this.securityContextHolderStrategy.getContext().getAuthentication())); } doAuthenticate((HttpServletRequest) request, (HttpServletResponse) response); } @@ -211,9 +215,9 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi protected void successfulAuthentication(HttpServletRequest request, HttpServletResponse response, Authentication authResult) throws IOException, ServletException { this.logger.debug(LogMessage.format("Authentication success: %s", authResult)); - SecurityContext context = SecurityContextHolder.createEmptyContext(); + SecurityContext context = this.securityContextHolderStrategy.createEmptyContext(); context.setAuthentication(authResult); - SecurityContextHolder.setContext(context); + this.securityContextHolderStrategy.setContext(context); this.securityContextRepository.saveContext(context, request, response); if (this.eventPublisher != null) { this.eventPublisher.publishEvent(new InteractiveAuthenticationSuccessEvent(authResult, this.getClass())); @@ -231,7 +235,7 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi */ protected void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, AuthenticationException failed) throws IOException, ServletException { - SecurityContextHolder.clearContext(); + this.securityContextHolderStrategy.clearContext(); this.logger.debug("Cleared security context due to exception", failed); request.setAttribute(WebAttributes.AUTHENTICATION_EXCEPTION, failed); if (this.authenticationFailureHandler != null) { @@ -335,6 +339,17 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi this.requiresAuthenticationRequestMatcher = requiresAuthenticationRequestMatcher; } + /** + * 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; + } + /** * Override to extract the principal information from the current request */ @@ -354,7 +369,8 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi @Override public boolean matches(HttpServletRequest request) { - Authentication currentUser = SecurityContextHolder.getContext().getAuthentication(); + Authentication currentUser = AbstractPreAuthenticatedProcessingFilter.this.securityContextHolderStrategy + .getContext().getAuthentication(); if (currentUser == null) { return true; } @@ -367,7 +383,7 @@ public abstract class AbstractPreAuthenticatedProcessingFilter extends GenericFi AbstractPreAuthenticatedProcessingFilter.this.logger .debug("Pre-authenticated principal has changed and will be reauthenticated"); if (AbstractPreAuthenticatedProcessingFilter.this.invalidateSessionOnPrincipalChange) { - SecurityContextHolder.clearContext(); + AbstractPreAuthenticatedProcessingFilter.this.securityContextHolderStrategy.clearContext(); HttpSession session = request.getSession(false); if (session != null) { AbstractPreAuthenticatedProcessingFilter.this.logger.debug("Invalidating existing session");