Browse Source

SEC-183: Minimise session creation as a consequence of SEC-168 and SEC-182 changes.

Ben Alex 20 years ago
parent
commit
a28a932598

+ 82 - 34
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");
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * you may not use this file except in compliance with the License.
@@ -34,8 +34,8 @@ import javax.servlet.http.HttpSession;
 
 
 /**
 /**
  * <p>
  * <p>
- * Populates the {@link SecurityContextHolder}</code> with information obtained
- * from the <code>HttpSession</code>.
+ * Populates the {@link SecurityContextHolder} with information obtained from
+ * the <code>HttpSession</code>.
  * </p>
  * </p>
  * 
  * 
  * <p>
  * <p>
@@ -50,7 +50,7 @@ import javax.servlet.http.HttpSession;
  * <p>
  * <p>
  * If a valid <code>SecurityContext</code> cannot be obtained from the
  * If a valid <code>SecurityContext</code> cannot be obtained from the
  * <code>HttpSession</code> for whatever reason, a fresh
  * <code>HttpSession</code> for whatever reason, a fresh
- * <code>SecurityContext</code> will be created and used instead.  The created
+ * <code>SecurityContext</code> will be created and used instead. The created
  * object will be of the instance defined by the {@link #setContext(Class)}
  * object will be of the instance defined by the {@link #setContext(Class)}
  * method (which defaults to {@link
  * method (which defaults to {@link
  * org.acegisecurity.context.SecurityContextImpl}.
  * org.acegisecurity.context.SecurityContextImpl}.
@@ -65,7 +65,11 @@ import javax.servlet.http.HttpSession;
  * java.lang.Object#equals(java.lang.Object)} to a <code>new</code> instance
  * java.lang.Object#equals(java.lang.Object)} to a <code>new</code> instance
  * of {@link #setContext(Class)}. This avoids needless
  * of {@link #setContext(Class)}. This avoids needless
  * <code>HttpSession</code> creation, but automates the storage of changes
  * <code>HttpSession</code> creation, but automates the storage of changes
- * made to the <code>SecurityContextHolder</code>.
+ * made to the <code>SecurityContextHolder</code>. There is one exception to
+ * this rule, that is if the {@link #forceEagerSessionCreation} property is
+ * <code>true</code>, in which case sessions will always be created
+ * irrespective of normal session-minimisation logic (the default is
+ * <code>false</code>, as this is resource intensive and not recommended).
  * </p>
  * </p>
  * 
  * 
  * <p>
  * <p>
@@ -77,18 +81,21 @@ import javax.servlet.http.HttpSession;
  * If for whatever reason no <code>HttpSession</code> should <b>ever</b> be
  * If for whatever reason no <code>HttpSession</code> should <b>ever</b> be
  * created (eg this filter is only being used with Basic authentication or
  * created (eg this filter is only being used with Basic authentication or
  * similar clients that will never present the same <code>jsessionid</code>
  * similar clients that will never present the same <code>jsessionid</code>
- * etc), the  {@link #setAllowSessionCreation(boolean)} should be set to
+ * etc), the {@link #setAllowSessionCreation(boolean)} should be set to
  * <code>false</code>. Only do this if you really need to conserve server
  * <code>false</code>. Only do this if you really need to conserve server
- * memory and ensure all classes using the <code>SecurityContextHolder</code> are
- * designed to have no persistence of the <code>SecurityContext</code> between web
- * requests.
+ * memory and ensure all classes using the <code>SecurityContextHolder</code>
+ * are designed to have no persistence of the <code>SecurityContext</code>
+ * between web requests. Please note that if {@link
+ * #forceEagerSessionCreation} is <code>true</code>, the
+ * <code>allowSessionCreation</code> must also be <code>true</code> (setting
+ * it to <code>false</code> will cause a startup time error).
  * </p>
  * </p>
  * 
  * 
  * <p>
  * <p>
- * This filter MUST be executed BEFORE any authentication processing mechanisms.
- * Authentication processing mechanisms (eg BASIC, CAS processing filters etc)
- * expect the <code>SecurityContextHolder</code> to contain a valid
- * <code>SecurityContext</code> 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 <code>SecurityContextHolder</code> to contain a
+ * valid <code>SecurityContext</code> by the time they execute.
  * </p>
  * </p>
  *
  *
  * @author Ben Alex
  * @author Ben Alex
@@ -99,39 +106,46 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
     Filter {
     Filter {
     //~ Static fields/initializers =============================================
     //~ Static fields/initializers =============================================
 
 
+    // ~ Static fields/initializers
+    // =============================================
     protected static final Log logger = LogFactory.getLog(HttpSessionContextIntegrationFilter.class);
     protected static final Log logger = LogFactory.getLog(HttpSessionContextIntegrationFilter.class);
     private static final String FILTER_APPLIED = "__acegi_session_integration_filter_applied";
     private static final String FILTER_APPLIED = "__acegi_session_integration_filter_applied";
     public static final String ACEGI_SECURITY_CONTEXT_KEY = "ACEGI_SECURITY_CONTEXT";
     public static final String ACEGI_SECURITY_CONTEXT_KEY = "ACEGI_SECURITY_CONTEXT";
 
 
     //~ Instance fields ========================================================
     //~ Instance fields ========================================================
 
 
+    // ~ Instance fields
+    // ========================================================
     private Class context = SecurityContextImpl.class;
     private Class context = SecurityContextImpl.class;
     private Object contextObject;
     private Object contextObject;
 
 
     /**
     /**
      * Indicates if this filter can create a <code>HttpSession</code> if needed
      * Indicates if this filter can create a <code>HttpSession</code> 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
+     * <code>false</code> will prohibit sessions from ever being created).
+     * Defaults to <code>true</code>. Do not set to <code>false</code> if you
+     * are have set {@link #forceEagerSessionCreation} to <code>true</code>,
+     * as the properties would be in conflict.
      */
      */
     private boolean allowSessionCreation = true;
     private boolean allowSessionCreation = true;
 
 
