Skip to content

Commit 9cb668a

Browse files
rwinchrwinch
committed
SessionManagementConfigurer properly defaults SecurityContextRepository
Previously the default was an HttpSessionSecurityContextRepository which meant that if a stateless authentication occurred the SecurityContext would be lost on ERROR dispatch. This commit ensures that the RequestAttributeSecurityContextRepository is also consulted by default. Closes gh-12070
1 parent a4858d9 commit 9cb668a

File tree

3 files changed

+109
-3
lines changed

3 files changed

+109
-3
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurer.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@
4848
import org.springframework.security.web.authentication.session.RegisterSessionAuthenticationStrategy;
4949
import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
5050
import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy;
51+
import org.springframework.security.web.context.DelegatingSecurityContextRepository;
5152
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
53+
import org.springframework.security.web.context.NullSecurityContextRepository;
5254
import org.springframework.security.web.context.RequestAttributeSecurityContextRepository;
5355
import org.springframework.security.web.context.SecurityContextRepository;
5456
import org.springframework.security.web.savedrequest.NullRequestCache;
@@ -141,6 +143,12 @@ public final class SessionManagementConfigurer<H extends HttpSecurityBuilder<H>>
141143

142144
private Boolean requireExplicitAuthenticationStrategy;
143145

146+
/**
147+
* This should not use RequestAttributeSecurityContextRepository since that is
148+
* stateless and sesison management is about state management.
149+
*/
150+
private SecurityContextRepository sessionManagementSecurityContextRepository = new HttpSessionSecurityContextRepository();
151+
144152
/**
145153
* Creates a new instance
146154
* @see HttpSecurity#sessionManagement()
@@ -356,6 +364,7 @@ public void init(H http) {
356364
if (securityContextRepository == null) {
357365
if (stateless) {
358366
http.setSharedObject(SecurityContextRepository.class, new RequestAttributeSecurityContextRepository());
367+
this.sessionManagementSecurityContextRepository = new NullSecurityContextRepository();
359368
}
360369
else {
361370
HttpSessionSecurityContextRepository httpSecurityRepository = new HttpSessionSecurityContextRepository();
@@ -365,7 +374,10 @@ public void init(H http) {
365374
if (trustResolver != null) {
366375
httpSecurityRepository.setTrustResolver(trustResolver);
367376
}
368-
http.setSharedObject(SecurityContextRepository.class, httpSecurityRepository);
377+
this.sessionManagementSecurityContextRepository = httpSecurityRepository;
378+
DelegatingSecurityContextRepository defaultRepository = new DelegatingSecurityContextRepository(
379+
httpSecurityRepository, new RequestAttributeSecurityContextRepository());
380+
http.setSharedObject(SecurityContextRepository.class, defaultRepository);
369381
}
370382
}
371383
RequestCache requestCache = http.getSharedObject(RequestCache.class);
@@ -420,7 +432,7 @@ private SessionManagementFilter createSessionManagementFilter(H http) {
420432
if (shouldRequireExplicitAuthenticationStrategy()) {
421433
return null;
422434
}
423-
SecurityContextRepository securityContextRepository = http.getSharedObject(SecurityContextRepository.class);
435+
SecurityContextRepository securityContextRepository = this.sessionManagementSecurityContextRepository;
424436
SessionManagementFilter sessionManagementFilter = new SessionManagementFilter(securityContextRepository,
425437
getSessionAuthenticationStrategy(http));
426438
if (this.sessionAuthenticationErrorUrl != null) {

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/SessionManagementConfigurerTests.java

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@
1616

1717
package org.springframework.security.config.annotation.web.configurers;
1818

19+
import java.io.IOException;
20+
21+
import jakarta.servlet.DispatcherType;
22+
import jakarta.servlet.Filter;
23+
import jakarta.servlet.FilterChain;
24+
import jakarta.servlet.ServletException;
25+
import jakarta.servlet.ServletRequest;
26+
import jakarta.servlet.ServletResponse;
1927
import jakarta.servlet.http.HttpServletRequest;
2028
import jakarta.servlet.http.HttpServletResponse;
2129
import jakarta.servlet.http.HttpSession;
@@ -25,8 +33,13 @@
2533
import org.springframework.beans.factory.annotation.Autowired;
2634
import org.springframework.context.annotation.Bean;
2735
import org.springframework.context.annotation.Configuration;
36+
import org.springframework.http.HttpStatus;
37+
import org.springframework.http.ResponseEntity;
38+
import org.springframework.mock.web.MockFilterChain;
39+
import org.springframework.mock.web.MockHttpServletRequest;
2840
import org.springframework.mock.web.MockHttpSession;
2941
import org.springframework.security.authentication.AuthenticationTrustResolver;
42+
import org.springframework.security.config.Customizer;
3043
import org.springframework.security.config.TestDeferredSecurityContext;
3144
import org.springframework.security.config.annotation.ObjectPostProcessor;
3245
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
@@ -55,8 +68,10 @@
5568
import org.springframework.test.web.servlet.MvcResult;
5669
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
5770
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
71+
import org.springframework.web.bind.annotation.GetMapping;
5872
import org.springframework.web.bind.annotation.RequestMapping;
5973
import org.springframework.web.bind.annotation.RestController;
74+
import org.springframework.web.util.WebUtils;
6075

6176
import static org.assertj.core.api.Assertions.assertThat;
6277
import static org.mockito.ArgumentMatchers.any;
@@ -360,6 +375,84 @@ public void loginWhenSessionCreationPolicyStatelessThenSecurityContextIsAvailabl
360375
assertThat(securityContext).isNotNull();
361376
}
362377

378+
/**
379+
* This ensures that if an ErrorDispatch occurs, then the SecurityContextRepository
380+
* defaulted by SessionManagementConfigurer is correct (looks at both Session and
381+
* Request Attributes).
382+
* @throws Exception
383+
*/
384+
@Test
385+
public void gh12070WhenErrorDispatchSecurityContextRepositoryWorks() throws Exception {
386+
Filter errorDispatchFilter = new Filter() {
387+
@Override
388+
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
389+
throws IOException, ServletException {
390+
try {
391+
chain.doFilter(request, response);
392+
}
393+
catch (ServletException ex) {
394+
if (request.getDispatcherType() == DispatcherType.ERROR) {
395+
throw ex;
396+
}
397+
MockHttpServletRequest httpRequest = WebUtils.getNativeRequest(request,
398+
MockHttpServletRequest.class);
399+
httpRequest.setDispatcherType(DispatcherType.ERROR);
400+
// necessary to prevent HttpBasicFilter from invoking again
401+
httpRequest.setAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE, "/error");
402+
httpRequest.setRequestURI("/error");
403+
MockFilterChain mockChain = (MockFilterChain) chain;
404+
mockChain.reset();
405+
mockChain.doFilter(httpRequest, response);
406+
}
407+
}
408+
};
409+
this.spring.addFilter(errorDispatchFilter).register(Gh12070IssueConfig.class).autowire();
410+
411+
// @formatter:off
412+
this.mvc.perform(get("/500").with(httpBasic("user", "password")))
413+
.andExpect(status().isInternalServerError());
414+
// @formatter:on
415+
}
416+
417+
@Configuration
418+
@EnableWebSecurity
419+
static class Gh12070IssueConfig {
420+
421+
@Bean
422+
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
423+
// @formatter:off
424+
http
425+
.authorizeHttpRequests((authorize) -> authorize
426+
.anyRequest().authenticated()
427+
)
428+
.httpBasic(Customizer.withDefaults())
429+
.formLogin(Customizer.withDefaults());
430+
return http.build();
431+
// @formatter:on
432+
}
433+
434+
@Bean
435+
UserDetailsService userDetailsService() {
436+
return new InMemoryUserDetailsManager(PasswordEncodedUser.user());
437+
}
438+
439+
@RestController
440+
static class ErrorController {
441+
442+
@GetMapping("/500")
443+
String error() throws ServletException {
444+
throw new ServletException("Error");
445+
}
446+
447+
@GetMapping("/error")
448+
ResponseEntity<String> errorHandler() {
449+
return new ResponseEntity<>("error", HttpStatus.INTERNAL_SERVER_ERROR);
450+
}
451+
452+
}
453+
454+
}
455+
363456
@Configuration
364457
@EnableWebSecurity
365458
static class SessionManagementRequestCacheConfig {

‎test/src/test/java/org/springframework/security/test/web/support/WebTestUtilsTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.springframework.security.web.DefaultSecurityFilterChain;
3434
import org.springframework.security.web.FilterChainProxy;
3535
import org.springframework.security.web.SecurityFilterChain;
36+
import org.springframework.security.web.context.DelegatingSecurityContextRepository;
3637
import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
3738
import org.springframework.security.web.context.SecurityContextHolderFilter;
3839
import org.springframework.security.web.context.SecurityContextPersistenceFilter;
@@ -119,7 +120,7 @@ public void getSecurityContextRepositoryNoSecurity() {
119120
public void getSecurityContextRepositorySecurityNoCsrf() {
120121
loadConfig(SecurityNoCsrfConfig.class);
121122
assertThat(WebTestUtils.getSecurityContextRepository(this.request))
122-
.isInstanceOf(HttpSessionSecurityContextRepository.class);
123+
.isInstanceOf(DelegatingSecurityContextRepository.class);
123124
}
124125

125126
@Test

0 commit comments

Comments
 (0)