From 2abeff20894d5f579992b8b9009c219b5f8b7dc1 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 18 Feb 2022 15:13:49 -0600 Subject: [PATCH] HttpSessionSecurityContextRepository saves with original response Previously, the HttpSessionSecurityContextRepository unnecessarily required the HttpServletResponse from the HttpReqeustResponseHolder passed into loadContext. This meant code that wanted to save a SecurityContext had to have a reference to the original HttpRequestResponseHolder. Often that implied that the code that saves the SecurityContext must also load the SecurityContext. This change allows any request / response to be used to save the SecurityContext which means any code can save the SecurityContext not just the code that loaded it. This sets up the code to be permit requiring explicit saves. Using the request/response from the HttpRequestResponseHolder is only necessary for implicit saves. Closes gh-10947 --- .../web/context/HttpSessionSecurityContextRepository.java | 7 +++++-- .../security/web/context/SecurityContextRepository.java | 5 ++++- .../HttpSessionSecurityContextRepositoryTests.java | 8 +++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java b/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java index ee9da40a2b..b322487c07 100644 --- a/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java +++ b/web/src/main/java/org/springframework/security/web/context/HttpSessionSecurityContextRepository.java @@ -134,8 +134,11 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo public void saveContext(SecurityContext context, HttpServletRequest request, HttpServletResponse response) { SaveContextOnUpdateOrErrorResponseWrapper responseWrapper = WebUtils.getNativeResponse(response, SaveContextOnUpdateOrErrorResponseWrapper.class); - Assert.state(responseWrapper != null, () -> "Cannot invoke saveContext on response " + response - + ". You must use the HttpRequestResponseHolder.response after invoking loadContext"); + if (responseWrapper == null) { + boolean httpSessionExists = request.getSession(false) != null; + SecurityContext initialContext = SecurityContextHolder.createEmptyContext(); + responseWrapper = new SaveToSessionResponseWrapper(response, request, httpSessionExists, initialContext); + } responseWrapper.saveContext(context); } diff --git a/web/src/main/java/org/springframework/security/web/context/SecurityContextRepository.java b/web/src/main/java/org/springframework/security/web/context/SecurityContextRepository.java index 1039e60c98..9518481928 100644 --- a/web/src/main/java/org/springframework/security/web/context/SecurityContextRepository.java +++ b/web/src/main/java/org/springframework/security/web/context/SecurityContextRepository.java @@ -48,9 +48,12 @@ public interface SecurityContextRepository { * to return wrapped versions of the request or response (or both), allowing them to * access implementation-specific state for the request. The values obtained from the * holder will be passed on to the filter chain and also to the saveContext - * method when it is finally called. Implementations may wish to return a subclass of + * method when it is finally called to allow implicit saves of the + * SecurityContext. Implementations may wish to return a subclass of * {@link SaveContextOnUpdateOrErrorResponseWrapper} as the response object, which * guarantees that the context is persisted when an error or redirect occurs. + * Implementations may allow passing in the original request response to allow + * explicit saves. * @param requestResponseHolder holder for the current request and response for which * the context should be loaded. * @return The security context which should be used for the current request, never diff --git a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java index e5e05edb56..870372eda7 100644 --- a/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java +++ b/web/src/test/java/org/springframework/security/web/context/HttpSessionSecurityContextRepositoryTests.java @@ -58,7 +58,6 @@ import org.springframework.security.core.userdetails.UserDetails; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; @@ -581,13 +580,16 @@ public class HttpSessionSecurityContextRepositoryTests { } @Test - public void failsWithStandardResponse() { + public void standardResponseWorks() { HttpSessionSecurityContextRepository repo = new HttpSessionSecurityContextRepository(); MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); SecurityContext context = SecurityContextHolder.createEmptyContext(); context.setAuthentication(this.testToken); - assertThatIllegalStateException().isThrownBy(() -> repo.saveContext(context, request, response)); + repo.saveContext(context, request, response); + assertThat(request.getSession(false)).isNotNull(); + assertThat(request.getSession().getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY)) + .isEqualTo(context); } @Test