SecurityContext when a sendError(), sendRedirect,
+ * Base class for response wrappers which encapsulate the logic for storing a security context and which store the
+ * SecurityContext when a sendError(), sendRedirect,
* getOutputStream().close(), getOutputStream().flush(), getWriter().close(), or
- * getWriter().flush() happens. See issue SEC-398 and SEC-2005.
+ * getWriter().flush() happens on the same thread that this
+ * {@link SaveContextOnUpdateOrErrorResponseWrapper} was created. See issue SEC-398 and SEC-2005.
* * Sub-classes should implement the {@link #saveContext(SecurityContext context)} method. *
@@ -39,6 +42,9 @@ import org.springframework.security.core.context.SecurityContextHolder;
* @since 3.0
*/
public abstract class SaveContextOnUpdateOrErrorResponseWrapper extends HttpServletResponseWrapper {
+ private final Log logger = LogFactory.getLog(getClass());
+
+ private final Thread SUPPORTED_THREAD = Thread.currentThread();
private boolean contextSaved = false;
/* See SEC-1052 */
@@ -120,11 +126,19 @@ public abstract class SaveContextOnUpdateOrErrorResponseWrapper extends HttpServ
}
/**
- * Calls saveContext() with the current contents of the SecurityContextHolder.
+ * Calls saveContext() with the current contents of the SecurityContextHolder as long as
+ * {@link #doSaveContext()} is invoked on the same Thread that {@link SaveContextOnUpdateOrErrorResponseWrapper} was
+ * created on. This prevents issues when dealing with Async Web Requests where the {@link SecurityContext} is not
+ * present on the Thread that processes the response.
*/
private void doSaveContext() {
- saveContext(SecurityContextHolder.getContext());
- contextSaved = true;
+ Thread currentThread = Thread.currentThread();
+ if(SUPPORTED_THREAD == currentThread) {
+ saveContext(SecurityContextHolder.getContext());
+ contextSaved = true;
+ } else if(logger.isDebugEnabled()){
+ logger.debug("Skip saving SecurityContext since processing the HttpServletResponse on a different Thread than the original HttpServletRequest");
+ }
}
@Override
diff --git a/web/src/test/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapperTests.java b/web/src/test/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapperTests.java
new file mode 100644
index 0000000000..2f39db55ac
--- /dev/null
+++ b/web/src/test/java/org/springframework/security/web/context/SaveContextOnUpdateOrErrorResponseWrapperTests.java
@@ -0,0 +1,261 @@
+/*
+ * Copyright 2002-2012 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. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package org.springframework.security.web.context;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpServletResponse;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.context.SecurityContextHolder;
+
+/**
+ *
+ * @author Rob Winch
+ *
+ */
+
+@RunWith(MockitoJUnitRunner.class)
+public class SaveContextOnUpdateOrErrorResponseWrapperTests {
+ @Mock
+ private SecurityContext securityContext;
+
+ private MockHttpServletResponse response;
+ private SaveContextOnUpdateOrErrorResponseWrapperStub wrappedResponse;
+
+ @Before
+ public void setUp() {
+ response = new MockHttpServletResponse();
+ wrappedResponse = new SaveContextOnUpdateOrErrorResponseWrapperStub(response, true);
+ SecurityContextHolder.setContext(securityContext);
+ }
+
+ @After
+ public void clearContext() {
+ SecurityContextHolder.clearContext();
+ }
+
+ @Test
+ public void sendErrorSavesSecurityContext() throws Exception {
+ int error = HttpServletResponse.SC_FORBIDDEN;
+ wrappedResponse.sendError(error);
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ assertThat(response.getStatus()).isEqualTo(error);
+ }
+
+ @Test
+ public void sendErrorSkipsSaveSecurityContextOnNewThread() throws Exception {
+ final int error = HttpServletResponse.SC_FORBIDDEN;
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.sendError(error);
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ assertThat(response.getStatus()).isEqualTo(error);
+ }
+
+ @Test
+ public void sendErrorWithMessageSavesSecurityContext() throws Exception {
+ int error = HttpServletResponse.SC_FORBIDDEN;
+ String message = "Forbidden";
+ wrappedResponse.sendError(error, message);
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ assertThat(response.getStatus()).isEqualTo(error);
+ assertThat(response.getErrorMessage()).isEqualTo(message);
+ }
+
+ @Test
+ public void sendErrorWithMessageSkipsSaveSecurityContextOnNewThread() throws Exception {
+ final int error = HttpServletResponse.SC_FORBIDDEN;
+ final String message = "Forbidden";
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.sendError(error, message);
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ assertThat(response.getStatus()).isEqualTo(error);
+ assertThat(response.getErrorMessage()).isEqualTo(message);
+ }
+
+ @Test
+ public void sendRedirectSavesSecurityContext() throws Exception {
+ String url = "/location";
+ wrappedResponse.sendRedirect(url);
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ assertThat(response.getRedirectedUrl()).isEqualTo(url);
+ }
+
+ @Test
+ public void sendRedirectSkipsSaveSecurityContextOnNewThread() throws Exception {
+ final String url = "/location";
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.sendRedirect(url);
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ assertThat(response.getRedirectedUrl()).isEqualTo(url);
+ }
+
+ @Test
+ public void outputFlushSavesSecurityContext() throws Exception {
+ wrappedResponse.getOutputStream().flush();
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ }
+
+ @Test
+ public void outputFlushSkipsSaveSecurityContextOnNewThread() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.getOutputStream().flush();
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ }
+
+ @Test
+ public void outputCloseSavesSecurityContext() throws Exception {
+ wrappedResponse.getOutputStream().close();
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ }
+
+ @Test
+ public void outputCloseSkipsSaveSecurityContextOnNewThread() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.getOutputStream().close();
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ }
+
+ @Test
+ public void writerFlushSavesSecurityContext() throws Exception {
+ wrappedResponse.getWriter().flush();
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ }
+
+ @Test
+ public void writerFlushSkipsSaveSecurityContextOnNewThread() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.getWriter().flush();
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ }
+
+ @Test
+ public void writerCloseSavesSecurityContext() throws Exception {
+ wrappedResponse.getWriter().close();
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ }
+
+ @Test
+ public void writerCloseSkipsSaveSecurityContextOnNewThread() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.getWriter().close();
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ }
+
+ @Test
+ public void flushBufferSavesSecurityContext() throws Exception {
+ wrappedResponse.flushBuffer();
+ assertThat(wrappedResponse.securityContext).isEqualTo(securityContext);
+ }
+
+ @Test
+ public void flushBufferSkipsSaveSecurityContextOnNewThread() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ try {
+ wrappedResponse.flushBuffer();
+ } catch(IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ t.start();
+ t.join();
+ assertThat(wrappedResponse.securityContext).isNull();
+ }
+
+ private static class SaveContextOnUpdateOrErrorResponseWrapperStub extends SaveContextOnUpdateOrErrorResponseWrapper {
+ private SecurityContext securityContext;
+
+ public SaveContextOnUpdateOrErrorResponseWrapperStub(HttpServletResponse response, boolean disableUrlRewriting) {
+ super(response, disableUrlRewriting);
+ }
+
+ @Override
+ protected void saveContext(SecurityContext context) {
+ securityContext = context;
+ }
+ }
+}