-    //~ Methods ================================================================
-
-    public void setAllowSessionCreation(boolean allowSessionCreation) {
-        this.allowSessionCreation = allowSessionCreation;
-    }
-
-    public boolean isAllowSessionCreation() {
-        return allowSessionCreation;
-    }
-
-    public void setContext(Class secureContext) {
-        this.context = secureContext;
-    }
+    /**
+     * Indicates if this filter is required to create a
+     * <code>HttpSession</code> for every request before proceeding through
+     * the filter chain, even if the <code>HttpSession</code> would not
+     * ordinarily have been created. By default this is <code>false</code>,
+     * which is entirely appropriate for most circumstances as you do not want
+     * a <code>HttpSession</code> created unless the filter actually needs
+     * one. It is envisaged the main situation in which this property would be
+     * set to <code>true</code> is if using other filters that depend on a
+     * <code>HttpSession</code> already existing, such as those which need to
+     * obtain a session ID. This is only required in specialised cases, so
+     * leave it set to <code>false</code> unless you have an actual
+     * requirement and are conscious of the session creation overhead.
+     */
+    private boolean forceEagerSessionCreation = false;
 
 
-    public Class getContext() {
-        return context;
-    }
+    //~ Methods ================================================================
 
 
     public void afterPropertiesSet() throws Exception {
     public void afterPropertiesSet() throws Exception {
         if ((this.context == null)
         if ((this.context == null)
@@ -141,6 +155,12 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
                 + this.context + ")");
                 + this.context + ")");
         }
         }
 
 
+        if ((forceEagerSessionCreation == true)
+            && (allowSessionCreation == false)) {
+            throw new IllegalArgumentException(
+                "If using forceEagerSessionCreation, you must set allowSessionCreation to also be true");
+        }
+
         this.contextObject = generateNewContext();
         this.contextObject = generateNewContext();
     }
     }
 
 
