From a28a932598f6f7d088f924bb87367904f8d1f313 Mon Sep 17 00:00:00 2001 From: Ben Alex Date: Thu, 9 Feb 2006 23:04:29 +0000 Subject: [PATCH] SEC-183: Minimise session creation as a consequence of SEC-168 and SEC-182 changes. --- .../HttpSessionContextIntegrationFilter.java | 116 +++++++++++++----- .../anonymous/AnonymousProcessingFilter.java | 2 +- .../TokenBasedRememberMeServices.java | 3 +- ...pSessionContextIntegrationFilterTests.java | 66 ++++++++-- 4 files changed, 139 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java b/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java index 6afc41ad71..c7e1ea2fb4 100644 --- a/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java +++ b/core/src/main/java/org/acegisecurity/context/HttpSessionContextIntegrationFilter.java @@ -1,4 +1,4 @@ -/* Copyright 2004, 2005 Acegi Technology Pty Limited +/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,8 +34,8 @@ import javax.servlet.http.HttpSession; /** *

- * Populates the {@link SecurityContextHolder} with information obtained - * from the HttpSession. + * Populates the {@link SecurityContextHolder} with information obtained from + * the HttpSession. *

* *

@@ -50,7 +50,7 @@ import javax.servlet.http.HttpSession; *

* If a valid SecurityContext cannot be obtained from the * HttpSession for whatever reason, a fresh - * SecurityContext will be created and used instead. The created + * SecurityContext will be created and used instead. The created * object will be of the instance defined by the {@link #setContext(Class)} * method (which defaults to {@link * org.acegisecurity.context.SecurityContextImpl}. @@ -65,7 +65,11 @@ import javax.servlet.http.HttpSession; * java.lang.Object#equals(java.lang.Object)} to a new instance * of {@link #setContext(Class)}. This avoids needless * HttpSession creation, but automates the storage of changes - * made to the SecurityContextHolder. + * made to the SecurityContextHolder. There is one exception to + * this rule, that is if the {@link #forceEagerSessionCreation} property is + * true, in which case sessions will always be created + * irrespective of normal session-minimisation logic (the default is + * false, as this is resource intensive and not recommended). *

* *

@@ -77,18 +81,21 @@ import javax.servlet.http.HttpSession; * If for whatever reason no HttpSession should ever be * created (eg this filter is only being used with Basic authentication or * similar clients that will never present the same jsessionid - * etc), the {@link #setAllowSessionCreation(boolean)} should be set to + * etc), the {@link #setAllowSessionCreation(boolean)} should be set to * false. Only do this if you really need to conserve server - * memory and ensure all classes using the SecurityContextHolder are - * designed to have no persistence of the SecurityContext between web - * requests. + * memory and ensure all classes using the SecurityContextHolder + * are designed to have no persistence of the SecurityContext + * between web requests. Please note that if {@link + * #forceEagerSessionCreation} is true, the + * allowSessionCreation must also be true (setting + * it to false will cause a startup time error). *

* *

- * This filter MUST be executed BEFORE any authentication processing mechanisms. - * Authentication processing mechanisms (eg BASIC, CAS processing filters etc) - * expect the SecurityContextHolder to contain a valid - * SecurityContext by the time they execute. + * This filter MUST be executed BEFORE any authentication processing + * mechanisms. Authentication processing mechanisms (eg BASIC, CAS processing + * filters etc) expect the SecurityContextHolder to contain a + * valid SecurityContext by the time they execute. *

* * @author Ben Alex @@ -99,40 +106,47 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, Filter { //~ Static fields/initializers ============================================= + // ~ Static fields/initializers + // ============================================= protected static final Log logger = LogFactory.getLog(HttpSessionContextIntegrationFilter.class); private static final String FILTER_APPLIED = "__acegi_session_integration_filter_applied"; public static final String ACEGI_SECURITY_CONTEXT_KEY = "ACEGI_SECURITY_CONTEXT"; //~ Instance fields ======================================================== + // ~ Instance fields + // ======================================================== private Class context = SecurityContextImpl.class; private Object contextObject; /** * Indicates if this filter can create a HttpSession if needed - * (sessions are always created sparingly, but setting this value to false - * will prohibit sessions from ever being created). Defaults to true. + * (sessions are always created sparingly, but setting this value to + * false will prohibit sessions from ever being created). + * Defaults to true. Do not set to false if you + * are have set {@link #forceEagerSessionCreation} to true, + * as the properties would be in conflict. */ private boolean allowSessionCreation = true; + /** + * Indicates if this filter is required to create a + * HttpSession for every request before proceeding through + * the filter chain, even if the HttpSession would not + * ordinarily have been created. By default this is false, + * which is entirely appropriate for most circumstances as you do not want + * a HttpSession created unless the filter actually needs + * one. It is envisaged the main situation in which this property would be + * set to true is if using other filters that depend on a + * HttpSession already existing, such as those which need to + * obtain a session ID. This is only required in specialised cases, so + * leave it set to false unless you have an actual + * requirement and are conscious of the session creation overhead. + */ + private boolean forceEagerSessionCreation = false; + //~ Methods ================================================================ - public void setAllowSessionCreation(boolean allowSessionCreation) { - this.allowSessionCreation = allowSessionCreation; - } - - public boolean isAllowSessionCreation() { - return allowSessionCreation; - } - - public void setContext(Class secureContext) { - this.context = secureContext; - } - - public Class getContext() { - return context; - } - public void afterPropertiesSet() throws Exception { if ((this.context == null) || (!SecurityContext.class.isAssignableFrom(this.context))) { @@ -141,6 +155,12 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, + this.context + ")"); } + if ((forceEagerSessionCreation == true) + && (allowSessionCreation == false)) { + throw new IllegalArgumentException( + "If using forceEagerSessionCreation, you must set allowSessionCreation to also be true"); + } + this.contextObject = generateNewContext(); } @@ -163,7 +183,7 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, boolean httpSessionExistedAtStartOfRequest = false; try { - httpSession = ((HttpServletRequest) request).getSession(false); + httpSession = ((HttpServletRequest) request).getSession(forceEagerSessionCreation); } catch (IllegalStateException ignored) {} if (httpSession != null) { @@ -265,8 +285,10 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, } } - // If HttpSession exists, store current SecurityContextHolder contents - // but only if SecurityContext has actually changed (see JIRA SEC-37) + // If HttpSession exists, store current SecurityContextHolder + // contents + // but only if SecurityContext has actually changed (see JIRA + // SEC-37) if ((httpSession != null) && (SecurityContextHolder.getContext().hashCode() != contextWhenChainProceeded)) { httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY, @@ -299,6 +321,10 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, } } + public Class getContext() { + return context; + } + /** * Does nothing. We use IoC container lifecycle services instead. * @@ -307,4 +333,26 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean, * @throws ServletException ignored */ public void init(FilterConfig filterConfig) throws ServletException {} + + public boolean isAllowSessionCreation() { + return allowSessionCreation; + } + + // ~ Methods + // ================================================================ + public boolean isForceEagerSessionCreation() { + return forceEagerSessionCreation; + } + + public void setAllowSessionCreation(boolean allowSessionCreation) { + this.allowSessionCreation = allowSessionCreation; + } + + public void setContext(Class secureContext) { + this.context = secureContext; + } + + public void setForceEagerSessionCreation(boolean forceEagerSessionCreation) { + this.forceEagerSessionCreation = forceEagerSessionCreation; + } } diff --git a/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousProcessingFilter.java b/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousProcessingFilter.java index 57a60acf23..312c70a27e 100644 --- a/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousProcessingFilter.java +++ b/core/src/main/java/org/acegisecurity/providers/anonymous/AnonymousProcessingFilter.java @@ -97,7 +97,7 @@ public class AnonymousProcessingFilter implements Filter, InitializingBean { AnonymousAuthenticationToken auth = new AnonymousAuthenticationToken(key, userAttribute.getPassword(), userAttribute.getAuthorities()); auth.setDetails(new WebAuthenticationDetails( - (HttpServletRequest) request)); + (HttpServletRequest) request, false)); return auth; } diff --git a/core/src/main/java/org/acegisecurity/ui/rememberme/TokenBasedRememberMeServices.java b/core/src/main/java/org/acegisecurity/ui/rememberme/TokenBasedRememberMeServices.java index 7767dde993..ffd5dd90d4 100644 --- a/core/src/main/java/org/acegisecurity/ui/rememberme/TokenBasedRememberMeServices.java +++ b/core/src/main/java/org/acegisecurity/ui/rememberme/TokenBasedRememberMeServices.java @@ -232,7 +232,8 @@ public class TokenBasedRememberMeServices implements RememberMeServices, RememberMeAuthenticationToken auth = new RememberMeAuthenticationToken(this.key, userDetails, userDetails.getAuthorities()); - auth.setDetails(new WebAuthenticationDetails(request)); + auth.setDetails(new WebAuthenticationDetails(request, + false)); return auth; } else { diff --git a/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java b/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java index df4b5377cd..5718798684 100644 --- a/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java +++ b/core/src/test/java/org/acegisecurity/context/HttpSessionContextIntegrationFilterTests.java @@ -1,4 +1,4 @@ -/* Copyright 2004, 2005 Acegi Technology Pty Limited +/* Copyright 2004, 2005, 2006 Acegi Technology Pty Limited * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,10 +56,36 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { //~ Methods ================================================================ + private void executeFilterInContainerSimulator(FilterConfig filterConfig, + Filter filter, ServletRequest request, ServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + filter.init(filterConfig); + filter.doFilter(request, response, filterChain); + filter.destroy(); + } + public static void main(String[] args) { junit.textui.TestRunner.run(HttpSessionContextIntegrationFilterTests.class); } + public void testDetectsIncompatibleSessionProperties() + throws Exception { + HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); + + try { + filter.setAllowSessionCreation(false); + filter.setForceEagerSessionCreation(true); + filter.afterPropertiesSet(); + fail("Shown have thrown IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + assertTrue(true); + } + + filter.setAllowSessionCreation(true); + filter.afterPropertiesSet(); + assertTrue(true); + } + public void testDetectsMissingOrInvalidContext() throws Exception { HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); @@ -95,7 +121,8 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { // Build a mock request MockHttpServletRequest request = new MockHttpServletRequest(); - request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, + request.getSession() + .setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, sc); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -142,7 +169,8 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { // Build a mock request MockHttpServletRequest request = new MockHttpServletRequest(); - request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, + request.getSession() + .setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, sc); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -194,6 +222,27 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { ((SecurityContext) context).getAuthentication()); } + public void testHttpSessionEagerlyCreatedWhenDirected() + throws Exception { + // Build a mock request + MockHttpServletRequest request = new MockHttpServletRequest(null, null); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = new MockFilterChain(null, null, null); + + // Prepare filter + HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter(); + filter.setContext(SecurityContextImpl.class); + filter.setForceEagerSessionCreation(true); // non-default + filter.afterPropertiesSet(); + + // Execute filter + executeFilterInContainerSimulator(new MockFilterConfig(), filter, + request, response, chain); + + // Check the session is not null + assertNotNull(request.getSession(false)); + } + public void testHttpSessionNotCreatedUnlessContextHolderChanges() throws Exception { // Build a mock request @@ -224,7 +273,8 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { // Build a mock request MockHttpServletRequest request = new MockHttpServletRequest(); - request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, + request.getSession() + .setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY, "NOT_A_CONTEXT_OBJECT"); MockHttpServletResponse response = new MockHttpServletResponse(); @@ -246,14 +296,6 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase { ((SecurityContext) context).getAuthentication()); } - private void executeFilterInContainerSimulator(FilterConfig filterConfig, - Filter filter, ServletRequest request, ServletResponse response, - FilterChain filterChain) throws ServletException, IOException { - filter.init(filterConfig); - filter.doFilter(request, response, filterChain); - filter.destroy(); - } - //~ Inner Classes ========================================================== private class MockFilterChain extends TestCase implements FilterChain {