diff --git a/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java b/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java index 55620deeb8..c08d19c6ef 100644 --- a/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java +++ b/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java @@ -212,65 +212,74 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, httpSession = null; // Proceed with chain - chain.doFilter(request, response); - - // Store context back to HttpSession try { - httpSession = ((HttpServletRequest) request).getSession(false); - } catch (IllegalStateException ignored) {} + chain.doFilter(request, response); + } catch (IOException ioe) { + throw ioe; + } catch (ServletException se) { + throw se; + } finally { + // do clean up, even if there was an exception + // Store context back to HttpSession + try { + httpSession = ((HttpServletRequest) request).getSession(false); + } catch (IllegalStateException ignored) {} + + if ((httpSession == null) && httpSessionExistedAtStartOfRequest) { + if (logger.isDebugEnabled()) { + logger.debug( + "HttpSession is now null, but was not null at start of request; session was invalidated, so do not create a new session"); + } + } + + // Generate a HttpSession only if we need to + if ((httpSession == null) + && !httpSessionExistedAtStartOfRequest) { + if (!allowSessionCreation) { + if (logger.isDebugEnabled()) { + logger.debug( + "The HttpSession is currently null, and the HttpSessionContextIntegrationFilter is prohibited from creating a HttpSession (because the allowSessionCreation property is false) - SecurityContext thus not stored for next request"); + } + } else if (!contextObject.equals( + SecurityContextHolder.getContext())) { + if (logger.isDebugEnabled()) { + logger.debug( + "HttpSession being created as SecurityContextHolder contents are non-default"); + } + + try { + httpSession = ((HttpServletRequest) request) + .getSession(true); + } catch (IllegalStateException ignored) {} + } else { + if (logger.isDebugEnabled()) { + logger.debug( + "HttpSession is null, but SecurityContextHolder has not changed from default: ' " + + SecurityContextHolder.getContext() + + "'; not creating HttpSession or storing SecurityContextHolder contents"); + } + } + } + + // If HttpSession exists, store current SecurityContextHolder contents + if (httpSession != null) { + httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY, + SecurityContextHolder.getContext()); + + if (logger.isDebugEnabled()) { + logger.debug("SecurityContext stored to HttpSession: '" + + SecurityContextHolder.getContext() + "'"); + } + } + + // Remove SecurityContextHolder contents + SecurityContextHolder.setContext(generateNewContext()); - if ((httpSession == null) && httpSessionExistedAtStartOfRequest) { if (logger.isDebugEnabled()) { logger.debug( - "HttpSession is now null, but was not null at start of request; session was invalidated, so do not create a new session"); + "SecurityContextHolder set to new context, as request processing completed"); } } - - // Generate a HttpSession only if we need to - if ((httpSession == null) && !httpSessionExistedAtStartOfRequest) { - if (!allowSessionCreation) { - if (logger.isDebugEnabled()) { - logger.debug( - "The HttpSession is currently null, and the HttpSessionContextIntegrationFilter is prohibited from creating a HttpSession (because the allowSessionCreation property is false) - SecurityContext thus not stored for next request"); - } - } else if (!contextObject.equals( - SecurityContextHolder.getContext())) { - if (logger.isDebugEnabled()) { - logger.debug( - "HttpSession being created as SecurityContextHolder contents are non-default"); - } - - try { - httpSession = ((HttpServletRequest) request).getSession(true); - } catch (IllegalStateException ignored) {} - } else { - if (logger.isDebugEnabled()) { - logger.debug( - "HttpSession is null, but SecurityContextHolder has not changed from default: ' " - + SecurityContextHolder.getContext() - + "'; not creating HttpSession or storing SecurityContextHolder contents"); - } - } - } - - // If HttpSession exists, store current SecurityContextHolder contents - if (httpSession != null) { - httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY, - SecurityContextHolder.getContext()); - - if (logger.isDebugEnabled()) { - logger.debug("SecurityContext stored to HttpSession: '" - + SecurityContextHolder.getContext() + "'"); - } - } - - // Remove SecurityContextHolder contents - SecurityContextHolder.setContext(generateNewContext()); - - if (logger.isDebugEnabled()) { - logger.debug( - "SecurityContextHolder set to new context, as request processing completed"); - } } } diff --git a/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java b/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java index 816fec9331..537350be27 100644 --- a/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java +++ b/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java @@ -83,6 +83,46 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { } } + public void testExceptionWithinFilterChainStillClearsSecurityContextHolder() + throws Exception { + // Build an Authentication object we simulate came from HttpSession + PrincipalAcegiUserToken sessionPrincipal = new PrincipalAcegiUserToken("key", + "someone", "password", + new GrantedAuthority[] {new GrantedAuthorityImpl("SOME_ROLE")}); + + // Build a Context to store in HttpSession (simulating prior request) + SecurityContext sc = new SecurityContextImpl(); + sc.setAuthentication(sessionPrincipal); + + // Build a mock request + MockHttpServletRequest request = new MockHttpServletRequest(); + request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, + sc); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = new MockFilterChain(sessionPrincipal, null, + new IOException()); + + // Prepare filter + HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); + filter.setContext(SecurityContextImpl.class); + filter.afterPropertiesSet(); + + // Execute filter + try { + executeFilterInContainerSimulator(new MockFilterConfig(), filter, + request, response, chain); + fail( + "We should have received the IOException thrown inside the filter chain here"); + } catch (IOException ioe) { + assertTrue(true); + } + + // Check the SecurityContextHolder is null, even though an exception was thrown during chain + assertEquals(new SecurityContextImpl(), + SecurityContextHolder.getContext()); + } + public void testExistingContextContentsCopiedIntoContextHolderFromSessionAndChangesToContextCopiedBackToSession() throws Exception { // Build an Authentication object we simulate came from HttpSession @@ -106,7 +146,7 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { MockHttpServletResponse response = new MockHttpServletResponse(); FilterChain chain = new MockFilterChain(sessionPrincipal, - updatedPrincipal); + updatedPrincipal, null); // Prepare filter HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); @@ -134,7 +174,7 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { // Build a mock request MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain chain = new MockFilterChain(null, updatedPrincipal); + FilterChain chain = new MockFilterChain(null, updatedPrincipal, null); // Prepare filter HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); @@ -157,7 +197,7 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { // Build a mock request MockHttpServletRequest request = new MockHttpServletRequest(null, null); MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain chain = new MockFilterChain(null, null); + FilterChain chain = new MockFilterChain(null, null, null); // Prepare filter HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); @@ -185,7 +225,7 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { "NOT_A_CONTEXT_OBJECT"); MockHttpServletResponse response = new MockHttpServletResponse(); - FilterChain chain = new MockFilterChain(null, updatedPrincipal); + FilterChain chain = new MockFilterChain(null, updatedPrincipal, null); // Prepare filter HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); @@ -216,11 +256,13 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { private class MockFilterChain extends TestCase implements FilterChain { private Authentication changeContextHolder; private Authentication expectedOnContextHolder; + private IOException toThrowDuringChain; public MockFilterChain(Authentication expectedOnContextHolder, - Authentication changeContextHolder) { + Authentication changeContextHolder, IOException toThrowDuringChain) { this.expectedOnContextHolder = expectedOnContextHolder; this.changeContextHolder = changeContextHolder; + this.toThrowDuringChain = toThrowDuringChain; } private MockFilterChain() {} @@ -237,6 +279,10 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { sc.setAuthentication(changeContextHolder); SecurityContextHolder.setContext(sc); } + + if (toThrowDuringChain != null) { + throw toThrowDuringChain; + } } } } diff --git a/doc/xdocs/changes.xml b/doc/xdocs/changes.xml index 1f97b3f080..9f2e23f10d 100644 --- a/doc/xdocs/changes.xml +++ b/doc/xdocs/changes.xml @@ -42,6 +42,7 @@ Remove getters and setters from JdbcDaoImpl so IoC container cannot modify MappingSqlQuerys Refactor DAO authentication failure events under a consistent abstract superclass JBoss container adapter to use getName() instead to toString() (see http://opensource.atlassian.com/projects/spring/browse/SEC-22) + HttpSessionContextIntegrationFilter elegantly handles IOExceptions and ServletExceptions within filter chain (see http://opensource.atlassian.com/projects/spring/browse/SEC-20) Correct location of AuthenticationSimpleHttpInvokerRequestExecutor in clientContext.xml