@@ -163,7 +183,7 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
             boolean httpSessionExistedAtStartOfRequest = false;
             boolean httpSessionExistedAtStartOfRequest = false;
 
 
             try {
             try {
-                httpSession = ((HttpServletRequest) request).getSession(false);
+                httpSession = ((HttpServletRequest) request).getSession(forceEagerSessionCreation);
             } catch (IllegalStateException ignored) {}
             } catch (IllegalStateException ignored) {}
 
 
             if (httpSession != null) {
             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)
                 if ((httpSession != null)
                     && (SecurityContextHolder.getContext().hashCode() != contextWhenChainProceeded)) {
                     && (SecurityContextHolder.getContext().hashCode() != contextWhenChainProceeded)) {
                     httpSession.setAttribute(ACEGI_SECURITY_CONTEXT_KEY,
                     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.
      * Does nothing. We use IoC container lifecycle services instead.
      *
      *
@@ -307,4 +333,26 @@ public class HttpSessionContextIntegrationFilter implements InitializingBean,
      * @throws ServletException ignored
      * @throws ServletException ignored
      */
      */
     public void init(FilterConfig filterConfig) throws ServletException {}
     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;
+    }
 }
 }

+ 1 - 1
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,
         AnonymousAuthenticationToken auth = new AnonymousAuthenticationToken(key,
                 userAttribute.getPassword(), userAttribute.getAuthorities());
                 userAttribute.getPassword(), userAttribute.getAuthorities());
         auth.setDetails(new WebAuthenticationDetails(
         auth.setDetails(new WebAuthenticationDetails(
-                (HttpServletRequest) request));
+                (HttpServletRequest) request, false));
 
 
         return auth;
         return auth;
     }
     }

+ 2 - 1
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,
                         RememberMeAuthenticationToken auth = new RememberMeAuthenticationToken(this.key,
                                 userDetails, userDetails.getAuthorities());
                                 userDetails, userDetails.getAuthorities());
-                        auth.setDetails(new WebAuthenticationDetails(request));
+                        auth.setDetails(new WebAuthenticationDetails(request,
+                                false));
 
 
                         return auth;
                         return auth;
                     } else {
                     } else {

+ 54 - 12
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");
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * you may not use this file except in compliance with the License.
@@ -56,10 +56,36 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase {
 
 
     //~ Methods ================================================================
     //~ 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) {
     public static void main(String[] args) {
         junit.textui.TestRunner.run(HttpSessionContextIntegrationFilterTests.class);
         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 {
     public void testDetectsMissingOrInvalidContext() throws Exception {
         HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter();
         HttpSessionContextIntegrationFilter filter = new HttpSessionContextIntegrationFilter();
 
 
@@ -95,7 +121,8 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase {
 
 
         // Build a mock request
         // Build a mock request
         MockHttpServletRequest request = new MockHttpServletRequest();
         MockHttpServletRequest request = new MockHttpServletRequest();
-        request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY,
+        request.getSession()
+               .setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY,
             sc);
             sc);
 
 
         MockHttpServletResponse response = new MockHttpServletResponse();
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -142,7 +169,8 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase {
 
 
         // Build a mock request
         // Build a mock request
         MockHttpServletRequest request = new MockHttpServletRequest();
         MockHttpServletRequest request = new MockHttpServletRequest();
-        request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY,
+        request.getSession()
+               .setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY,
             sc);
             sc);
 
 
         MockHttpServletResponse response = new MockHttpServletResponse();
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -194,6 +222,27 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase {
             ((SecurityContext) context).getAuthentication());
             ((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()
     public void testHttpSessionNotCreatedUnlessContextHolderChanges()
         throws Exception {
         throws Exception {
         // Build a mock request
         // Build a mock request
@@ -224,7 +273,8 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase {
 
 
         // Build a mock request
         // Build a mock request
         MockHttpServletRequest request = new MockHttpServletRequest();
         MockHttpServletRequest request = new MockHttpServletRequest();
-        request.getSession().setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY,
+        request.getSession()
+               .setAttribute(HttpSessionContextIntegrationFilter.ACEGI_SECURITY_CONTEXT_KEY,
             "NOT_A_CONTEXT_OBJECT");
             "NOT_A_CONTEXT_OBJECT");
 
 
         MockHttpServletResponse response = new MockHttpServletResponse();
         MockHttpServletResponse response = new MockHttpServletResponse();
@@ -246,14 +296,6 @@ public class HttpSessionContextIntegrationFilterTests extends TestCase {
             ((SecurityContext) context).getAuthentication());
             ((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 ==========================================================
     //~ Inner Classes ==========================================================
 
 
     private class MockFilterChain extends TestCase implements FilterChain {
     private class MockFilterChain extends TestCase implements FilterChain {