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
This commit is contained in:
Rob Winch 2022-02-18 15:13:49 -06:00
parent bc2bb8cb96
commit 65ec2659c4
3 changed files with 14 additions and 6 deletions

View File

@ -134,8 +134,11 @@ public class HttpSessionSecurityContextRepository implements SecurityContextRepo
public void saveContext(SecurityContext context, HttpServletRequest request, HttpServletResponse response) { public void saveContext(SecurityContext context, HttpServletRequest request, HttpServletResponse response) {
SaveContextOnUpdateOrErrorResponseWrapper responseWrapper = WebUtils.getNativeResponse(response, SaveContextOnUpdateOrErrorResponseWrapper responseWrapper = WebUtils.getNativeResponse(response,
SaveContextOnUpdateOrErrorResponseWrapper.class); SaveContextOnUpdateOrErrorResponseWrapper.class);
Assert.state(responseWrapper != null, () -> "Cannot invoke saveContext on response " + response if (responseWrapper == null) {
+ ". You must use the HttpRequestResponseHolder.response after invoking loadContext"); boolean httpSessionExists = request.getSession(false) != null;
SecurityContext initialContext = SecurityContextHolder.createEmptyContext();
responseWrapper = new SaveToSessionResponseWrapper(response, request, httpSessionExists, initialContext);
}
responseWrapper.saveContext(context); responseWrapper.saveContext(context);
} }

View File

@ -48,9 +48,12 @@ public interface SecurityContextRepository {
* to return wrapped versions of the request or response (or both), allowing them to * 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 * 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 <tt>saveContext</tt> * holder will be passed on to the filter chain and also to the <tt>saveContext</tt>
* 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
* <tt>SecurityContext</tt>. Implementations may wish to return a subclass of
* {@link SaveContextOnUpdateOrErrorResponseWrapper} as the response object, which * {@link SaveContextOnUpdateOrErrorResponseWrapper} as the response object, which
* guarantees that the context is persisted when an error or redirect occurs. * 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 * @param requestResponseHolder holder for the current request and response for which
* the context should be loaded. * the context should be loaded.
* @return The security context which should be used for the current request, never * @return The security context which should be used for the current request, never

View File

@ -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.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; 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.ArgumentMatchers.anyBoolean;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
@ -581,13 +580,16 @@ public class HttpSessionSecurityContextRepositoryTests {
} }
@Test @Test
public void failsWithStandardResponse() { public void standardResponseWorks() {
HttpSessionSecurityContextRepository repo = new HttpSessionSecurityContextRepository(); HttpSessionSecurityContextRepository repo = new HttpSessionSecurityContextRepository();
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
SecurityContext context = SecurityContextHolder.createEmptyContext(); SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(this.testToken); 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 @Test